Skip to content

Make overloads and witnesses permit @Sendable variance #42194

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2155,6 +2155,13 @@ ERROR(witness_not_accessible_proto,none,
"requirement in %select{private|fileprivate|internal|public|%error}4 protocol "
"%5",
(RequirementKind, DeclName, bool, AccessLevel, AccessLevel, Identifier))
ERROR(witness_not_as_sendable,none,
"sendability of function types in %0 %1 does not match requirement in "
"protocol %2",
(DescriptiveDeclKind, DeclName, Identifier))
NOTE(less_sendable_reqt_here,none,
"expected sendability to match requirement here",
())
ERROR(witness_not_accessible_type,none,
"%select{initializer %1|method %1|%select{|setter for }2property %1"
"|subscript%select{| setter}2}0 must be as accessible as its enclosing "
Expand Down Expand Up @@ -2303,6 +2310,9 @@ NOTE(ambiguous_witnesses_type,none,
"multiple matching types named %0", (Identifier))
NOTE(protocol_witness_exact_match,none,
"candidate exactly matches%0", (StringRef))
NOTE(protocol_witness_non_sendable,none,
"candidate matches except for closure sendability%0%select{; this will be "
"an error in Swift 6|}1", (StringRef, bool))
NOTE(protocol_witness_renamed,none,
"rename to %0 to satisfy this requirement%1", (DeclName, StringRef))
NOTE(protocol_witness_kind_conflict,none,
Expand Down Expand Up @@ -2596,6 +2606,9 @@ WARNING(generic_param_usable_from_inline_warn,none,
ERROR(override_multiple_decls_base,none,
"declaration %0 cannot override more than one superclass declaration",
(DeclName))
ERROR(override_sendability_mismatch,none,
"declaration %0 has a type with different sendability from any potential "
"overrides", (DeclName))
ERROR(override_multiple_decls_arg_mismatch,none,
"declaration %0 has different argument labels from any potential "
"overrides", (DeclName))
Expand Down
4 changes: 3 additions & 1 deletion include/swift/AST/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,9 @@ enum class TypeMatchFlags {
/// to be non-escaping, but Swift currently does not.
IgnoreNonEscapingForOptionalFunctionParam = 1 << 4,
/// Allow compatible opaque archetypes.
AllowCompatibleOpaqueTypeArchetypes = 1 << 5
AllowCompatibleOpaqueTypeArchetypes = 1 << 5,
/// Ignore the @Sendable attributes on functions when matching types.
IgnoreFunctionSendability = 1 << 6,
};
using TypeMatchOptions = OptionSet<TypeMatchFlags>;

Expand Down
10 changes: 10 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3167,6 +3167,16 @@ static bool matchesFunctionType(CanAnyFunctionType fn1, CanAnyFunctionType fn2,
matchMode.contains(TypeMatchFlags::AllowABICompatible))) {
ext1 = ext1.withThrows(true);
}

// 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);
Copy link
Member

Choose a reason for hiding this comment

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

Oh, good catch here.

}

if (matchMode.contains(TypeMatchFlags::IgnoreFunctionSendability)) {
ext1 = ext1.withConcurrent(false);
ext2 = ext2.withConcurrent(false);
}

// If specified, allow an escaping function parameter to override a
Expand Down
22 changes: 20 additions & 2 deletions lib/Sema/TypeCheckDeclOverride.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ static bool noteFixableMismatchedTypes(ValueDecl *decl, const ValueDecl *base) {
namespace {
enum class OverrideCheckingAttempt {
PerfectMatch,
MismatchedSendability,
MismatchedOptional,
MismatchedTypes,
BaseName,
Expand Down Expand Up @@ -547,6 +548,12 @@ static void diagnoseGeneralOverrideFailure(ValueDecl *decl,
diags.diagnose(decl, diag::override_multiple_decls_base,
decl->getName());
break;
case OverrideCheckingAttempt::MismatchedSendability:
// FIXME: suppress if any matches brought in via @preconcurrency import?
diags.diagnose(decl, diag::override_sendability_mismatch,
decl->getName())
.warnUntilSwiftVersion(6);
Copy link
Member

Choose a reason for hiding this comment

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

This is where we would do the @preconcurrency check, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

break;
case OverrideCheckingAttempt::BaseName:
diags.diagnose(decl, diag::override_multiple_decls_arg_mismatch,
decl->getName());
Expand Down Expand Up @@ -886,6 +893,7 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
DeclName name;
switch (attempt) {
case OverrideCheckingAttempt::PerfectMatch:
case OverrideCheckingAttempt::MismatchedSendability:
case OverrideCheckingAttempt::MismatchedOptional:
case OverrideCheckingAttempt::MismatchedTypes:
name = decl->getName();
Expand Down Expand Up @@ -976,6 +984,8 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
matchMode |= TypeMatchFlags::AllowNonOptionalForIUOParam;
matchMode |= TypeMatchFlags::IgnoreNonEscapingForOptionalFunctionParam;
}
if (attempt == OverrideCheckingAttempt::MismatchedSendability)
matchMode |= TypeMatchFlags::IgnoreFunctionSendability;

auto declFnTy = getDeclComparisonType()->getAs<AnyFunctionType>();
auto parentDeclTy = getMemberTypeForComparison(parentDecl, decl);
Expand Down Expand Up @@ -1277,8 +1287,15 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
if (emittedMatchError)
return true;

if (attempt == OverrideCheckingAttempt::MismatchedSendability) {
// FIXME: suppress if any matches brought in via @preconcurrency import?
diags.diagnose(decl, diag::override_sendability_mismatch,
decl->getName())
.warnUntilSwiftVersion(6);
Copy link
Member

Choose a reason for hiding this comment

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

... and here, for @preconcurrency checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And yes.

diags.diagnose(baseDecl, diag::overridden_here);
}
// Catch-all to make sure we don't silently accept something we shouldn't.
if (attempt != OverrideCheckingAttempt::PerfectMatch) {
else if (attempt != OverrideCheckingAttempt::PerfectMatch) {
OverrideMatch match{decl, /*isExact=*/false};
diagnoseGeneralOverrideFailure(decl, match, attempt);
}
Expand Down Expand Up @@ -1374,11 +1391,12 @@ bool swift::checkOverrides(ValueDecl *decl) {
switch (attempt) {
case OverrideCheckingAttempt::PerfectMatch:
break;
case OverrideCheckingAttempt::MismatchedOptional:
case OverrideCheckingAttempt::MismatchedSendability:
// Don't keep looking if the user didn't indicate it's an override.
if (!decl->getAttrs().hasAttribute<OverrideAttr>())
return false;
break;
case OverrideCheckingAttempt::MismatchedOptional:
case OverrideCheckingAttempt::MismatchedTypes:
break;
case OverrideCheckingAttempt::BaseName:
Expand Down
54 changes: 48 additions & 6 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,25 @@ getTypesToCompare(ValueDecl *reqt, Type reqtType, bool reqtTypeIsIUO,
// function types that aren't in a parameter can be Sendable or not.
// FIXME: Should we check for a Sendable bound on the requirement type?
bool inRequirement = (adjustment != TypeAdjustment::NoescapeToEscaping);
(void)adjustInferredAssociatedType(adjustment, reqtType, inRequirement);
Type adjustedReqtType =
adjustInferredAssociatedType(adjustment, reqtType, inRequirement);

bool inWitness = false;
Type adjustedWitnessType =
adjustInferredAssociatedType(adjustment, witnessType, inWitness);
if (inWitness && !inRequirement)
witnessType = adjustedWitnessType;

switch (variance) {
case VarianceKind::None:
break;
case VarianceKind::Covariant:
if (inRequirement && !inWitness)
reqtType = adjustedReqtType;
break;
case VarianceKind::Contravariant:
if (inWitness && !inRequirement)
witnessType = adjustedWitnessType;
break;
}
};

applyAdjustment(TypeAdjustment::NoescapeToEscaping);
Expand Down Expand Up @@ -1150,15 +1162,26 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
return *result;
}
}
if (!solution || solution->Fixes.size())
return RequirementMatch(witness, MatchKind::TypeConflict,
witnessType);
bool requiresNonSendable = false;
if (!solution || solution->Fixes.size()) {
/// 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;
});
Comment on lines +1167 to +1172
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This bit seems a little questionable—I'd like reviewers to let me know if they think it'll work as intended.

Copy link
Member

Choose a reason for hiding this comment

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

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).

if (!requiresNonSendable)
return RequirementMatch(witness, MatchKind::TypeConflict,
witnessType);
}

MatchKind matchKind = MatchKind::ExactMatch;
if (hasAnyError(optionalAdjustments))
matchKind = MatchKind::OptionalityConflict;
else if (anyRenaming)
matchKind = MatchKind::RenamedMatch;
else if (requiresNonSendable)
matchKind = MatchKind::RequiresNonSendable;
else if (getEffects(witness).containsOnly(getEffects(req)))
matchKind = MatchKind::FewerEffects;

Expand Down Expand Up @@ -2518,6 +2541,12 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
withAssocTypes);
break;

case MatchKind::RequiresNonSendable:
diags.diagnose(match.Witness, diag::protocol_witness_non_sendable,
withAssocTypes,
module->getASTContext().isSwiftVersionAtLeast(6));
break;

case MatchKind::RenamedMatch: {
auto diag = diags.diagnose(match.Witness, diag::protocol_witness_renamed,
req->getName(), withAssocTypes);
Expand Down Expand Up @@ -4259,6 +4288,19 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
requirement->getName());
});
}
if (best.Kind == MatchKind::RequiresNonSendable)
diagnoseOrDefer(requirement, getASTContext().isSwiftVersionAtLeast(6),
[this, requirement, witness]
(NormalProtocolConformance *conformance) {
auto &diags = DC->getASTContext().Diags;

diags.diagnose(getLocForDiagnosingWitness(conformance, witness),
diag::witness_not_as_sendable,
witness->getDescriptiveKind(), witness->getName(),
conformance->getProtocol()->getName())
.warnUntilSwiftVersion(6);
diags.diagnose(requirement, diag::less_sendable_reqt_here);
});

auto check = checkWitness(requirement, best);

Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/TypeCheckProtocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ enum class MatchKind : uint8_t {
/// The witness has fewer effects than the requirement, which is okay.
FewerEffects,

/// The witness is @Sendable and the requirement is not. Okay in certain
/// language modes.
RequiresNonSendable,

/// There is a difference in optionality.
OptionalityConflict,

Expand Down Expand Up @@ -474,6 +478,7 @@ struct RequirementMatch {
switch(Kind) {
case MatchKind::ExactMatch:
case MatchKind::FewerEffects:
case MatchKind::RequiresNonSendable:
return true;

case MatchKind::OptionalityConflict:
Expand Down Expand Up @@ -510,6 +515,7 @@ struct RequirementMatch {
switch(Kind) {
case MatchKind::ExactMatch:
case MatchKind::FewerEffects:
case MatchKind::RequiresNonSendable:
case MatchKind::OptionalityConflict:
case MatchKind::RenamedMatch:
return true;
Expand Down Expand Up @@ -545,6 +551,7 @@ struct RequirementMatch {
switch(Kind) {
case MatchKind::ExactMatch:
case MatchKind::FewerEffects:
case MatchKind::RequiresNonSendable:
case MatchKind::RenamedMatch:
case MatchKind::TypeConflict:
case MatchKind::MissingRequirement:
Expand Down
28 changes: 28 additions & 0 deletions test/attr/attr_concurrent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,31 @@ func testExplicitConcurrentClosure() {
}
let _: String = fn // expected-error{{cannot convert value of type '@Sendable () -> Int' to specified type 'String'}}
}

class SuperSendable {
func runsInBackground(_: @Sendable () -> Void) {}
func runsInForeground(_: () -> Void) {} // expected-note {{overridden declaration is here}}
func runnableInBackground() -> @Sendable () -> Void { fatalError() } // expected-note {{overridden declaration is here}}
func runnableInForeground() -> () -> Void { fatalError() }
}

class SubSendable: SuperSendable {
override func runsInBackground(_: () -> Void) {}
override func runsInForeground(_: @Sendable () -> Void) {} // expected-warning {{declaration 'runsInForeground' has a type with different sendability from any potential overrides; this is an error in Swift 6}}
override func runnableInBackground() -> () -> Void { fatalError() } // expected-warning {{declaration 'runnableInBackground()' has a type with different sendability from any potential overrides; this is an error in Swift 6}}
override func runnableInForeground() -> @Sendable () -> Void { fatalError() }
}

protocol AbstractSendable {
func runsInBackground(_: @Sendable () -> Void)
func runsInForeground(_: () -> Void) // expected-note {{expected sendability to match requirement here}}
func runnableInBackground() -> @Sendable () -> Void // expected-note {{expected sendability to match requirement here}}
func runnableInForeground() -> () -> Void
}

struct ConcreteSendable: AbstractSendable {
func runsInBackground(_: () -> Void) {}
func runsInForeground(_: @Sendable () -> Void) {} // expected-warning {{sendability of function types in instance method 'runsInForeground' does not match requirement in protocol 'AbstractSendable'; this is an error in Swift 6}}
func runnableInBackground() -> () -> Void { fatalError() } // expected-warning {{sendability of function types in instance method 'runnableInBackground()' does not match requirement in protocol 'AbstractSendable'; this is an error in Swift 6}}
func runnableInForeground() -> @Sendable () -> Void { fatalError() }
}
18 changes: 9 additions & 9 deletions test/decl/protocol/conforms/associated_type.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Foo: FooType {
protocol P1 {
associatedtype A

func f(_: A) // expected-note {{protocol requires function 'f' with type '(@escaping SendableX1b.A) -> ()' (aka '(@escaping @Sendable (Int) -> Int) -> ()'); do you want to add a stub?}}
func f(_: A) // expected-note {{expected sendability to match requirement here}}
}

struct X1a : P1 {
Expand All @@ -61,10 +61,10 @@ struct X1b : P1 {
// instead of adjusting types before adding them to the constraint
// graph, we should introduce a new constraint kind that allows only a
// witness's adjustments.
struct SendableX1b : P1 { // expected-error {{type 'SendableX1b' does not conform to protocol 'P1'}}
struct SendableX1b : P1 {
typealias A = @Sendable (Int) -> Int

func f(_: (Int) -> Int) { } // expected-note {{candidate has non-matching type '((Int) -> Int) -> ()'}}
func f(_: (Int) -> Int) { } // expected-warning {{sendability of function types in instance method 'f' does not match requirement in protocol 'P1'; this is an error in Swift 6}}
}

struct X1c : P1 {
Expand All @@ -78,8 +78,8 @@ struct X1d : P1 {
}

protocol P2 {
func f(_: (Int) -> Int) // expected-note 3 {{protocol requires function 'f' with type '((Int) -> Int) -> ()'; do you want to add a stub?}}
func g(_: @escaping (Int) -> Int) // expected-note 2 {{protocol requires function 'g' with type '(@escaping (Int) -> Int) -> ()'; do you want to add a stub?}}
func f(_: (Int) -> Int) // expected-note{{expected sendability to match requirement here}} expected-note 2{{protocol requires function 'f' with type '((Int) -> Int) -> ()'; do you want to add a stub?}}
func g(_: @escaping (Int) -> Int) // expected-note 2 {{expected sendability to match requirement here}}
func h(_: @Sendable (Int) -> Int) // expected-note 2 {{protocol requires function 'h' with type '(@Sendable (Int) -> Int) -> ()'; do you want to add a stub?}}
func i(_: @escaping @Sendable (Int) -> Int)
}
Expand All @@ -98,16 +98,16 @@ struct X2b : P2 { // expected-error{{type 'X2b' does not conform to protocol 'P2
func i(_: @escaping (Int) -> Int) { }
}

struct X2c : P2 { // expected-error{{type 'X2c' does not conform to protocol 'P2'}}
func f(_: @Sendable (Int) -> Int) { } // expected-note{{candidate has non-matching type '(@Sendable (Int) -> Int) -> ()'}}
func g(_: @Sendable (Int) -> Int) { } // expected-note{{candidate has non-matching type '(@Sendable (Int) -> Int) -> ()'}}
struct X2c : P2 {
func f(_: @Sendable (Int) -> Int) { } // expected-warning{{sendability of function types in instance method 'f' does not match requirement in protocol 'P2'; this is an error in Swift 6}}
func g(_: @Sendable (Int) -> Int) { } // expected-warning{{sendability of function types in instance method 'g' does not match requirement in protocol 'P2'; this is an error in Swift 6}}
func h(_: @Sendable (Int) -> Int) { }
func i(_: @Sendable (Int) -> Int) { }
}

struct X2d : P2 { // expected-error{{type 'X2d' does not conform to protocol 'P2'}}
func f(_: @escaping @Sendable (Int) -> Int) { } // expected-note{{candidate has non-matching type '(@escaping @Sendable (Int) -> Int) -> ()'}}
func g(_: @escaping @Sendable (Int) -> Int) { } // expected-note{{candidate has non-matching type '(@escaping @Sendable (Int) -> Int) -> ()'}}
func g(_: @escaping @Sendable (Int) -> Int) { } // expected-warning{{sendability of function types in instance method 'g' does not match requirement in protocol 'P2'; this is an error in Swift 6}}
func h(_: @escaping @Sendable (Int) -> Int) { } // expected-note{{candidate has non-matching type '(@escaping @Sendable (Int) -> Int) -> ()'}}
func i(_: @escaping @Sendable (Int) -> Int) { }
}
Expand Down