Finish and default-enable named lazy member loading

include/swift/AST/DeclContext.h

@@ -610,10 +610,15 @@ class IterableDeclContext {
   mutable llvm::PointerIntPair<Decl *, 2, IterableDeclContextKind> 
     LastDeclAndKind;
 
-  // The DeclID this IDC was deserialized from, if any. Used for named lazy
-  // member loading, as a key when doing lookup in this IDC.
+  /// The DeclID this IDC was deserialized from, if any. Used for named lazy
+  /// member loading, as a key when doing lookup in this IDC.
   serialization::DeclID SerialID;
 
+  /// Lazy member loading has a variety of feedback loops that need to
+  /// switch to pseudo-empty-member behaviour to avoid infinite recursion;
+  /// we use this flag to control them.
+  bool lazyMemberLoadingInProgress = false;
+
   template<class A, class B, class C>
   friend struct ::llvm::cast_convert_val;
 
@@ -643,6 +648,14 @@ class IterableDeclContext {
     return FirstDeclAndLazyMembers.getInt();
   }
 
+  bool isLoadingLazyMembers() {
+    return lazyMemberLoadingInProgress;
+  }
+
+  void setLoadingLazyMembers(bool inProgress) {
+    lazyMemberLoadingInProgress = inProgress;
+  }
+
   /// Setup the loader for lazily-loaded members.
   void setMemberLoader(LazyMemberLoader *loader, uint64_t contextData);
 

include/swift/Basic/LangOptions.h

@@ -154,7 +154,7 @@ namespace swift {
     bool IterativeTypeChecker = false;
 
     /// \brief Enable named lazy member loading.
-    bool NamedLazyMemberLoading = false;
+    bool NamedLazyMemberLoading = true;
 
     /// Debug the generic signatures computed by the generic signature builder.
     bool DebugGenericSignatures = false;

include/swift/Option/FrontendOptions.td

@@ -155,8 +155,8 @@ def propagate_constraints : Flag<["-"], "propagate-constraints">,
 def iterative_type_checker : Flag<["-"], "iterative-type-checker">,
   HelpText<"Enable the iterative type checker">;
 
-def enable_named_lazy_member_loading : Flag<["-"], "enable-named-lazy-member-loading">,
-  HelpText<"Enable per-name lazy member loading">;
+def disable_named_lazy_member_loading : Flag<["-"], "disable-named-lazy-member-loading">,
+  HelpText<"Disable per-name lazy member loading">;
 
 def debug_generic_signatures : Flag<["-"], "debug-generic-signatures">,
   HelpText<"Debug generic signatures">;

lib/AST/NameLookup.cpp

@@ -1024,15 +1024,6 @@ class swift::MemberLookupTable {
     return Lookup.find(name);
   }
 
-  /// \brief Add an empty entry to the lookup map for a given name if it does
-  /// not yet have one.
-  void addEmptyEntry(DeclName name) {
-    (void)Lookup[name];
-    if (!name.isSimpleName()) {
-      (void)Lookup[name.getBaseName()];
-    }
-  }
-
   // \brief Mark all Decls in this table as not-resident in a table, drop
   // references to them. Should only be called when this was not fully-populated
   // from an IterableDeclContext.
@@ -1289,13 +1280,14 @@ populateLookupTableEntryFromLazyIDCLoader(ASTContext &ctx,
                                           MemberLookupTable &LookupTable,
                                           DeclName name,
                                           IterableDeclContext *IDC) {
+  if (IDC->isLoadingLazyMembers()) {
+    return false;
+  }
+  IDC->setLoadingLazyMembers(true);
   auto ci = ctx.getOrCreateLazyIterableContextData(IDC,
                                                    /*lazyLoader=*/nullptr);
-  // Populate LookupTable with an empty vector before we call into our loader,
-  // so that any reentry of this routine will find the set-so-far, and not
-  // fall into infinite recursion.
-  LookupTable.addEmptyEntry(name);
   if (auto res = ci->loader->loadNamedMembers(IDC, name, ci->memberData)) {
+    IDC->setLoadingLazyMembers(false);
     if (auto s = ctx.Stats) {
       ++s->getFrontendCounters().NamedLazyMemberLoadSuccessCount;
     }
@@ -1304,6 +1296,7 @@ populateLookupTableEntryFromLazyIDCLoader(ASTContext &ctx,
     }
     return false;
   } else {
+    IDC->setLoadingLazyMembers(false);
     if (auto s = ctx.Stats) {
       ++s->getFrontendCounters().NamedLazyMemberLoadFailureCount;
     }
@@ -1346,6 +1339,13 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
   bool useNamedLazyMemberLoading = (ctx.LangOpts.NamedLazyMemberLoading &&
                                     hasLazyMembers());
 
+  // FIXME: At present, lazy member loading conflicts with a bunch of other code
+  // that appears to special-case initializers (clang-imported initializer
+  // sorting, implicit initializer synthesis), so for the time being we have to
+  // turn it off for them entirely.
+  if (name.getBaseName() == ctx.Id_init)
+    useNamedLazyMemberLoading = false;
+
   // We check the LookupTable at most twice, possibly treating a miss in the
   // first try as a cache-miss that we then do a cache-fill on, and retry.
   for (int i = 0; i < 2; ++i) {
@@ -1355,10 +1355,11 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
     // will flip the hasLazyMembers() flag to false as well.
     if (!useNamedLazyMemberLoading) {
       // If we're about to load members here, purge the MemberLookupTable;
-      // it will be rebuilt in prepareLookup, below.
-      if (hasLazyMembers() && LookupTable.getPointer()) {
-        // We should not have scanned the IDC list yet. Double check.
-        assert(!LookupTable.getInt());
+      // it will be rebuilt in prepareLookup, below. Base this decision on
+      // the LookupTable's int value, not hasLazyMembers(), since the latter
+      // can sometimes change underfoot if some other party calls getMembers
+      // outside of lookup (eg. typo correction).
+      if (LookupTable.getPointer() && !LookupTable.getInt()) {
         LookupTable.getPointer()->clear();
       }
 
@@ -1400,7 +1401,7 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
     } else {
       if (!ignoreNewExtensions) {
         for (auto E : getExtensions()) {
-          if (E->wasDeserialized()) {
+          if (E->wasDeserialized() || E->hasClangNode()) {
             if (populateLookupTableEntryFromLazyIDCLoader(ctx, Table,
                                                           name, E)) {
               useNamedLazyMemberLoading = false;

lib/ClangImporter/ClangImporter.cpp

@@ -3278,8 +3278,23 @@ ClangImporter::Implementation::loadNamedMembers(
   auto *D = IDC->getDecl();
   auto *DC = cast<DeclContext>(D);
   auto *CD = D->getClangDecl();
+  auto *CDC = cast<clang::DeclContext>(CD);
   assert(CD && "loadNamedMembers on a Decl without a clangDecl");
 
+
+  // FIXME: The legacy of mirroring protocol members rears its ugly head,
+  // and as a result we have to bail on any @interface or @category that
+  // has a declared protocol conformance.
+  if (auto *ID = dyn_cast<clang::ObjCInterfaceDecl>(CD)) {
+    if (ID->protocol_begin() != ID->protocol_end())
+      return None;
+  }
+  if (auto *CCD = dyn_cast<clang::ObjCCategoryDecl>(CD)) {
+    if (CCD->protocol_begin() != CCD->protocol_end())
+      return None;
+  }
+
+
   // There are 3 cases:
   //
   //  - The decl is from a bridging header, CMO is Some(nullptr)
@@ -3303,29 +3318,32 @@ ClangImporter::Implementation::loadNamedMembers(
 
   clang::ASTContext &clangCtx = getClangASTContext();
 
+  assert(isa<clang::ObjCContainerDecl>(CD));
+
   TinyPtrVector<ValueDecl *> Members;
-  if (auto *CCD = dyn_cast<clang::ObjCContainerDecl>(CD)) {
-    for (auto entry : table->lookup(SerializedSwiftName(N.getBaseName()), CCD)) {
-      if (!entry.is<clang::NamedDecl *>()) continue;
-      auto member = entry.get<clang::NamedDecl *>();
-      if (!isVisibleClangEntry(clangCtx, member)) continue;
-      SmallVector<Decl*, 4> tmp;
-      insertMembersAndAlternates(member, tmp);
-      for (auto *TD : tmp) {
-        if (auto *V = dyn_cast<ValueDecl>(TD)) {
-          // Skip ValueDecls if they import into different DeclContexts
-          // or under different names than the one we asked about.
-          if (V->getDeclContext() == DC &&
-              V->getFullName().matchesRef(N)) {
-            Members.push_back(V);
-          }
+  auto *Nominal = DC->getAsNominalTypeOrNominalTypeExtensionContext();
+  auto ClangContext = getEffectiveClangContext(Nominal);
+  for (auto entry : table->lookup(SerializedSwiftName(N.getBaseName()),
+                                  ClangContext)) {
+    if (!entry.is<clang::NamedDecl *>()) continue;
+    auto member = entry.get<clang::NamedDecl *>();
+    if (!isVisibleClangEntry(clangCtx, member)) continue;
+
+    // Skip Decls from different clang::DeclContexts
+    if (member->getDeclContext() != CDC) continue;
+
+    SmallVector<Decl*, 4> tmp;
+    insertMembersAndAlternates(member, tmp);
+    for (auto *TD : tmp) {
+      if (auto *V = dyn_cast<ValueDecl>(TD)) {
+        // Skip ValueDecls if they import under different names.
+        if (V->getFullName().matchesRef(N)) {
+          Members.push_back(V);
         }
       }
     }
-    return Members;
   }
-
-  return None;
+  return Members;
 }
 
 

lib/ClangImporter/ImportDecl.cpp

@@ -4970,6 +4970,7 @@ SwiftDeclConverter::importCFClassType(const clang::TypedefNameDecl *decl,
     }
   }
 
+  theClass->addImplicitDestructor();
   return theClass;
 }
 

lib/Frontend/CompilerInvocation.cpp

@@ -849,7 +849,7 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   Opts.DebugConstraintSolver |= Args.hasArg(OPT_debug_constraints);
   Opts.EnableConstraintPropagation |= Args.hasArg(OPT_propagate_constraints);
   Opts.IterativeTypeChecker |= Args.hasArg(OPT_iterative_type_checker);
-  Opts.NamedLazyMemberLoading |= Args.hasArg(OPT_enable_named_lazy_member_loading);
+  Opts.NamedLazyMemberLoading &= !Args.hasArg(OPT_disable_named_lazy_member_loading);
   Opts.DebugGenericSignatures |= Args.hasArg(OPT_debug_generic_signatures);
 
   Opts.DebuggerSupport |= Args.hasArg(OPT_debugger_support);

lib/Serialization/ModuleFile.cpp

@@ -19,6 +19,7 @@
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/ModuleLoader.h"
 #include "swift/AST/NameLookup.h"
+#include "swift/AST/PrettyStackTrace.h"
 #include "swift/AST/USRGeneration.h"
 #include "swift/Basic/Range.h"
 #include "swift/ClangImporter/ClangImporter.h"
@@ -1794,15 +1795,15 @@ void ModuleFile::loadObjCMethods(
 Optional<TinyPtrVector<ValueDecl *>>
 ModuleFile::loadNamedMembers(const IterableDeclContext *IDC, DeclName N,
                              uint64_t contextData) {
+  PrettyStackTraceDecl trace("loading members for", IDC->getDecl());
 
   assert(IDC->wasDeserialized());
+  assert(DeclMemberNames);
 
-  if (!DeclMemberNames)
-    return None;
-
+  TinyPtrVector<ValueDecl *> results;
   auto i = DeclMemberNames->find(N.getBaseName());
   if (i == DeclMemberNames->end())
-    return None;
+    return results;
 
   BitOffset subTableOffset = *i;
   std::unique_ptr<SerializedDeclMembersTable> &subTable =
@@ -1825,7 +1826,6 @@ ModuleFile::loadNamedMembers(const IterableDeclContext *IDC, DeclName N,
   }
 
   assert(subTable);
-  TinyPtrVector<ValueDecl *> results;
   auto j = subTable->find(IDC->getDeclID());
   if (j != subTable->end()) {
     for (DeclID d : *j) {
@@ -1838,8 +1838,9 @@ ModuleFile::loadNamedMembers(const IterableDeclContext *IDC, DeclName N,
       } else {
         if (!getContext().LangOpts.EnableDeserializationRecovery)
           fatal(mem.takeError());
-        // Treat this as a cache-miss to the caller and let them attempt
-        // to refill through the normal loadAllMembers() path.
+        // Eat the error and treat this as a cache-miss to the caller; let
+        // them attempt to refill through the normal loadAllMembers() path.
+        llvm::consumeError(mem.takeError());
         return None;
       }
     }

test/NameBinding/Inputs/NamedLazyMembers/NamedLazyMembers.h

@@ -50,3 +50,50 @@
 
 @end
 
+
+// Don't conform to the protocol; that loads all protocol members.
+@interface SimpleDoer (Category)
+- (void)categoricallyDoSomeWork;
+- (void)categoricallyDoSomeWorkWithSpeed:(int)s;
+- (void)categoricallyDoSomeWorkWithSpeed:(int)s thoroughness:(int)t
+  NS_SWIFT_NAME(categoricallyDoVeryImportantWork(speed:thoroughness:));
+- (void)categoricallyDoSomeWorkWithSpeed:(int)s alacrity:(int)a
+  NS_SWIFT_NAME(categoricallyDoSomeWorkWithSpeed(speed:levelOfAlacrity:));
+
+// These we are generally trying to not-import, via laziness.
+- (void)categoricallyGoForWalk;
+- (void)categoricallyTakeNap;
+- (void)categoricallyEatMeal;
+- (void)categoricallyTidyHome;
+- (void)categoricallyCallFamily;
+- (void)categoricallySingSong;
+- (void)categoricallyReadBook;
+- (void)categoricallyAttendLecture;
+- (void)categoricallyWriteLetter;
+@end
+
+
+@protocol MirroredBase
++ (void)mirroredBaseClassMethod;
+- (void)mirroredBaseInstanceMethod;
+@end
+
+@protocol MirroredDoer <MirroredBase>
++ (void)mirroredDerivedClassMethod;
+- (void)mirroredDerivedInstanceMethod;
+@end
+
+@interface MirroringDoer : NSObject<MirroredDoer>
+- (void)unobtrusivelyGoForWalk;
+- (void)unobtrusivelyTakeNap;
+- (void)unobtrusivelyEatMeal;
+- (void)unobtrusivelyTidyHome;
+- (void)unobtrusivelyCallFamily;
+- (void)unobtrusivelySingSong;
+- (void)unobtrusivelyReadBook;
+- (void)unobtrusivelyAttendLecture;
+- (void)unobtrusivelyWriteLetter;
+@end
+
+@interface DerivedFromMirroringDoer : MirroringDoer
+@end

test/NameBinding/named_lazy_member_loading_objc_category.swift

@@ -0,0 +1,20 @@
+// REQUIRES: objc_interop
+// REQUIRES: OS=macosx
+// RUN: rm -rf %t && mkdir -p %t/stats-pre && mkdir -p %t/stats-post
+//
+// Prime module cache
+// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -typecheck %s
+//
+// 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 %s
+// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -stats-output-dir %t/stats-post %s
+// RUN: %utils/process-stats-dir.py --evaluate-delta 'NumTotalClangImportedEntities < -10' %t/stats-pre %t/stats-post
+
+import NamedLazyMembers
+
+public func foo(d: SimpleDoer) {
+  let _ = d.categoricallyDoSomeWork()
+  let _ = d.categoricallyDoSomeWork(withSpeed:10)
+  let _ = d.categoricallyDoVeryImportantWork(speed:10, thoroughness:12)
+  let _ = d.categoricallyDoSomeWorkWithSpeed(speed:10, levelOfAlacrity:12)
+}

test/NameBinding/named_lazy_member_loading_objc_interface.swift

@@ -6,8 +6,8 @@
 // RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -typecheck %s
 //
 // Check that named-lazy-member-loading reduces the number of Decls deserialized
-// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -stats-output-dir %t/stats-pre %s
-// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -enable-named-lazy-member-loading -stats-output-dir %t/stats-post %s
+// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -disable-named-lazy-member-loading -stats-output-dir %t/stats-pre %s
+// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -stats-output-dir %t/stats-post %s
 // RUN: %utils/process-stats-dir.py --evaluate-delta 'NumTotalClangImportedEntities < -10' %t/stats-pre %t/stats-post
 
 import NamedLazyMembers

test/NameBinding/named_lazy_member_loading_objc_protocol.swift

@@ -6,9 +6,9 @@
 // RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -typecheck %s
 //
 // Check that named-lazy-member-loading reduces the number of Decls deserialized
-// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -stats-output-dir %t/stats-pre %s
-// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -enable-named-lazy-member-loading -stats-output-dir %t/stats-post %s
-// RUN: %utils/process-stats-dir.py --evaluate-delta 'NumTotalClangImportedEntities < -10' %t/stats-pre %t/stats-post
+// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -disable-named-lazy-member-loading -stats-output-dir %t/stats-pre %s
+// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -stats-output-dir %t/stats-post %s
+// RUN: %utils/process-stats-dir.py --evaluate-delta 'NumTotalClangImportedEntities < -9' %t/stats-pre %t/stats-post
 
 import NamedLazyMembers
 

test/NameBinding/named_lazy_member_loading_protocol_mirroring.swift

@@ -0,0 +1,26 @@
+// REQUIRES: objc_interop
+// REQUIRES: OS=macosx
+// RUN: rm -rf %t && mkdir -p %t/stats-pre && mkdir -p %t/stats-post
+//
+// Prime module cache
+// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -typecheck %s
+//
+// 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 %s
+// RUN: %target-swift-frontend -typecheck -I %S/Inputs/NamedLazyMembers -stats-output-dir %t/stats-post %s
+
+import NamedLazyMembers
+
+public func foo(d: MirroringDoer) {
+  let _ = MirroringDoer.mirroredBaseClassMethod()
+  let _ = MirroringDoer.mirroredDerivedClassMethod()
+  let _ = d.mirroredBaseInstanceMethod()
+  let _ = d.mirroredDerivedInstanceMethod()
+}
+
+public func foo(d: DerivedFromMirroringDoer) {
+  let _ = MirroringDoer.mirroredBaseClassMethod()
+  let _ = MirroringDoer.mirroredDerivedClassMethod()
+  let _ = d.mirroredBaseInstanceMethod()
+  let _ = d.mirroredDerivedInstanceMethod()
+}