Sema: Ban global actor on getters in a future lang mode, and more #80399
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AnthonyLatsis
requested review from
ktoso,
hborla,
slavapestov and
xedin
as code owners
|
@swift-ci please smoke test macOS |
AnthonyLatsis
force-pushed
the
danaus-plexippus-5
branch
2 times, most recently
from
bd74bc8 to
d587460
Compare
|
@swift-ci please smoke test macOS |
1 similar comment
|
@swift-ci please smoke test macOS |
| } else if (auto *accessor = dyn_cast<AccessorDecl>(decl)) { | ||
| auto &ctx = accessor->getASTContext(); | ||
| const auto attrRange = globalActorAttr->getRangeWithAt(); | ||
| const unsigned langModeForError = accessor->isGetter() ? 7 : 6; |
There was a problem hiding this comment.
FYI we should replace 7 with Version::getFutureMajorLanguageVersion here once #80853 lands (doesn't have to block this PR though)
Global actor attributes were hitherto allowed on getters and (until Swift 6) other accessors, but intendedly ignored in favor of the enclosing `var` or `subscript` declaration's isolation when accessing the storage declaration. This change deprecates the getter case as well, to become an error in the next language mode. Also do a better job at suggesting to move the attribute to the storage: * Tighten up the condition for when the attr is legal on the storage. * Don't suggest if the storage already specifies some kind of explicit isolation. * Highlight the attr. * Also remove the attr from the accessor.
Accesses to storage declarations are checked relative to the storage's isolation, not the accessor's, whereas accessors are checked relative to their own isolation. This inconsistency exposes a data race safety hole because global actor attributes are allowed on accessors in Swift 5 and even in Swift 6 on getters. This fixes the bug by inferring an accessor's actor isolation from its from its storage declaration in Swift 6 and onward. Expected source compatibility impact is negligible. First, an accessor with a global actor attribute is not a common pattern. Second, this change will only affect clients that depend on resilient dynamic libraries that declare such an accessor as inlinable.
AnthonyLatsis
force-pushed
the
danaus-plexippus-5
branch
from
d587460 to
fd7e187
Compare
|
@swift-ci please smoke test macOS |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Global actor attributes were hitherto allowed on getters and (until Swift 6) other accessors, but intendedly ignored in favor of the enclosing
varorsubscriptdeclaration's isolation when accessing the storage declaration. This change deprecates the getter case as well, to become an error in the next language mode.Also do a better job at suggesting to move the attribute to the storage:
Accesses to storage declarations are checked relative to the storage's isolation, not the accessor's, whereas accessors are checked relative to their own isolation. This inconsistency exposes a data race safety hole because global actor attributes are allowed on accessors in Swift 5 and even in Swift 6 on getters.
This fixes the bug by inferring an accessor's actor isolation from its from its storage declaration in Swift 6 and onward.
Expected source compatibility impact is negligible. First, an accessor with a global actor attribute is not a common pattern. Second, this change will only affect clients that depend on resilient dynamic libraries that declare such an accessor as inlinable.