Merge pull request #70569 from tshortli/dont-be-such-a-completionist

AST: Improve handling of missing @objc async protocol witnesses

Author: tshortli

lib/Sema/TypeCheckProtocol.cpp

@@ -3804,7 +3804,7 @@ filterProtocolRequirements(
 
 /// Prune the set of missing witnesses for the given conformance, eliminating
 /// any requirements that do not actually need to satisfied.
-static ArrayRef<MissingWitness> pruneMissingWitnesses( // FIXME: this does not remove the missing witness note here!!!
+static ArrayRef<MissingWitness> pruneMissingWitnesses(
     ConformanceChecker &checker,
     ProtocolDecl *proto,
     NormalProtocolConformance *conformance,
@@ -3814,14 +3814,6 @@ static ArrayRef<MissingWitness> pruneMissingWitnesses( // FIXME: this does not r
     return missingWitnesses;
   }
 
-  // For an @objc protocol defined in Objective-C, the Clang importer might
-  // have imported the same underlying Objective-C declaration as more than
-  // one Swift declaration. If we aren't in an imported @objc protocol, there
-  // is nothing to do.
-  if (!proto->isObjC()) {
-    return missingWitnesses;
-  }
-
   // Consider each of the missing witnesses to remove any that should not
   // longer be considered "missing".
   llvm::SmallDenseSet<ConformanceChecker::ObjCMethodKey>
@@ -3857,30 +3849,46 @@ static ArrayRef<MissingWitness> pruneMissingWitnesses( // FIXME: this does not r
       continue;
     }
 
+    // For an @objc protocol defined in Objective-C, the Clang importer might
+    // have imported the same underlying Objective-C declaration as more than
+    // one Swift declaration. Only one of those requirements should be witnessed
+    // so if the requirement sibling is witnessed then this requirement is not
+    // missing.
+
+    // Prune requirements that come from other protocols - we don't want to
+    // attempt to diagnose those here since we won't be able to find their
+    // siblings.
+    if (proto != missingWitness.requirement->getDeclContext()->getAsDecl()) {
+      skipWitness();
+      continue;
+    }
+
+    if (!proto->isObjC()) {
+      addWitness();
+      continue;
+    }
+
     auto fnRequirement = cast<AbstractFunctionDecl>(missingWitness.requirement);
     auto key = checker.getObjCMethodKey(fnRequirement);
 
-    // If we have already reported a function with this selector as missing,
-    // don't do it again.
-    if (alreadyReportedAsMissing.count(key)) {
+    if (checker.getObjCRequirementSibling(
+            fnRequirement, [conformance](AbstractFunctionDecl *candidate) {
+              return static_cast<bool>(conformance->getWitness(candidate));
+            })) {
       skipWitness();
       continue;
     }
 
-    auto sibling = checker.getObjCRequirementSibling(
-        fnRequirement, [conformance](AbstractFunctionDecl *candidate) {
-          return static_cast<bool>(conformance->getWitness(candidate));
-        });
-
-    if (!sibling) {
-      alreadyReportedAsMissing.insert(key);
-      addWitness();
+    // If we have already reported a function with this selector as missing,
+    // don't do it again.
+    if (alreadyReportedAsMissing.count(key)) {
+      skipWitness();
       continue;
     }
 
-    // Otherwise, there is a witness for any of the *other* requirements with
-    // this same selector, so prune it out.
-    skipWitness();
+    // There is really a missing requirement for this witness.
+    alreadyReportedAsMissing.insert(key);
+    addWitness();
   }
 
   if (removedAny) {
@@ -5402,22 +5410,6 @@ void ConformanceChecker::resolveValueWitnesses() {
       continue;
     }
 
-    // If this requirement is part of a pair of imported async requirements,
-    // where one has already been witnessed, we can skip it.
-    //
-    // This situation primarily arises when the ClangImporter translates an
-    // async-looking ObjC protocol method requirement into two Swift protocol
-    // requirements: an async version and a sync version. Exactly one of the two
-    // must be witnessed by the conformer.
-    if (!requirement->isImplicit() && getObjCRequirementSibling(
-            requirement, [this](AbstractFunctionDecl *cand) {
-              return !cand->getAttrs().hasAttribute<OptionalAttr>() &&
-                     !cand->isImplicit() &&
-                     this->Conformance->hasWitness(cand);
-            })) {
-      continue;
-    }
-
     // Try substituting into the requirement's interface type. If we fail,
     // either a generic requirement was not satisfied or we tripped on an
     // invalid type witness, and there's no point in resolving a witness.

test/Serialization/conformance-objc-async.swift

@@ -0,0 +1,28 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+// RUN: %target-swift-emit-module-interface(%t/Conformance.swiftinterface) -module-name Conformance -I %t %t/Conformance.swift
+// RUN: %target-swift-frontend -compile-module-from-interface %t/Conformance.swiftinterface -module-name Conformance -o /dev/null -I %t
+// REQUIRES: objc_interop
+
+//--- module.modulemap
+module ObjCProto {
+  header "objc_proto.h"
+  export *
+}
+
+//--- objc_proto.h
+@protocol Doable
+- (void)doItWithCompletion:(void (^)())completion;
+@end
+
+//--- Conformance.swift
+import ObjCProto
+
+public final class ConformsToDoableWithCompletionHandler: Doable {
+  public func doIt(completion: @escaping () -> Void) {}
+}
+
+@available(SwiftStdlib 5.5, *)
+public final class ConformsToDoableWithAsync:Doable {
+  public func doIt() async {}
+}