Make overloads and witnesses permit @Sendable variance #42194
Conversation
Specifically, an override can validly have a parameter type that is not `@Sendable` or a function type that is `@Sendable` even if that contradicts the base declaration. That’s becuase it’s always valid to provide a sendable function when one is not actually needed, but not vice versa.
It previously treated parameters and return types as the same, instead of properly handling variance.
| /// If the *only* problems are that `@Sendable` attributes are missing, | ||
| /// allow the match in some circumstances. | ||
| requiresNonSendable = solution | ||
| && llvm::all_of(solution->Fixes, [](constraints::ConstraintFix *fix) { | ||
| return fix->getKind() == constraints::FixKind::AddSendableAttribute; | ||
| }); |
There was a problem hiding this comment.
This bit seems a little questionable—I'd like reviewers to let me know if they think it'll work as intended.
There was a problem hiding this comment.
I think this will work. I do wonder if we need to do a similar thing for global-actor-qualified function parameters (although I suspect they'll be much more rare).
beccadax
force-pushed
the
witness-accounts-are-only-approximate
branch
from
46dae55 to
4ab5233
Compare
|
@swift-ci please smoke test |
| // Removing '@Sendable' is ABI-compatible because there's nothing wrong with | ||
| // a function being sendable when it doesn't need to be. | ||
| if (!ext2.isSendable()) | ||
| ext1 = ext1.withConcurrent(false); |
| // FIXME: suppress if any matches brought in via @preconcurrency import? | ||
| diags.diagnose(decl, diag::override_sendability_mismatch, | ||
| decl->getName()) | ||
| .warnUntilSwiftVersion(6); |
There was a problem hiding this comment.
This is where we would do the @preconcurrency check, right?
| // FIXME: suppress if any matches brought in via @preconcurrency import? | ||
| diags.diagnose(decl, diag::override_sendability_mismatch, | ||
| decl->getName()) | ||
| .warnUntilSwiftVersion(6); |
There was a problem hiding this comment.
... and here, for @preconcurrency checks.
| /// If the *only* problems are that `@Sendable` attributes are missing, | ||
| /// allow the match in some circumstances. | ||
| requiresNonSendable = solution | ||
| && llvm::all_of(solution->Fixes, [](constraints::ConstraintFix *fix) { | ||
| return fix->getKind() == constraints::FixKind::AddSendableAttribute; | ||
| }); |
There was a problem hiding this comment.
I think this will work. I do wonder if we need to do a similar thing for global-actor-qualified function parameters (although I suspect they'll be much more rare).
|
@swift-ci smoke test and merge |
|
Flaky test? @swift-ci please smoke test macOS platform |
For both overloaded methods and protocol witnesses, this PR:
@Sendablefunction type in the override/witness's parameters to match an@Sendablefunction type in the base/requirement.@Sendablefunction type in the override/witness's results to match a non-@Sendablefunction type in the base/requirement.We'd like these warnings to also go away when
@preconcurrencyis used, but this seems like a reasonable stopping point.Fixes rdar://91109455.