Make @preconcurrency import hide sendable variance #42255
Merged
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
If a type is explicitly non-Sendable, we don’t want to recommend softening errors with `@preconcurrency import` (even thought that would still work), because these probably represent *real* errors that you ought to fix. And at the extreme end of this, if a module is built with `-warn-concurrency` or `-swift-version 6`, *all* of its types should be treated as though they have explicit sendability, since we assume the entire module has been audited. (Eventually, we plan to do this at module build time by actually serializing out unavailable conformances rather than by looking at the settings used to build the module, but we aren’t there quite yet.) Implement this and test it by checking that the preconcurrency sendable tests never recommend adding `@preconcurrency` to an import of `StrictModule`, even when we do the same illegal things with it that cause that remark to be printed for `NonStrictModule`.
beccadax
changed the title
|
@swift-ci please smoke test |
beccadax
commented
Apr 8, 2022
beccadax
commented
Apr 8, 2022
lib/Sema/TypeCheckConcurrency.h
Outdated
| /// | ||
| /// \returns \c true if any errors were produced, \c false if no diagnostics or | ||
| /// only warnings and notes were produced. | ||
| bool diagnoseAsNonSendableBasedOn( |
There was a problem hiding this comment.
This could probably use a better name—suggestions welcome.
There was a problem hiding this comment.
diagnoseAsNonSendableFromContext? It's not that much better.
There was a problem hiding this comment.
I ended up with diagnoseSendabilityErrorBasedOn(), which is at least more vague about the kind of error we're emitting.
DougGregor
approved these changes
Apr 8, 2022
lib/Sema/TypeCheckConcurrency.h
Outdated
| /// | ||
| /// \returns \c true if any errors were produced, \c false if no diagnostics or | ||
| /// only warnings and notes were produced. | ||
| bool diagnoseAsNonSendableBasedOn( |
There was a problem hiding this comment.
diagnoseAsNonSendableFromContext? It's not that much better.
`diagnoseAsNonSendableBasedOn()` extracts just the “make a diagnostic sensitive to @preconcurrency import of this type” logic from `diagnoseNonSendableTypes()`. We need it because conformance and override checking should be sensitive to whether the conformed-to protocol/base class was imported with @preconcurrency or not, although this change is a pure refactoring.
When the conformance checker and override checker detect a difference in a function type’s @sendable attribute that varies in an illegal way, they now check if the protocol/base class was imported with an @preconcurrency import, and either limit the diagnostic or suggest adding @preconcurrency to the import as appropriate. Completes rdar://91109455.
beccadax
force-pushed
the
witness-accounts-are-only-approximate-2
branch
from
9625e48 to
ecdd42f
Compare
|
@swift-ci smoke test and merge |
|
@swift-ci please smoke test linux platform |
|
@swift-ci smoke test macOS platform |
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.
When the conformance checker and override checker detect a difference in a function type’s
@Sendableattribute that varies in an illegal way, they now check if the protocol/base class was imported with an@preconcurrency import, and either limit the diagnostic or suggest adding@preconcurrencyto the import as appropriate.Additionally, this PR makes it so we don't emit the remark recommending that
@preconcurrencybe added to an import if the type we're using has explicit sendability; after all, the author of the type has audited it and determined that any sendability problems in your code are not an error. Types inisConcurrencyChecked()modules are treated as always having explicit sendability for these purposes, since eventually we want to eliminateisConcurrencyChecked()and just serialize their public types as though they are explicitly non-sendable.Completes rdar://91109455. Followup for #42194.