Teach loadAllMembers() implementations to add the members themselves.

The contract for LazyResolver::loadAllMembers() was that the caller
would handle actually adding the members, since it was an iterable
declaration context and could centralize that (simple) logic. However,
this fails in the Clang importer in rare but amusing ways when some of
the deferred actions (e.g., finishing a protocol conformance) depend
on having the members already set. The deferred action occurs after
the member list is complete in ClangImporter's loadAllMembers(), but
before its caller actual set the member list, leaving incomplete
conformances. Fixes rdar://problem/18884272.

Swift SVN r25630

include/swift/AST/LazyResolver.h

@@ -114,13 +114,12 @@ public:
 
   /// Populates the given vector with all member decls for \p D.
   ///
-  /// The implementation should \em not call setMembers on \p D.
+  /// The implementation should add the members to D.
   ///
   /// \param[out] hasMissingRequiredMembers If present, set to true if any
   /// members failed to import and were non-optional protocol requirements.
   virtual void
-  loadAllMembers(const Decl *D, uint64_t contextData,
+  loadAllMembers(Decl *D, uint64_t contextData,
-                 SmallVectorImpl<Decl *> &Members,
                  bool *hasMissingRequiredMembers = nullptr) {
     llvm_unreachable("unimplemented");
   }

include/swift/Serialization/ModuleFile.h

@@ -574,9 +574,8 @@ public:
   /// Has no effect in NDEBUG builds.
   void verify() const;
 
-  virtual void loadAllMembers(const Decl *D,
+  virtual void loadAllMembers(Decl *D,
                               uint64_t contextData,
-                              SmallVectorImpl<Decl *> &Members,
                               bool *ignored) override;
 
   virtual void

lib/AST/DeclContext.cpp

@@ -683,12 +683,8 @@ void IterableDeclContext::loadAllMembers() const {
   }
 
   bool hasMissingRequiredMembers = false;
-  SmallVector<Decl *, 16> Members;
+  resolver->loadAllMembers(const_cast< Decl *>(container), contextData,
-  resolver->loadAllMembers(container, contextData, Members,
                            &hasMissingRequiredMembers);
-  for (auto member : Members) {
-    const_cast<IterableDeclContext *>(this)->addMember(member);
-  }
 
   if (hasMissingRequiredMembers)
     if (auto proto = dyn_cast<ProtocolDecl>(this))

lib/ClangImporter/ImportDecl.cpp

@@ -5717,31 +5717,35 @@ createUnavailableDecl(Identifier name, DeclContext *dc, Type type,
 
 
 void
-ClangImporter::Implementation::loadAllMembers(const Decl *D, uint64_t unused,
+ClangImporter::Implementation::loadAllMembers(Decl *D, uint64_t unused,
-                                              SmallVectorImpl<Decl *> &members,
                                               bool *hasMissingRequiredMembers) {
   assert(D->hasClangNode());
   auto clangDecl = cast<clang::ObjCContainerDecl>(D->getClangDecl());
 
   SwiftDeclConverter converter(*this);
 
-  const DeclContext *DC;
+  DeclContext *DC;
+  IterableDeclContext *IDC;
   ArrayRef<ProtocolDecl *> protos;
 
   // Figure out the declaration context we're importing into.
   if (auto nominal = dyn_cast<NominalTypeDecl>(D)) {
     DC = nominal;
+    IDC = nominal;
   } else {
-    DC = cast<ExtensionDecl>(D);
+    auto ext = cast<ExtensionDecl>(D);
+    DC = ext;
+    IDC = ext;
   }
 
   ImportingEntityRAII Importing(*this);
 
+  SmallVector<Decl *, 16> members;
   bool scratch;
   if (!hasMissingRequiredMembers)
     hasMissingRequiredMembers = &scratch;
   *hasMissingRequiredMembers = false;
-  converter.importObjCMembers(clangDecl, const_cast<DeclContext *>(DC),
+  converter.importObjCMembers(clangDecl, DC,
                               members, *hasMissingRequiredMembers);
 
   if (auto clangClass = dyn_cast<clang::ObjCInterfaceDecl>(clangDecl)) {
@@ -5766,9 +5770,15 @@ ClangImporter::Implementation::loadAllMembers(const Decl *D, uint64_t unused,
   // Import mirrored declarations for protocols to which this category
   // or extension conforms.
   // FIXME: This is supposed to be a short-term hack.
-  converter.importMirroredProtocolMembers(clangDecl,
+  converter.importMirroredProtocolMembers(clangDecl, DC,
-                                          const_cast<DeclContext *>(DC),
                                           protos, members, SwiftContext);
+
+  // Add the members now, before ~ImportingEntityRAII does work that might
+  // involve them.
+  for (auto member : members) {
+    IDC->addMember(member);
+  }
+
 }
 
 Optional<MappedTypeNameKind>

lib/ClangImporter/ImporterImpl.h

@@ -955,8 +955,7 @@ public:
   }
 
   virtual void
-  loadAllMembers(const Decl *D, uint64_t unused,
+  loadAllMembers(Decl *D, uint64_t unused,
-                 SmallVectorImpl<Decl *> &Members,
                  bool *hasMissingRequiredMembers) override;
 
   template <typename DeclTy, typename ...Targs>

lib/Serialization/Deserialization.cpp

@@ -595,7 +595,7 @@ ModuleFile::maybeReadConformance(Type conformingType,
       witness = ConcreteDeclRef(ctx, second, substitutions);
 
     witnesses.insert(std::make_pair(first, witness));
-    if (second)
+    if (second && second != first)
       ctx.recordConformingDecl(second, first);
   }
   assert(rawIDIter <= rawIDs.end() && "read too much");
@@ -3782,16 +3782,25 @@ Type ModuleFile::getType(TypeID TID) {
   return typeOrOffset;
 }
 
-void ModuleFile::loadAllMembers(const Decl *D,
+void ModuleFile::loadAllMembers(Decl *D,
                                 uint64_t contextData,
-                                SmallVectorImpl<Decl *> &Members,
                                 bool *) {
   // FIXME: Add PrettyStackTrace.
   BCOffsetRAII restoreOffset(DeclTypeCursor);
   DeclTypeCursor.JumpToBit(contextData);
-  bool Err = readMembers(Members);
+  SmallVector<Decl *, 16> members;
+  bool Err = readMembers(members);
   assert(!Err && "unable to read members");
   (void)Err;
+
+  IterableDeclContext *IDC;
+  if (auto nominal = dyn_cast<NominalTypeDecl>(D))
+    IDC = nominal;
+  else
+    IDC = cast<ExtensionDecl>(D);
+
+  for (auto member : members)
+    IDC->addMember(member);
 }
 
 void

test/Interpreter/SDK/Foundation_test.swift

@@ -6,6 +6,12 @@
 import Foundation
 import StdlibUnittest
 
+// rdar://problem/18884272
+// Make sure that NSObject conforms to NSObjectProtocol. This
+// particular bug is ridiculously hard to trigger without a complete
+// SDK, so it sits here.
+let objcProtocol: NSObjectProtocol = NSObject()
+
 var FoundationTestSuite = TestSuite("Foundation")
 
 func asNSString(s: String) -> NSString { return s as NSString }