Teach Named Lazy Member Loading To Import Inherited Constructors

lib/AST/NameLookup.cpp

@@ -1274,13 +1274,6 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
   bool includeAttrImplements =
       flags.contains(LookupDirectFlags::IncludeAttrImplements);
 
-  // FIXME: At present, lazy member is not able to find inherited constructors
-  // in imported classes, because SwiftDeclConverter::importInheritedConstructors()
-  // is only called via ClangImporter::Implementation::loadAllMembers().
-  if (hasClangNode() &&
-      name.getBaseName() == DeclBaseName::createConstructor())
-    useNamedLazyMemberLoading = false;
-
   LLVM_DEBUG(llvm::dbgs() << getNameStr() << ".lookupDirect("
              << name << ")"
         << ", isLookupTablePopulated()=" << isLookupTablePopulated()

lib/ClangImporter/ClangImporter.cpp

@@ -3822,6 +3822,16 @@ ClangImporter::Implementation::loadNamedMembers(
       }
     }
   }
+
+  if (N == DeclBaseName::createConstructor()) {
+    if (auto *classDecl = dyn_cast<ClassDecl>(D)) {
+      SmallVector<Decl *, 4> ctors;
+      importInheritedConstructors(cast<clang::ObjCInterfaceDecl>(CD),
+                                  classDecl, ctors);
+      for (auto ctor : ctors)
+        Members.push_back(cast<ValueDecl>(ctor));
+    }
+  }
   return Members;
 }
 

lib/ClangImporter/DWARFImporter.cpp

@@ -128,6 +128,17 @@ ModuleDecl *ClangImporter::Implementation::loadModuleDWARF(
   return decl;
 }
 
+// This function exists to defeat the lazy member importing mechanism. The
+// DWARFImporter is not capable of loading individual members, so it cannot
+// benefit from this optimization yet anyhow. Besides, if you're importing a
+// type here, you more than likely want to dump it and its fields. Loading all
+// members populates lookup tables in the Clang Importer and ensures the
+// absence of cache-fill-related side effects.
+static void forceLoadAllMembers(IterableDeclContext *IDC) {
+  if (!IDC) return;
+  IDC->loadAllMembers();
+}
+
 void ClangImporter::Implementation::lookupValueDWARF(
     DeclName name, NLKind lookupKind, Identifier inModule,
     SmallVectorImpl<ValueDecl *> &results) {
@@ -150,8 +161,10 @@ void ClangImporter::Implementation::lookupValueDWARF(
       continue;
 
     if (swiftDecl->getFullName().matchesRef(name) &&
-        swiftDecl->getDeclContext()->isModuleScopeContext())
+        swiftDecl->getDeclContext()->isModuleScopeContext()) {
+      forceLoadAllMembers(dyn_cast<IterableDeclContext>(swiftDecl));
       results.push_back(swiftDecl);
+    }
   }
 }
 
@@ -174,8 +187,10 @@ void ClangImporter::Implementation::lookupTypeDeclDWARF(
     Decl *importedDecl = cast_or_null<ValueDecl>(
         importDeclReal(namedDecl->getMostRecentDecl(), CurrentVersion));
 
-    if (auto *importedType = dyn_cast_or_null<TypeDecl>(importedDecl))
+    if (auto *importedType = dyn_cast_or_null<TypeDecl>(importedDecl)) {
+      forceLoadAllMembers(dyn_cast<IterableDeclContext>(importedType));
       receiver(importedType);
+    }
   }
 }
 

lib/ClangImporter/ImportDecl.cpp

@@ -104,7 +104,7 @@ getDefaultMakeStructRawValuedOptions() {
   return opts;
 }
 
-static bool isInSystemModule(DeclContext *D) {
+static bool isInSystemModule(const DeclContext *D) {
   return cast<ClangModuleUnit>(D->getModuleScopeContext())->isSystemModule();
 }
 
@@ -4091,7 +4091,7 @@ namespace {
     /// Check whether we have already imported a method with the given
     /// selector in the given context.
     bool isMethodAlreadyImported(ObjCSelector selector, bool isInstance,
-                                 DeclContext *dc,
+                                 const DeclContext *dc,
                     llvm::function_ref<bool(AbstractFunctionDecl *fn)> filter) {
       // We only need to perform this check for classes.
       auto classDecl
@@ -4425,7 +4425,7 @@ namespace {
     /// NSArray(capacity: 1024)
     /// \endcode
     ConstructorDecl *importConstructor(const clang::ObjCMethodDecl *objcMethod,
-                                       DeclContext *dc,
+                                       const DeclContext *dc,
                                        bool implicit,
                                        Optional<CtorInitializerKind> kind,
                                        bool required);
@@ -4454,7 +4454,7 @@ namespace {
     /// This variant of the function is responsible for actually binding the
     /// constructor declaration appropriately.
     ConstructorDecl *importConstructor(const clang::ObjCMethodDecl *objcMethod,
-                                       DeclContext *dc,
+                                       const DeclContext *dc,
                                        bool implicit,
                                        CtorInitializerKind kind,
                                        bool required,
@@ -4518,16 +4518,15 @@ namespace {
     void importMirroredProtocolMembers(const clang::ObjCContainerDecl *decl,
                                        DeclContext *dc,
                                        ArrayRef<ProtocolDecl *> protocols,
-                                       SmallVectorImpl<Decl *> &members,
-                                       ASTContext &Ctx);
+                                       SmallVectorImpl<Decl *> &members);
 
     void importNonOverriddenMirroredMethods(DeclContext *dc,
                                   MutableArrayRef<MirroredMethodEntry> entries,
                                   SmallVectorImpl<Decl *> &newMembers);
 
     /// Import constructors from our superclasses (and their
     /// categories/extensions), effectively "inheriting" constructors.
-    void importInheritedConstructors(ClassDecl *classDecl,
+    void importInheritedConstructors(const ClassDecl *classDecl,
                                      SmallVectorImpl<Decl *> &newMembers);
 
     Decl *VisitObjCCategoryDecl(const clang::ObjCCategoryDecl *decl) {
@@ -6083,7 +6082,7 @@ SwiftDeclConverter::getImplicitProperty(ImportedName importedName,
 }
 
 ConstructorDecl *SwiftDeclConverter::importConstructor(
-    const clang::ObjCMethodDecl *objcMethod, DeclContext *dc, bool implicit,
+    const clang::ObjCMethodDecl *objcMethod, const DeclContext *dc, bool implicit,
     Optional<CtorInitializerKind> kind, bool required) {
   // Only methods in the 'init' family can become constructors.
   assert(isInitMethod(objcMethod) && "Not a real init method");
@@ -6234,7 +6233,7 @@ bool SwiftDeclConverter::existingConstructorIsWorse(
 /// This variant of the function is responsible for actually binding the
 /// constructor declaration appropriately.
 ConstructorDecl *SwiftDeclConverter::importConstructor(
-    const clang::ObjCMethodDecl *objcMethod, DeclContext *dc, bool implicit,
+    const clang::ObjCMethodDecl *objcMethod, const DeclContext *dc, bool implicit,
     CtorInitializerKind kind, bool required, ObjCSelector selector,
     ImportedName importedName, ArrayRef<const clang::ParmVarDecl *> args,
     bool variadic, bool &redundant) {
@@ -6354,7 +6353,7 @@ ConstructorDecl *SwiftDeclConverter::importConstructor(
       /*NameLoc=*/SourceLoc(), failability, /*FailabilityLoc=*/SourceLoc(),
       /*Throws=*/importedName.getErrorInfo().hasValue(),
       /*ThrowsLoc=*/SourceLoc(), bodyParams,
-      /*GenericParams=*/nullptr, dc);
+      /*GenericParams=*/nullptr, const_cast<DeclContext *>(dc));
 
   addObjCAttribute(result, selector);
 
@@ -6941,8 +6940,7 @@ Optional<GenericParamList *> SwiftDeclConverter::importObjCGenericParams(
 
 void SwiftDeclConverter::importMirroredProtocolMembers(
     const clang::ObjCContainerDecl *decl, DeclContext *dc,
-    ArrayRef<ProtocolDecl *> protocols, SmallVectorImpl<Decl *> &members,
-    ASTContext &Ctx) {
+    ArrayRef<ProtocolDecl *> protocols, SmallVectorImpl<Decl *> &members) {
   assert(dc);
   const clang::ObjCInterfaceDecl *interfaceDecl = nullptr;
   const ClangModuleUnit *declModule;
@@ -7189,7 +7187,7 @@ void SwiftDeclConverter::importNonOverriddenMirroredMethods(DeclContext *dc,
 }
 
 void SwiftDeclConverter::importInheritedConstructors(
-    ClassDecl *classDecl, SmallVectorImpl<Decl *> &newMembers) {
+    const ClassDecl *classDecl, SmallVectorImpl<Decl *> &newMembers) {
   if (!classDecl->hasSuperclass())
     return;
 
@@ -8673,6 +8671,15 @@ void ClangImporter::Implementation::insertMembersAndAlternates(
   });
 }
 
+void ClangImporter::Implementation::importInheritedConstructors(
+     const clang::ObjCInterfaceDecl *curObjCClass,
+     const ClassDecl *classDecl, SmallVectorImpl<Decl *> &newMembers) {
+   if (curObjCClass->getName() != "Protocol") {
+     SwiftDeclConverter converter(*this, CurrentVersion);
+     converter.importInheritedConstructors(classDecl, newMembers);
+   }
+ }
+
 void ClangImporter::Implementation::collectMembersToAdd(
     const clang::ObjCContainerDecl *objcContainer, Decl *D, DeclContext *DC,
     SmallVectorImpl<Decl *> &members) {
@@ -8687,24 +8694,16 @@ void ClangImporter::Implementation::collectMembersToAdd(
 
   auto protos = getImportedProtocols(D);
   if (auto clangClass = dyn_cast<clang::ObjCInterfaceDecl>(objcContainer)) {
-    auto swiftClass = cast<ClassDecl>(D);
     objcContainer = clangClass = clangClass->getDefinition();
-
-    // Imported inherited initializers.
-    if (clangClass->getName() != "Protocol") {
-      converter.importInheritedConstructors(const_cast<ClassDecl *>(swiftClass),
-                                            members);
-    }
-
+    importInheritedConstructors(clangClass, cast<ClassDecl>(D), members);
   } else if (auto clangProto
                = dyn_cast<clang::ObjCProtocolDecl>(objcContainer)) {
     objcContainer = clangProto->getDefinition();
   }
   // Import mirrored declarations for protocols to which this category
   // or extension conforms.
   // FIXME: This is supposed to be a short-term hack.
-  converter.importMirroredProtocolMembers(objcContainer, DC,
-                                          protos, members, SwiftContext);
+  converter.importMirroredProtocolMembers(objcContainer, DC, protos, members);
 }
 
 void ClangImporter::Implementation::loadAllConformances(

lib/ClangImporter/ImporterImpl.h

@@ -492,7 +492,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   /// Keep track of initializer declarations that correspond to
   /// imported methods.
   llvm::DenseMap<
-      std::tuple<const clang::ObjCMethodDecl *, DeclContext *, Version>,
+      std::tuple<const clang::ObjCMethodDecl *, const DeclContext *, Version>,
       ConstructorDecl *> Constructors;
 
   /// Keep track of all initializers that have been imported into a
@@ -812,6 +812,10 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   Decl *importMirroredDecl(const clang::NamedDecl *decl, DeclContext *dc,
                            Version version, ProtocolDecl *proto);
 
+  void importInheritedConstructors(const clang::ObjCInterfaceDecl *curObjCClass,
+                                   const ClassDecl *classDecl,
+                                   SmallVectorImpl<Decl *> &newMembers);
+
   /// Utility function for building simple generic signatures.
   GenericSignature buildGenericSignature(GenericParamList *genericParams,
                                           DeclContext *dc);

test/ClangImporter/attr-swift_name.swift

@@ -1,5 +1,5 @@
 // RUN: %empty-directory(%t.mcp)
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -Xcc -w -typecheck %s -module-cache-path %t.mcp 2>&1 | %FileCheck %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -Xcc -w -typecheck %s -module-cache-path %t.mcp -disable-named-lazy-member-loading 2>&1 | %FileCheck %s
 
 // REQUIRES: objc_interop
 

test/NameBinding/Inputs/NamedLazyMembers/NamedLazyMembers.h

@@ -26,6 +26,8 @@
 // Don't conform to the protocol; that loads all protocol members.
 @interface SimpleDoer
 
+- (instancetype)initWithValue: (int)value;
+
 // These are names we're hoping don't interfere with Doer, above.
 + (SimpleDoer*)Doer;
 + (SimpleDoer*)DoerOfNoWork;
@@ -107,4 +109,14 @@
 @interface SimpleDoerSubclass : SimpleDoer
 - (void)simplyDoSomeWorkWithSpeed:(int)s thoroughness:(int)t
   NS_SWIFT_NAME(simplyDoVeryImportantWork(speed:thoroughness:));
+
+- (void)exuberantlyGoForWalk;
+- (void)exuberantlyTakeNap;
+- (void)exuberantlyEatMeal;
+- (void)exuberantlyTidyHome;
+- (void)exuberantlyCallFamily;
+- (void)exuberantlySingSong;
+- (void)exuberantlyReadBook;
+- (void)exuberantlyAttendLecture;
+- (void)exuberantlyWriteLetter;
 @end

test/NameBinding/named_lazy_member_loading_objc_interface.swift

@@ -8,17 +8,23 @@
 // Check that named-lazy-member-loading reduces the number of Decls deserialized
 // RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -disable-named-lazy-member-loading -stats-output-dir %t/stats-pre -primary-file %s %S/Inputs/NamedLazyMembers/NamedLazyMembersExt.swift
 // RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -stats-output-dir %t/stats-post -primary-file %s %S/Inputs/NamedLazyMembers/NamedLazyMembersExt.swift
-// RUN: %{python} %utils/process-stats-dir.py --evaluate-delta 'NumTotalClangImportedEntities <= -1' %t/stats-pre %t/stats-post
+// RUN: %{python} %utils/process-stats-dir.py --evaluate-delta 'NumTotalClangImportedEntities <= -10' %t/stats-pre %t/stats-post
 
 import NamedLazyMembers
 
-public func bar(d: SimpleDoerSubclass) {
-    let _ = d.simplyDoVeryImportantWork(speed: 10, motivation: 42)
+public func bar() {
+  let d = SimpleDoerSubclass(value: 123)!
+  let _ = d.simplyDoVeryImportantWork(speed: 10, motivation: 42)
 }
 
-public func foo(d: SimpleDoer) {
+public func foo() {
+  let d = SimpleDoer(value: 123)!
   let _ = d.simplyDoSomeWork()
   let _ = d.simplyDoSomeWork(withSpeed:10)
   let _ = d.simplyDoVeryImportantWork(speed:10, thoroughness:12)
   let _ = d.simplyDoSomeWorkWithSpeed(speed:10, levelOfAlacrity:12)
 }
+
+// Make sure that simply subclassing an imported subclass doesn't page in all
+// members.
+class MostDoerSubclass : SimpleDoerSubclass {}