Merge pull request #13204 from apple/revert-12843-force-on-named-lazy-member-loading

Revert "Finish and default-enable named lazy member loading"

Author: aschwaighofer

include/swift/AST/DeclContext.h

@@ -610,15 +610,10 @@ 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;
 
@@ -648,14 +643,6 @@ 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 = true;
+    bool NamedLazyMemberLoading = false;
 
     /// 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 disable_named_lazy_member_loading : Flag<["-"], "disable-named-lazy-member-loading">,
-  HelpText<"Disable per-name lazy member loading">;
+def enable_named_lazy_member_loading : Flag<["-"], "enable-named-lazy-member-loading">,
+  HelpText<"Enable per-name lazy member loading">;
 
 def debug_generic_signatures : Flag<["-"], "debug-generic-signatures">,
   HelpText<"Debug generic signatures">;

lib/AST/NameLookup.cpp

@@ -1024,6 +1024,15 @@ 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.
@@ -1280,14 +1289,13 @@ 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;
     }
@@ -1296,7 +1304,6 @@ populateLookupTableEntryFromLazyIDCLoader(ASTContext &ctx,
     }
     return false;
   } else {
-    IDC->setLoadingLazyMembers(false);
     if (auto s = ctx.Stats) {
       ++s->getFrontendCounters().NamedLazyMemberLoadFailureCount;
     }
@@ -1339,13 +1346,6 @@ 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,11 +1355,10 @@ 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. 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()) {
+      // 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());
         LookupTable.getPointer()->clear();
       }
 
@@ -1401,7 +1400,7 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
     } else {
       if (!ignoreNewExtensions) {
         for (auto E : getExtensions()) {
-          if (E->wasDeserialized() || E->hasClangNode()) {
+          if (E->wasDeserialized()) {
             if (populateLookupTableEntryFromLazyIDCLoader(ctx, Table,
                                                           name, E)) {
               useNamedLazyMemberLoading = false;

lib/ClangImporter/ClangImporter.cpp

@@ -3278,23 +3278,8 @@ 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)
@@ -3318,32 +3303,29 @@ ClangImporter::Implementation::loadNamedMembers(
 
   clang::ASTContext &clangCtx = getClangASTContext();
 
-  assert(isa<clang::ObjCContainerDecl>(CD));
-
   TinyPtrVector<ValueDecl *> Members;
-  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);
+  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);
+          }
         }
       }
     }
+    return Members;
   }
-  return Members;
+
+  return None;
 }
 
 

lib/ClangImporter/ImportDecl.cpp

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

lib/Frontend/CompilerInvocation.cpp

@@ -989,7 +989,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_disable_named_lazy_member_loading);
+  Opts.NamedLazyMemberLoading |= Args.hasArg(OPT_enable_named_lazy_member_loading);
   Opts.DebugGenericSignatures |= Args.hasArg(OPT_debug_generic_signatures);
 
   Opts.DebuggerSupport |= Args.hasArg(OPT_debugger_support);

lib/Serialization/ModuleFile.cpp

@@ -19,7 +19,6 @@
 #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"
@@ -1795,15 +1794,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);
 
-  TinyPtrVector<ValueDecl *> results;
+  if (!DeclMemberNames)
+    return None;
+
   auto i = DeclMemberNames->find(N.getBaseName());
   if (i == DeclMemberNames->end())
-    return results;
+    return None;
 
   BitOffset subTableOffset = *i;
   std::unique_ptr<SerializedDeclMembersTable> &subTable =
@@ -1826,6 +1825,7 @@ 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,9 +1838,8 @@ ModuleFile::loadNamedMembers(const IterableDeclContext *IDC, DeclName N,
       } else {
         if (!getContext().LangOpts.EnableDeserializationRecovery)
           fatal(mem.takeError());
-        // 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());
+        // Treat this as a cache-miss to the caller and let them attempt
+        // to refill through the normal loadAllMembers() path.
         return None;
       }
     }

test/NameBinding/Inputs/NamedLazyMembers/NamedLazyMembers.h

@@ -50,50 +50,3 @@
 
 @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

@@ -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 -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: %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
 
 import NamedLazyMembers

test/NameBinding/named_lazy_member_loading_objc_interface.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 -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
+// 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
 
 import NamedLazyMembers