[ClangImporter] Make conformance loading lazier. (#10737)

Previously, the importer queued up conformances to complete once it
was done importing the current batch of declarations. However, if
there was a serialized Swift module that extended an imported type to
add a conformance in exactly the wrong way, the importer could end up
asking for that conformance later---even before the reference to the
imported type was resolved. This led to a crash in the deserializer
"while reading conformance for type X".

Instead of this "pending actions" queue, we can just use the
mechanisms already in place for lazily loading conformances. That way
they'll get filled out on demand, which is better all around anyway.
This does mean putting the requirement signature into the "lazy" part
of the conformance, though.

This does as a side effect mean that /all/ of the witnesses for the
imported conformance may be opaque---that is, they will never be
devirtualized to a particular implementation. However, they previously
would have referred to methods implemented in Objective-C anyway,
which are always dispatched with objc_msgSend. So this should have no
practical effect.

rdar://problem/32346184
(cherry picked from commit a8bc132)

Author: jrose-apple

include/swift/AST/ProtocolConformance.h

@@ -462,6 +462,8 @@ class NormalProtocolConformance : public ProtocolConformance,
   /// protocol, which line up with the conformance constraints in the
   /// protocol's requirement signature.
   ArrayRef<ProtocolConformanceRef> getSignatureConformances() const {
+    if (Resolver)
+      resolveLazyInfo();
     return SignatureConformances;
   }
 

lib/ClangImporter/ImportDecl.cpp

@@ -6348,7 +6348,8 @@ void SwiftDeclConverter::addObjCProtocolConformances(
     auto conformance = ctx.getConformance(dc->getDeclaredTypeInContext(),
                                           protocols[i], SourceLoc(), dc,
                                           ProtocolConformanceState::Incomplete);
-    Impl.scheduleFinishProtocolConformance(conformance);
+    conformance->setLazyLoader(&Impl, /*context*/0);
+    conformance->setState(ProtocolConformanceState::Complete);
     conformances.push_back(conformance);
   }
 
@@ -7286,6 +7287,9 @@ void ClangImporter::Implementation::finishedImportingEntity() {
 
 void ClangImporter::Implementation::finishPendingActions() {
   while (true) {
+    // The odd shape of this loop comes from previously having more than one
+    // possible kind of pending action. It's left this way to make it easy to
+    // add another one back in an `else if` clause.
     if (!RegisteredExternalDecls.empty()) {
       if (hasFinishedTypeChecking()) {
         RegisteredExternalDecls.clear();
@@ -7297,22 +7301,22 @@ void ClangImporter::Implementation::finishPendingActions() {
             if (!nominal->hasDelayedMembers())
               typeResolver->resolveExternalDeclImplicitMembers(nominal);
       }
-    } else if (!DelayedProtocolConformances.empty()) {
-      NormalProtocolConformance *conformance =
-          DelayedProtocolConformances.pop_back_val();
-      finishProtocolConformance(conformance);
     } else {
       break;
     }
   }
 }
 
-/// Finish the given protocol conformance (for an imported type)
-/// by filling in any missing witnesses.
-void ClangImporter::Implementation::finishProtocolConformance(
-    NormalProtocolConformance *conformance) {
+void ClangImporter::Implementation::finishNormalConformance(
+    NormalProtocolConformance *conformance,
+    uint64_t unused) {
+  (void)unused;
   const ProtocolDecl *proto = conformance->getProtocol();
 
+  PrettyStackTraceType trace(SwiftContext, "completing conformance for",
+                             conformance->getType());
+  PrettyStackTraceDecl traceTo("... to", proto);
+
   // Create witnesses for requirements not already met.
   for (auto req : proto->getMembers()) {
     auto valueReq = dyn_cast<ValueDecl>(req);

lib/ClangImporter/ImporterImpl.h

@@ -496,9 +496,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   /// External Decls that we have imported but not passed to the ASTContext yet.
   SmallVector<Decl *, 4> RegisteredExternalDecls;
 
-  /// Protocol conformances that may be missing witnesses.
-  SmallVector<NormalProtocolConformance *, 4> DelayedProtocolConformances;
-
   unsigned NumCurrentImportingEntities = 0;
 
   /// Mapping from delayed conformance IDs to the set of delayed
@@ -517,7 +514,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   void startedImportingEntity();
   void finishedImportingEntity();
   void finishPendingActions();
-  void finishProtocolConformance(NormalProtocolConformance *conformance);
 
   struct ImportingEntityRAII {
     Implementation &Impl;
@@ -567,10 +563,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
     RegisteredExternalDecls.push_back(D);
   }
 
-  void scheduleFinishProtocolConformance(NormalProtocolConformance *C) {
-    DelayedProtocolConformances.push_back(C);
-  }
-
   /// \brief Retrieve the Clang AST context.
   clang::ASTContext &getClangASTContext() const {
     return Instance->getASTContext();
@@ -1115,6 +1107,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
     const Decl *D, uint64_t contextData,
     SmallVectorImpl<ProtocolConformance *> &Conformances) override;
 
+  void finishNormalConformance(NormalProtocolConformance *conformance,
+                               uint64_t unused) override;
+
   template <typename DeclTy, typename ...Targs>
   DeclTy *createDeclWithClangNode(ClangNode ClangN, Accessibility access,
                                   Targs &&... Args) {

test/ClangImporter/Inputs/SwiftPrivateAttr.txt

@@ -107,7 +107,7 @@ struct NSOptions : OptionSet {
   static var __PrivA: NSOptions { get }
   static var B: NSOptions { get }
 }
-class __PrivCFType : _CFObject {
+class __PrivCFType {
 }
 @available(swift, obsoleted: 3, renamed: "__PrivCFType")
 typealias __PrivCFTypeRef = __PrivCFType

test/ClangImporter/Inputs/custom-modules/Protocols.h

@@ -9,3 +9,12 @@ Class <FooProto> _Nonnull processFooType(Class <FooProto> _Nonnull);
 Class <FooProto, AnotherProto> _Nonnull processComboType(Class <FooProto, AnotherProto> _Nonnull);
 Class <AnotherProto, FooProto> _Nonnull processComboType2(Class <AnotherProto, FooProto> _Nonnull);
 
+
+@protocol SubProto <FooProto>
+@end
+
+@interface ProtocolTestingBase
+@end
+
+@interface SubProtoImpl: ProtocolTestingBase <SubProto>
+@end

test/ClangImporter/protocol-conformance-in-extension.swift

@@ -0,0 +1,21 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -o %t/a~partial.swiftmodule -I %S/Inputs/custom-modules -module-name TEST -primary-file %s
+// RUN: %target-swift-frontend -emit-module -o %t/test.swiftmodule -I %S/Inputs/custom-modules -module-name TEST %t/a~partial.swiftmodule
+
+// REQUIRES: objc_interop
+
+import TestProtocols
+
+// The protocol in the extension has to refine something that the base class
+// conforms to to trigger the error in rdar://problem/32346184.
+protocol SomeSwiftProto: Equatable {}
+extension ProtocolTestingBase: Equatable {
+  public static func ==(left: ProtocolTestingBase, right: ProtocolTestingBase) -> Bool {
+    return left === right
+  }
+}
+
+// The extension going through the typealias also makes a difference.
+typealias SpecialObject = SubProtoImpl
+extension SpecialObject: SomeSwiftProto {
+}