Merge pull request swiftlang#42194 from beccadax/witness-accounts-are-only-approximate

beccadax · web-flow · commit 4caaa3bf249c · 2022-04-07T22:21:16.000-07:00
Make overloads and witnesses permit @sendable variance
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -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 "
@@ -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,
@@ -2602,6 +2612,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))
diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h
@@ -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>;
 
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
@@ -3181,6 +3181,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);
+  }
+
+  if (matchMode.contains(TypeMatchFlags::IgnoreFunctionSendability)) {
+    ext1 = ext1.withConcurrent(false);
+    ext2 = ext2.withConcurrent(false);
   }
 
   // If specified, allow an escaping function parameter to override a
diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp
@@ -518,6 +518,7 @@ static bool noteFixableMismatchedTypes(ValueDecl *decl, const ValueDecl *base) {
 namespace {
   enum class OverrideCheckingAttempt {
     PerfectMatch,
+    MismatchedSendability,
     MismatchedOptional,
     MismatchedTypes,
     BaseName,
@@ -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);
+    break;
   case OverrideCheckingAttempt::BaseName:
     diags.diagnose(decl, diag::override_multiple_decls_arg_mismatch,
                    decl->getName());
@@ -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();
@@ -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);
@@ -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);
+    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);
   }
@@ -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:
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -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);
@@ -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;
+          });
+      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;
 
@@ -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);
@@ -4125,6 +4154,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);
 
diff --git a/lib/Sema/TypeCheckProtocol.h b/lib/Sema/TypeCheckProtocol.h
@@ -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,
 
@@ -474,6 +478,7 @@ struct RequirementMatch {
     switch(Kind) {
     case MatchKind::ExactMatch:
     case MatchKind::FewerEffects:
+    case MatchKind::RequiresNonSendable:
       return true;
 
     case MatchKind::OptionalityConflict:
@@ -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;
@@ -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:
diff --git a/test/attr/attr_concurrent.swift b/test/attr/attr_concurrent.swift
@@ -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() }
+}
diff --git a/test/decl/protocol/conforms/associated_type.swift b/test/decl/protocol/conforms/associated_type.swift
@@ -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 {
@@ -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 {
@@ -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)
 }
@@ -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) { }
 }
