Merge pull request #72277 from xedin/improvements-to-protocol-witness-matching

[Sema/ClangImporter] Improvements to witness matching

Author: xedin

include/swift/Sema/ConstraintLocator.h

@@ -820,18 +820,6 @@ class LocatorPathElt::Witness final : public StoredPointerElement<ValueDecl> {
   }
 };
 
-class LocatorPathElt::ProtocolRequirement final : public StoredPointerElement<ValueDecl> {
-public:
-  ProtocolRequirement(ValueDecl *decl)
-      : StoredPointerElement(PathElementKind::ProtocolRequirement, decl) {}
-
-  ValueDecl *getDecl() const { return getStoredPointer(); }
-
-  static bool classof(const LocatorPathElt *elt) {
-    return elt->getKind() == PathElementKind::ProtocolRequirement;
-  }
-};
-
 class LocatorPathElt::GenericParameter final : public StoredPointerElement<GenericTypeParamType> {
 public:
   GenericParameter(GenericTypeParamType *type)

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -161,7 +161,7 @@ SIMPLE_LOCATOR_PATH_ELT(ParentType)
 
 /// The requirement that we're matching during protocol conformance
 /// checking.
-CUSTOM_LOCATOR_PATH_ELT(ProtocolRequirement)
+SIMPLE_LOCATOR_PATH_ELT(ProtocolRequirement)
 
 /// Type parameter requirements.
 ABSTRACT_LOCATOR_PATH_ELT(AnyRequirement)

lib/AST/Type.cpp

@@ -994,6 +994,9 @@ Type TypeBase::stripConcurrency(bool recurse, bool dropGlobalActor) {
             existentialType->getConstraintType().getPointer())
       return Type(this);
 
+    if (newConstraintType->getClassOrBoundGenericClass())
+      return newConstraintType;
+
     return ExistentialType::get(newConstraintType);
   }
 

lib/ClangImporter/ImportType.cpp

@@ -1916,7 +1916,13 @@ class GetSendableType :
     auto composition =
         ProtocolCompositionType::get(ctx, members, inverses, explicitAnyObject);
 
-    return { composition, true };
+    // If we started from a protocol or a composition we should already
+    // be in an existential context. Otherwise we'd have to wrap a new
+    // composition into an existential.
+    if (isa<ProtocolType>(ty) || isa<ProtocolCompositionType>(ty))
+      return {composition, true};
+
+    return {ExistentialType::get(composition), true};
   }
 
   /// Visitor action: Recurse into the children of this type and try to add

lib/Sema/CSSimplify.cpp

@@ -5586,6 +5586,35 @@ bool ConstraintSystem::repairFailures(
     return false;
   }
 
+  if (auto *VD = getAsDecl<ValueDecl>(anchor)) {
+    // Matching a witness to a ObjC protocol requirement.
+    if (VD->isObjC() && VD->isProtocolRequirement() &&
+        path[0].is<LocatorPathElt::Witness>() && 
+        // Note that the condition below is very important,
+        // we need to wait until the very last moment to strip
+        // the concurrency annotations from the inner most type.
+        conversionsOrFixes.empty()) {
+      // Allow requirements to introduce `swift_attr` annotations
+      // (note that `swift_attr` in type contexts weren't supported
+      // before) and for witnesses to adopt them gradually by matching
+      // with a warning in non-strict concurrency mode.
+      if (!(Context.isSwiftVersionAtLeast(6) ||
+            Context.LangOpts.StrictConcurrencyLevel ==
+                StrictConcurrency::Complete)) {
+        auto strippedLHS = lhs->stripConcurrency(/*resursive=*/true,
+                                                 /*dropGlobalActor=*/true);
+        auto strippedRHS = rhs->stripConcurrency(/*resursive=*/true,
+                                                 /*dropGlobalActor=*/true);
+        auto result = matchTypes(strippedLHS, strippedRHS, matchKind,
+                                 flags | TMF_ApplyingFix, locator);
+        if (!result.isFailure()) {
+          increaseScore(SK_MissingSynthesizableConformance, locator);
+          return true;
+        }
+      }
+    }
+  }
+
   auto elt = path.back();
   switch (elt.getKind()) {
   case ConstraintLocator::LValueConversion: {
@@ -8560,7 +8589,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
         auto synthesizeConformance = [&]() {
           ProtocolConformanceRef synthesized(protocol);
           auto witnessLoc = getConstraintLocator(
-              /*anchor=*/{}, LocatorPathElt::Witness(witness));
+              locator.getAnchor(), LocatorPathElt::Witness(witness));
           SynthesizedConformances.insert({witnessLoc, synthesized});
           return recordConformance(synthesized);
         };

lib/Sema/ConstraintLocator.cpp

@@ -304,9 +304,7 @@ void LocatorPathElt::dump(raw_ostream &out) const {
     break;
   }
   case ConstraintLocator::ProtocolRequirement: {
-    auto reqElt = elt.castTo<LocatorPathElt::ProtocolRequirement>();
-    out << "protocol requirement ";
-    reqElt.getDecl()->dumpRef(out);
+    out << "protocol requirement";
     break;
   }
   case ConstraintLocator::Witness: {

lib/Sema/ConstraintSystem.cpp

@@ -7985,6 +7985,16 @@ void constraints::dumpAnchor(ASTNode anchor, SourceManager *SM,
       out << '@';
       pattern->getLoc().print(out, *SM);
     }
+  } else if (auto *decl = anchor.dyn_cast<Decl *>()) {
+    if (auto *VD = dyn_cast<ValueDecl>(decl)) {
+      VD->dumpRef(out);
+    } else {
+      out << "<<" << Decl::getKindName(decl->getKind()) << ">>";
+      if (SM) {
+        out << "@";
+        decl->getLoc().print(out, *SM);
+      }
+    }
   }
   // TODO(diagnostics): Implement the rest of the cases.
 }

lib/Sema/TypeCheckProtocol.cpp

@@ -1026,7 +1026,6 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
 
   // Initialized by the setup operation.
   std::optional<ConstraintSystem> cs;
-  ConstraintLocator *locator = nullptr;
   ConstraintLocator *reqLocator = nullptr;
   ConstraintLocator *witnessLocator = nullptr;
   Type witnessType, openWitnessType;
@@ -1115,8 +1114,8 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
       selfTy = syntheticEnv->mapTypeIntoContext(selfTy);
 
     // Open up the type of the requirement.
-    reqLocator = cs->getConstraintLocator(
-        static_cast<Expr *>(nullptr), LocatorPathElt::ProtocolRequirement(req));
+    reqLocator =
+        cs->getConstraintLocator(req, ConstraintLocator::ProtocolRequirement);
     OpenedTypeMap reqReplacements;
     reqType = cs->getTypeOfMemberReference(selfTy, req, dc,
                                            /*isDynamicResult=*/false,
@@ -1151,10 +1150,8 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
 
     // Open up the witness type.
     witnessType = witness->getInterfaceType();
-    // FIXME: witness as a base locator?
-    locator = cs->getConstraintLocator({});
-    witnessLocator = cs->getConstraintLocator({},
-                                              LocatorPathElt::Witness(witness));
+    witnessLocator =
+        cs->getConstraintLocator(req, LocatorPathElt::Witness(witness));
     if (witness->getDeclContext()->isTypeContext()) {
       openWitnessType = cs->getTypeOfMemberReference(
           selfTy, witness, dc, /*isDynamicResult=*/false,
@@ -1174,44 +1171,8 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
   // Match a type in the requirement to a type in the witness.
   auto matchTypes = [&](Type reqType,
                         Type witnessType) -> std::optional<RequirementMatch> {
-    // `swift_attr` attributes in the type context were ignored before,
-    // which means that we need to maintain status quo to avoid breaking
-    // witness matching by stripping everything concurrency related from
-    // inner types.
-    auto shouldStripConcurrency = [&]() {
-      if (!req->isObjC())
-        return false;
-
-      auto &ctx = dc->getASTContext();
-      return !(ctx.isSwiftVersionAtLeast(6) ||
-               ctx.LangOpts.StrictConcurrencyLevel ==
-                   StrictConcurrency::Complete);
-    };
-
-    if (shouldStripConcurrency()) {
-      if (reqType->is<FunctionType>()) {
-        auto *fnTy = reqType->castTo<FunctionType>();
-        SmallVector<AnyFunctionType::Param, 4> params;
-        llvm::transform(fnTy->getParams(), std::back_inserter(params),
-                        [&](const AnyFunctionType::Param &param) {
-                          return param.withType(
-                              param.getPlainType()->stripConcurrency(
-                                  /*recursive=*/true,
-                                  /*dropGlobalActor=*/true));
-                        });
-
-        auto resultTy =
-            fnTy->getResult()->stripConcurrency(/*recursive=*/true,
-                                                /*dropGlobalActor=*/true);
-
-        reqType = FunctionType::get(params, resultTy, fnTy->getExtInfo());
-      } else {
-        reqType = reqType->stripConcurrency(/*recursive=*/true,
-                                            /*dropGlobalActor=*/true);
-      }
-    }
-
-    cs->addConstraint(ConstraintKind::Bind, reqType, witnessType, locator);
+    cs->addConstraint(ConstraintKind::Bind, reqType, witnessType,
+                      witnessLocator);
     // FIXME: Check whether this has already failed.
     return std::nullopt;
   };
@@ -1235,14 +1196,31 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
           return *result;
       }
     }
-    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;
-        });
+
+    bool requiresNonSendable = [&]() {
+      if (!solution)
+        return false;
+
+      // If the *only* problems are that `@Sendable` attributes are missing,
+      // allow the match in some circumstances.
+      if (!solution->Fixes.empty()) {
+        return llvm::all_of(solution->Fixes,
+                            [](constraints::ConstraintFix *fix) {
+                              return fix->getKind() ==
+                                     constraints::FixKind::AddSendableAttribute;
+                            });
+      }
+
+      // If there are no other issues, let's check whether this are
+      // missing Sendable conformances when matching ObjC requirements.
+      // This is not an error until Swift 6 because `swift_attr` wasn't
+      // allowed in type contexts initially.
+      return req->isObjC() &&
+             solution->getFixedScore()
+                     .Data[SK_MissingSynthesizableConformance] > 0;
+    }();
+
+    if (!solution || !solution->Fixes.empty()) {
       if (!requiresNonSendable)
         return RequirementMatch(witness, MatchKind::TypeConflict,
                                 witnessType);

test/Concurrency/sendable_objc_attr_in_type_context_swift5.swift

@@ -58,6 +58,7 @@ void doSomethingConcurrently(__attribute__((noescape)) void SWIFT_SENDABLE (^blo
 @end
 
 @protocol InnerSendableTypes
+-(void) testComposition:(SWIFT_SENDABLE MyValue *)composition;
 -(void) test:(NSDictionary<NSString *, SWIFT_SENDABLE id> *)options;
 -(void) testWithCallback:(NSString *)name handler:(MAIN_ACTOR void (^)(NSDictionary<NSString *, SWIFT_SENDABLE id> *, NSError * _Nullable))handler;
 @end
@@ -109,21 +110,26 @@ func test_sendable_attr_in_type_context(test: Test) {
 }
 
 class TestConformanceWithStripping : InnerSendableTypes {
-  func test(_ options: [String: Any]) { // Ok
+  func testComposition(_: MyValue) {
+    // expected-warning@-1 {{sendability of function types in instance method 'testComposition' does not match requirement in protocol 'InnerSendableTypes'; this is an error in the Swift 6 language mode}}
+  }
+
+  func test(_ options: [String: Any]) {
+    // expected-warning@-1 {{sendability of function types in instance method 'test' does not match requirement in protocol 'InnerSendableTypes'; this is an error in the Swift 6 language mode}}
   }
 
   func test(withCallback name: String, handler: @escaping @MainActor ([String : Any], (any Error)?) -> Void) { // Ok
+    // expected-warning@-1 {{sendability of function types in instance method 'test(withCallback:handler:)' does not match requirement in protocol 'InnerSendableTypes'; this is an error in the Swift 6 language mode}}
   }
 }
 
 class TestConformanceWithoutStripping : InnerSendableTypes {
-  // expected-error@-1 {{type 'TestConformanceWithoutStripping' does not conform to protocol 'InnerSendableTypes'}}
+  func testComposition(_: any MyValue & Sendable) { // Ok
+  }
 
-  func test(_ options: [String: any Sendable]) {
-    // expected-note@-1 {{candidate has non-matching type '([String : any Sendable]) -> ()'}}
+  func test(_ options: [String: any Sendable]) { // Ok
   }
 
-  func test(withCallback name: String, handler: @escaping @MainActor ([String : any Sendable], (any Error)?) -> Void) {
-    // expected-note@-1 {{candidate has non-matching type '(String, @escaping @MainActor ([String : any Sendable], (any Error)?) -> Void) -> ()'}}
+  func test(withCallback name: String, handler: @escaping @MainActor ([String : any Sendable], (any Error)?) -> Void) { // Ok
   }
 }

test/Concurrency/sendable_objc_attr_in_type_context_swift5_strict.swift

@@ -59,6 +59,7 @@ void doSomethingConcurrently(__attribute__((noescape)) void SWIFT_SENDABLE (^blo
 @end
 
 @protocol InnerSendableTypes
+-(void) testComposition:(SWIFT_SENDABLE MyValue *)composition;
 -(void) test:(NSDictionary<NSString *, SWIFT_SENDABLE id> *)options;
 -(void) testWithCallback:(NSString *)name handler:(MAIN_ACTOR void (^)(NSDictionary<NSString *, SWIFT_SENDABLE id> *, NSError * _Nullable))handler;
 @end
@@ -116,6 +117,10 @@ func test_sendable_attr_in_type_context(test: Test) {
 class TestConformanceWithStripping : InnerSendableTypes {
   // expected-error@-1 {{type 'TestConformanceWithStripping' does not conform to protocol 'InnerSendableTypes'}}
 
+  func testComposition(_: MyValue) {
+    // expected-note@-1 {{candidate has non-matching type '(MyValue) -> ()'}}
+  }
+
   func test(_ options: [String: Any]) {
     // expected-note@-1 {{candidate has non-matching type '([String : Any]) -> ()'}}
   }
@@ -126,6 +131,9 @@ class TestConformanceWithStripping : InnerSendableTypes {
 }
 
 class TestConformanceWithoutStripping : InnerSendableTypes {
+  func testComposition(_: MyValue & Sendable) { // Ok
+  }
+
   func test(_ options: [String: any Sendable]) { // Ok
   }