-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Sema] Warn about 'Any' to 'any Sendable' override #73543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Sema] Warn about 'Any' to 'any Sendable' override #73543
Conversation
@swift-ci please smoke test |
919483f
to
9b22b0b
Compare
bool failed = true; | ||
if (baseDecl->preconcurrency() && | ||
(attempt == OverrideCheckingAttempt::PerfectMatch || | ||
attempt == OverrideCheckingAttempt::MismatchedSendability)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One major drawback of this approach is that it strips concurrency annotations from both sides which means that override with concurrency annotations would match against parent without annotation(s) in the same position which is not the case for witness matching today. For example:
protocol P {
@preconcurrency func test(_: Any)
}
struct S : P {
func test(_: any Sendable) {}
}
Produces the following error:
error: type 'S' does not conform to protocol 'P'
1 | protocol P {
2 | @preconcurrency func test(_: Any)
| `- note: protocol requires function 'test' with type '(Any) -> ()'; add a stub for conformance
3 | }
4 |
5 | struct S : P {
| `- error: type 'S' does not conform to protocol 'P'
6 | func test(_: any Sendable) {}
| `- note: candidate has non-matching type '(any Sendable) -> ()'
7 | }
Taking a step back here, at line https://github.com/apple/swift/pull/73543/files#diff-149ea7785899898da39642522a1b7d555757f96e8a3d53f045f9ae80e04c361bR1208 the types are checked for exact match which means that it should be possible to set a new flag based on whether base is marked as preconcurrency or not before attempting propertyTy->matches(parentPropertyCanTy, options)
and handle it inside matches
but that would make it harder to deal with check that compares object types (that happens right under this one) and diagnose sendability mismatches as warnings, so I'd like to propose a combination:
- We still ran
if (!propertyTy->matches(parentPropertyCanTy, options))
first; - If that fails we add
TypeMatchFlags::IgnoreSendability
andTypeMatchFlags::IgnoreFunctionSendability
and re-runmatches
- If
propertyTy->matches(parentPropertyCanTy, options)
returnstrue
we setmismatchedOnSendability = true;
this would allow us diagnose sendability mismatch and avoid dealing with optionality mismatches diagnosed in https://github.com/apple/swift/pull/73543/files#diff-149ea7785899898da39642522a1b7d555757f96e8a3d53f045f9ae80e04c361bR1304 - Change
matches
to handle the case whereany Sendable
is matched againstAny
and ignore it ifIgnoreSendability
flag is set.
- This means that
matches
has to be a bit more work by walking into tuples and collections but only if the flag is set.
} | ||
|
||
class Child : Parent { | ||
override var userInfo: [String : Any] { // expected-warning {{declaration 'userInfo' has a type with different sendability from any potential overrides; this is an error in the Swift 6 language mode}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a comment about adding more tests with different combinations of attributes and any Sendable
use in the previous PR.
@@ -905,6 +905,25 @@ unsigned int TypeBase::getOptionalityDepth() { | |||
} | |||
|
|||
Type TypeBase::stripConcurrency(bool recurse, bool dropGlobalActor) { | |||
if (auto *arrayTy = dyn_cast<ArraySliceType>(this)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not technically tied for this PR since matching happens on canonical types, you can land this separately.
Until Swift 6, warn about overriding Any property in parent class to any Sendable
9b22b0b
to
b4cf34b
Compare
Until Swift 6, warn about overriding Any property in parent class to any Sendable