Skip to content

Conversation

@xymus
Copy link
Contributor

@xymus xymus commented Nov 20, 2024

Ensure that calls to SPI operators are only accepted in source files importing the corresponding SPI group. This applies the same logic to operators as we do for functions, patching a hole in type-checking. SPI operators were already correctly detected when used in inlinable code and printed as expected in public module interfaces only.

rdar://137713966

@xymus xymus requested a review from tshortli November 20, 2024 18:12
@xymus xymus changed the title Sema: Restrict use of SPI operators to files importing the same SPI group Sema: Restrict use of SPI operators to files importing the corresponding SPI group Nov 20, 2024
@xymus
Copy link
Contributor Author

xymus commented Nov 20, 2024

@swift-ci Please smoke test

// RUN: %target-swift-frontend -typecheck %s -emit-module-interface-path %t/Main.swiftinterface -emit-private-module-interface-path %t/Main.private.swiftinterface -enable-library-evolution -swift-version 5 -I %t -module-name Main
// RUN: %FileCheck -check-prefix=CHECK-PUBLIC %s < %t/Main.swiftinterface
// RUN: %FileCheck -check-prefix=CHECK-PRIVATE %s < %t/Main.private.swiftinterface
// RUN: cat %t/Main.swiftinterface | %FileCheck -check-prefix=CHECK-PUBLIC %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use cat instead of < here, out of curiosity? FWIW, switching to --input-file instead would improve debugging somewhat (that way FileCheck will print the path to the file when there are mismatches)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was it, I prefer that style now to see the name of the file in the outputs. I'll take a look at --input-file, maybe it will be worth switching style again.

lib/AST/Decl.cpp Outdated
case AccessLevel::Public:
case AccessLevel::Open:
case AccessLevel::Open: {
if (VD->isOperator() && VD->isSPI()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth a comment explaining why operators need to be special cased here. I'm having trouble following the surrounding logic and why the existing centralized logic does not apply to them.

@xymus xymus force-pushed the restrict-spi-operators branch from 1548887 to 04378f1 Compare November 12, 2025 18:15
@xymus xymus requested a review from DougGregor as a code owner November 12, 2025 18:15
@xymus
Copy link
Contributor Author

xymus commented Nov 12, 2025

@swift-ci Please smoke test

…erface

This was already working as expected. Adding a test to ensure it doesn't
regress.
Ensure that calls to SPI operators are only accepted in source files
importing the corresponding SPI group. This applies the same logic to
operators as we do for functions.

rdar://137713966
This new check is source breaking. Let's gate it behind an opt-in flag
for now and enable it by default on a new language mode or similar.
@xymus xymus force-pushed the restrict-spi-operators branch from 04378f1 to a4865be Compare November 12, 2025 18:47
@xymus
Copy link
Contributor Author

xymus commented Nov 12, 2025

@swift-ci Please smoke test

@xymus
Copy link
Contributor Author

xymus commented Nov 12, 2025

@swift-ci Please smoke test macOS

@xymus xymus merged commit 3545603 into swiftlang:main Nov 13, 2025
3 checks passed
@xymus xymus deleted the restrict-spi-operators branch November 13, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants