Merge pull request #13216 from graydon/force-on-named-lazy-member-loading-retry

Force on named lazy member loading retry

Author: graydon

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;
 
@@ -634,6 +639,11 @@ class IterableDeclContext {
   /// Retrieve the set of members in this context.
   DeclRange getMembers() const;
 
+  /// Retrieve the set of members in this context without loading any from the
+  /// associated lazy loader; this should only be used as part of implementing
+  /// abstractions on top of member loading, such as a name lookup table.
+  DeclRange getCurrentMembersWithoutLoading() const;
+
   /// Add a member to this context. If the hint decl is specified, the new decl
   /// is inserted immediately after the hint.
   void addMember(Decl *member, Decl *hint = nullptr);
@@ -643,6 +653,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

@@ -159,7 +159,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/DeclContext.cpp

@@ -872,10 +872,14 @@ ASTContext &IterableDeclContext::getASTContext() const {
   return getDecl()->getASTContext();
 }
 
+DeclRange IterableDeclContext::getCurrentMembersWithoutLoading() const {
+  return DeclRange(FirstDeclAndLazyMembers.getPointer(), nullptr);
+}
+
 DeclRange IterableDeclContext::getMembers() const {
   loadAllMembers();
 
-  return DeclRange(FirstDeclAndLazyMembers.getPointer(), nullptr);
+  return getCurrentMembersWithoutLoading();
 }
 
 /// Add a member to this context.

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.
@@ -1247,55 +1238,19 @@ void ExtensionDecl::addedMember(Decl *member) {
 // In all lookup routines, the 'ignoreNewExtensions' flag means that
 // lookup should only use the set of extensions already observed.
 
-void NominalTypeDecl::prepareLookupTable(bool ignoreNewExtensions) {
-  // If we haven't allocated the lookup table yet, do so now.
-  if (!LookupTable.getPointer()) {
-    auto &ctx = getASTContext();
-    LookupTable.setPointer(new (ctx) MemberLookupTable(ctx));
-  }
-
-  // If we're an IDC that has not yet loaded its full member set, we're doing
-  // NamedLazyMemberLoading, and we should not force getMembers().
-  if (hasLazyMembers())
-    return;
-
-  // If we haven't walked the member list yet to update the lookup
-  // table, do so now.
-  if (!LookupTable.getInt()) {
-    // Note that we'll have walked the members now.
-    LookupTable.setInt(true);
-
-    // Add the members of the nominal declaration to the table.
-    LookupTable.getPointer()->addMembers(getMembers());
-  }
-
-  if (!ignoreNewExtensions) {
-    // Update the lookup table to introduce members from extensions.
-    LookupTable.getPointer()->updateLookupTable(this);
-  }
-}
-
-void NominalTypeDecl::makeMemberVisible(ValueDecl *member) {
-  if (!LookupTable.getPointer()) {
-    auto &ctx = getASTContext();
-    LookupTable.setPointer(new (ctx) MemberLookupTable(ctx));
-  }
-  
-  LookupTable.getPointer()->addMember(member);
-}
-
 static bool
 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,19 +1259,18 @@ populateLookupTableEntryFromLazyIDCLoader(ASTContext &ctx,
     }
     return false;
   } else {
+    IDC->setLoadingLazyMembers(false);
     if (auto s = ctx.Stats) {
       ++s->getFrontendCounters().NamedLazyMemberLoadFailureCount;
     }
     return true;
   }
 }
 
-static void
-populateLookupTableEntryFromMembers(ASTContext &ctx,
-                                    MemberLookupTable &LookupTable,
-                                    DeclName name,
-                                    IterableDeclContext *IDC) {
-  for (auto m : IDC->getMembers()) {
+static void populateLookupTableEntryFromCurrentMembersWithoutLoading(
+    ASTContext &ctx, MemberLookupTable &LookupTable, DeclName name,
+    IterableDeclContext *IDC) {
+  for (auto m : IDC->getCurrentMembersWithoutLoading()) {
     if (auto v = dyn_cast<ValueDecl>(m)) {
       if (v->getFullName().matchesRef(name)) {
         LookupTable.addMember(m);
@@ -1325,6 +1279,75 @@ populateLookupTableEntryFromMembers(ASTContext &ctx,
   }
 }
 
+static bool
+populateLookupTableEntryFromExtensions(ASTContext &ctx,
+                                       MemberLookupTable &table,
+                                       NominalTypeDecl *nominal,
+                                       DeclName name,
+                                       bool ignoreNewExtensions) {
+  if (!ignoreNewExtensions) {
+    for (auto e : nominal->getExtensions()) {
+      if (e->wasDeserialized() || e->hasClangNode()) {
+        if (populateLookupTableEntryFromLazyIDCLoader(ctx, table,
+                                                      name, e)) {
+          return true;
+        }
+      } else {
+        populateLookupTableEntryFromCurrentMembersWithoutLoading(ctx, table,
+                                                                 name, e);
+      }
+    }
+  }
+  return false;
+}
+
+void NominalTypeDecl::prepareLookupTable(bool ignoreNewExtensions) {
+  // If we haven't allocated the lookup table yet, do so now.
+  if (!LookupTable.getPointer()) {
+    auto &ctx = getASTContext();
+    LookupTable.setPointer(new (ctx) MemberLookupTable(ctx));
+  }
+
+  if (hasLazyMembers()) {
+    // Lazy members: if the table needs population, populate the table _only
+    // from those members already in the IDC member list_ such as implicits or
+    // globals-as-members, then update table entries from the extensions that
+    // have the same names as any such initial-population members.
+    if (!LookupTable.getInt()) {
+      LookupTable.setInt(true);
+      LookupTable.getPointer()->addMembers(getCurrentMembersWithoutLoading());
+      for (auto *m : getCurrentMembersWithoutLoading()) {
+        if (auto v = dyn_cast<ValueDecl>(m)) {
+          populateLookupTableEntryFromExtensions(getASTContext(),
+                                                 *LookupTable.getPointer(),
+                                                 this, v->getBaseName(),
+                                                 ignoreNewExtensions);
+        }
+      }
+    }
+
+  } else {
+    // No lazy members: if the table needs population, populate the table
+    // en-masse; and in either case update the extensions.
+    if (!LookupTable.getInt()) {
+      LookupTable.setInt(true);
+      LookupTable.getPointer()->addMembers(getMembers());
+    }
+    if (!ignoreNewExtensions) {
+      LookupTable.getPointer()->updateLookupTable(this);
+    }
+  }
+}
+
+void NominalTypeDecl::makeMemberVisible(ValueDecl *member) {
+  if (!LookupTable.getPointer()) {
+    auto &ctx = getASTContext();
+    LookupTable.setPointer(new (ctx) MemberLookupTable(ctx));
+  }
+  
+  LookupTable.getPointer()->addMember(member);
+}
+
 TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
                                                   DeclName name,
                                                   bool ignoreNewExtensions) {
@@ -1336,16 +1359,24 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
                 .NominalTypeDecl__lookupDirect.getGuard();
   }
 
-  DEBUG(llvm::dbgs() << getNameStr() << ".lookupDirect(" << name << ")"
-        << ", lookupTable.getInt()=" << LookupTable.getInt()
-        << ", hasLazyMembers()=" << hasLazyMembers()
-        << "\n");
-
   // We only use NamedLazyMemberLoading when a user opts-in and we have
   // not yet loaded all the members into the IDC list in the first place.
   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;
+
+  DEBUG(llvm::dbgs() << getNameStr() << ".lookupDirect(" << name << ")"
+        << ", lookupTable.getInt()=" << LookupTable.getInt()
+        << ", hasLazyMembers()=" << hasLazyMembers()
+        << ", useNamedLazyMemberLoading=" << useNamedLazyMemberLoading
+        << "\n");
+
   // 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) {
@@ -1354,12 +1385,22 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
     // populated the IDC and brought it up to date with any extensions. This
     // 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's possible that the lookup table exists but has information in it
+      // that is either currently out of date or soon to be out of date.
+      // This can happen two ways:
+      //
+      //   - We've not yet indexed the members we have (LookupTable.getInt()
+      //     is zero).
+      //
+      //   - We've still got more lazy members left to load; this can happen
+      //     even if we _did_ index some members.
+      //
+      // In either of these cases, we want to reset the table to empty and
+      // mark it as needing reconstruction.
+      if (LookupTable.getPointer() &&
+          (hasLazyMembers() || !LookupTable.getInt())) {
         LookupTable.getPointer()->clear();
+        LookupTable.setInt(false);
       }
 
       (void)getMembers();
@@ -1373,8 +1414,7 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
     }
 
     // Next, in all cases, prepare the lookup table for use, possibly
-    // repopulating it from the IDC if just did our initial IDC population
-    // above.
+    // repopulating it from the IDC if the IDC member list has just grown.
     prepareLookupTable(ignoreNewExtensions);
 
     // Look for a declaration with this name.
@@ -1395,23 +1435,10 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
     // false, and we fall back to loading all members during the retry.
     auto &Table = *LookupTable.getPointer();
     if (populateLookupTableEntryFromLazyIDCLoader(ctx, Table,
-                                                  name, this)) {
+                                                  name, this) ||
+        populateLookupTableEntryFromExtensions(ctx, Table, this, name,
+                                               ignoreNewExtensions)) {
       useNamedLazyMemberLoading = false;
-    } else {
-      if (!ignoreNewExtensions) {
-        for (auto E : getExtensions()) {
-          if (E->wasDeserialized()) {
-            if (populateLookupTableEntryFromLazyIDCLoader(ctx, Table,
-                                                          name, E)) {
-              useNamedLazyMemberLoading = false;
-              break;
-            }
-          } else {
-            populateLookupTableEntryFromMembers(ctx, Table,
-                                                name, E);
-          }
-        }
-      }
     }
   }