AST: Improve handling of missing @objc async protocol witnesses.

Serialization now enumerates the value witnesses of conformances with a
resolver in order to facilitate lazy type checking
(#68262). This change in behavior introduced
an assertion failure when compiling modules from swiftinterface when the
interface contains a conformance to an `@objc` protocol that has a requirement
that is imported with an `async` variant. The `forEachValueWitness()`
invocation during serialization was causing the missing witness for an objc
async protocol requirement to be resolved after conformance check was already
marked "complete". This witness had not been previously resolved because
`ConformanceChecker::resolveValueWitnesses()` had special logic to skip objc
protocol requirements with witnessed siblings, whereas `forEachValueWitness()`
did not.

There are multiple potential solutions to this problem, but the one that seemed
least disruptive to me was to stop skipping resolution of these sibling
witnesses during `ConformanceChecker::resolveValueWitnesses()`. When I looked
into why they were being skipped, I discovered that this seemed to be a
concession to bugs in the logic for pruning missing witnesses. When I adjusted
the pruning logic it no longer became necessary to skip witnesses in
`ConformanceChecker::resolveValueWitnesses()` in order to avoid incorrect
diagnostics.

Resolves rdar://119435253

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 {}
+}