[modules] Remove redundant import of lexical decls when building a lookup table

for a DeclContext, and fix propagation of exception specifications along
redeclaration chains.

This reverts r232905, r232907, and r232907, which reverted r232793, r232853,
and r232853.

One additional change is present here to resolve issues with LLDB: distinguish
between whether lexical decls missing from the lookup table are local or are
provided by the external AST source, and still look in the external source if
that's where they came from.

llvm-svn: 232928

2 files changed, 49 insertions, 52 deletions

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 89e1d52c643..57621a7b1f6 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -7946,7 +7946,7 @@ static GVALinkage basicGVALinkageForVariable(const ASTContext &Context,
     while (LexicalContext && !isa<FunctionDecl>(LexicalContext))
       LexicalContext = LexicalContext->getLexicalParent();
 
-    // Let the static local variable inherit it's linkage from the nearest
+    // Let the static local variable inherit its linkage from the nearest
     // enclosing function.
     if (LexicalContext)
       StaticLocalLinkage =
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index a8e151cc937..1f08a64d3ab 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1011,10 +1011,10 @@ DeclContext::BuildDeclChain(ArrayRef<Decl*> Decls,
 /// built a lookup map. For every name in the map, pull in the new names from
 /// the external storage.
 void DeclContext::reconcileExternalVisibleStorage() const {
-  assert(NeedToReconcileExternalVisibleStorage && LookupPtr.getPointer());
+  assert(NeedToReconcileExternalVisibleStorage && LookupPtr);
   NeedToReconcileExternalVisibleStorage = false;
 
-  for (auto &Lookup : *LookupPtr.getPointer())
+  for (auto &Lookup : *LookupPtr)
     Lookup.second.setHasExternalDecls();
 }
 
@@ -1027,10 +1027,13 @@ DeclContext::LoadLexicalDeclsFromExternalStorage() const {
 
   // Notify that we have a DeclContext that is initializing.
   ExternalASTSource::Deserializing ADeclContext(Source);
-  
+
+  ExternalLexicalStorage = false;
+  bool HadLazyExternalLexicalLookups = HasLazyExternalLexicalLookups;
+  HasLazyExternalLexicalLookups = false;
+
   // Load the external declarations, if any.
   SmallVector<Decl*, 64> Decls;
-  ExternalLexicalStorage = false;
   switch (Source->FindExternalLexicalDecls(this, Decls)) {
   case ELR_Success:
     break;
@@ -1043,6 +1046,9 @@ DeclContext::LoadLexicalDeclsFromExternalStorage() const {
   if (Decls.empty())
     return;
 
+  if (HadLazyExternalLexicalLookups)
+    HasLazyLocalLexicalLookups = true;
+
   // We may have already loaded just the fields of this record, in which case
   // we need to ignore them.
   bool FieldsAlreadyLoaded = false;
@@ -1065,7 +1071,7 @@ ExternalASTSource::SetNoExternalVisibleDeclsForName(const DeclContext *DC,
                                                     DeclarationName Name) {
   ASTContext &Context = DC->getParentASTContext();
   StoredDeclsMap *Map;
-  if (!(Map = DC->LookupPtr.getPointer()))
+  if (!(Map = DC->LookupPtr))
     Map = DC->CreateStoredDeclsMap(Context);
   if (DC->NeedToReconcileExternalVisibleStorage)
     DC->reconcileExternalVisibleStorage();
@@ -1081,7 +1087,7 @@ ExternalASTSource::SetExternalVisibleDeclsForName(const DeclContext *DC,
                                                   ArrayRef<NamedDecl*> Decls) {
   ASTContext &Context = DC->getParentASTContext();
   StoredDeclsMap *Map;
-  if (!(Map = DC->LookupPtr.getPointer()))
+  if (!(Map = DC->LookupPtr))
     Map = DC->CreateStoredDeclsMap(Context);
   if (DC->NeedToReconcileExternalVisibleStorage)
     DC->reconcileExternalVisibleStorage();
@@ -1175,7 +1181,7 @@ void DeclContext::removeDecl(Decl *D) {
     // Remove only decls that have a name
     if (!ND->getDeclName()) return;
 
-    StoredDeclsMap *Map = getPrimaryContext()->LookupPtr.getPointer();
+    StoredDeclsMap *Map = getPrimaryContext()->LookupPtr;
     if (!Map) return;
 
     StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName());
@@ -1263,32 +1269,28 @@ static bool shouldBeHidden(NamedDecl *D) {
 StoredDeclsMap *DeclContext::buildLookup() {
   assert(this == getPrimaryContext() && "buildLookup called on non-primary DC");
 
-  // FIXME: Should we keep going if hasExternalVisibleStorage?
-  if (!LookupPtr.getInt())
-    return LookupPtr.getPointer();
+  if (!HasLazyLocalLexicalLookups && !HasLazyExternalLexicalLookups)
+    return LookupPtr;
+
+  if (HasLazyExternalLexicalLookups)
+    LoadLexicalDeclsFromExternalStorage();
 
   SmallVector<DeclContext *, 2> Contexts;
   collectAllContexts(Contexts);
   for (unsigned I = 0, N = Contexts.size(); I != N; ++I)
-    buildLookupImpl<&DeclContext::decls_begin,
-                    &DeclContext::decls_end>(Contexts[I], false);
+    buildLookupImpl(Contexts[I], hasExternalVisibleStorage());
 
   // We no longer have any lazy decls.
-  LookupPtr.setInt(false);
-  return LookupPtr.getPointer();
+  HasLazyLocalLexicalLookups = false;
+  return LookupPtr;
 }
 
 /// buildLookupImpl - Build part of the lookup data structure for the
 /// declarations contained within DCtx, which will either be this
 /// DeclContext, a DeclContext linked to it, or a transparent context
 /// nested within it.
-template<DeclContext::decl_iterator (DeclContext::*Begin)() const,
-         DeclContext::decl_iterator (DeclContext::*End)() const>
 void DeclContext::buildLookupImpl(DeclContext *DCtx, bool Internal) {
-  for (decl_iterator I = (DCtx->*Begin)(), E = (DCtx->*End)();
-       I != E; ++I) {
-    Decl *D = *I;
-
+  for (Decl *D : DCtx->noload_decls()) {
     // Insert this declaration into the lookup structure, but only if
     // it's semantically within its decl context. Any other decls which
     // should be found in this context are added eagerly.
@@ -1309,7 +1311,7 @@ void DeclContext::buildLookupImpl(DeclContext *DCtx, bool Internal) {
     // context (recursively).
     if (DeclContext *InnerCtx = dyn_cast<DeclContext>(D))
       if (InnerCtx->isTransparentContext() || InnerCtx->isInlineNamespace())
-        buildLookupImpl<Begin, End>(InnerCtx, Internal);
+        buildLookupImpl(InnerCtx, Internal);
   }
 }
 
@@ -1337,9 +1339,9 @@ DeclContext::lookup(DeclarationName Name) const {
     if (NeedToReconcileExternalVisibleStorage)
       reconcileExternalVisibleStorage();
 
-    StoredDeclsMap *Map = LookupPtr.getPointer();
+    StoredDeclsMap *Map = LookupPtr;
 
-    if (LookupPtr.getInt())
+    if (HasLazyLocalLexicalLookups || HasLazyExternalLexicalLookups)
       // FIXME: Make buildLookup const?
       Map = const_cast<DeclContext*>(this)->buildLookup();
 
@@ -1353,7 +1355,7 @@ DeclContext::lookup(DeclarationName Name) const {
       return R.first->second.getLookupResult();
 
     if (Source->FindExternalVisibleDeclsByName(this, Name) || !R.second) {
-      if (StoredDeclsMap *Map = LookupPtr.getPointer()) {
+      if (StoredDeclsMap *Map = LookupPtr) {
         StoredDeclsMap::iterator I = Map->find(Name);
         if (I != Map->end())
           return I->second.getLookupResult();
@@ -1363,8 +1365,8 @@ DeclContext::lookup(DeclarationName Name) const {
     return lookup_result();
   }
 
-  StoredDeclsMap *Map = LookupPtr.getPointer();
-  if (LookupPtr.getInt())
+  StoredDeclsMap *Map = LookupPtr;
+  if (HasLazyLocalLexicalLookups || HasLazyExternalLexicalLookups)
     Map = const_cast<DeclContext*>(this)->buildLookup();
 
   if (!Map)
@@ -1388,26 +1390,18 @@ DeclContext::noload_lookup(DeclarationName Name) {
   if (PrimaryContext != this)
     return PrimaryContext->noload_lookup(Name);
 
-  StoredDeclsMap *Map = LookupPtr.getPointer();
-  if (LookupPtr.getInt()) {
-    // Carefully build the lookup map, without deserializing anything.
+  // If we have any lazy lexical declarations not in our lookup map, add them
+  // now. Don't import any external declarations, not even if we know we have
+  // some missing from the external visible lookups.
+  if (HasLazyLocalLexicalLookups) {
     SmallVector<DeclContext *, 2> Contexts;
     collectAllContexts(Contexts);
     for (unsigned I = 0, N = Contexts.size(); I != N; ++I)
-      buildLookupImpl<&DeclContext::noload_decls_begin,
-                      &DeclContext::noload_decls_end>(Contexts[I], true);
-
-    // We no longer have any lazy decls.
-    LookupPtr.setInt(false);
-
-    // There may now be names for which we have local decls but are
-    // missing the external decls. FIXME: Just set the hasExternalDecls
-    // flag on those names that have external decls.
-    NeedToReconcileExternalVisibleStorage = true;
-
-    Map = LookupPtr.getPointer();
+      buildLookupImpl(Contexts[I], hasExternalVisibleStorage());
+    HasLazyLocalLexicalLookups = false;
   }
 
+  StoredDeclsMap *Map = LookupPtr;
   if (!Map)
     return lookup_result();
 
@@ -1429,8 +1423,9 @@ void DeclContext::localUncachedLookup(DeclarationName Name,
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.
-  if (Name && !LookupPtr.getInt()) {
-    if (StoredDeclsMap *Map = LookupPtr.getPointer()) {
+  // FIXME: Should we be checking these flags on the primary context?
+  if (Name && !HasLazyLocalLexicalLookups && !HasLazyExternalLexicalLookups) {
+    if (StoredDeclsMap *Map = LookupPtr) {
       StoredDeclsMap::iterator Pos = Map->find(Name);
       if (Pos != Map->end()) {
         Results.insert(Results.end(),
@@ -1443,6 +1438,8 @@ void DeclContext::localUncachedLookup(DeclarationName Name,
 
   // Slow case: grovel through the declarations in our chain looking for 
   // matches.
+  // FIXME: If we have lazy external declarations, this will not find them!
+  // FIXME: Should we CollectAllContexts and walk them all here?
   for (Decl *D = FirstDecl; D; D = D->getNextDeclInContext()) {
     if (NamedDecl *ND = dyn_cast<NamedDecl>(D))
       if (ND->getDeclName() == Name)
@@ -1523,7 +1520,7 @@ void DeclContext::makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal,
   // FIXME: As a performance hack, don't add such decls into the translation
   // unit unless we're in C++, since qualified lookup into the TU is never
   // performed.
-  if (LookupPtr.getPointer() || hasExternalVisibleStorage() ||
+  if (LookupPtr || hasExternalVisibleStorage() ||
       ((!Recoverable || D->getDeclContext() != D->getLexicalDeclContext()) &&
        (getParentASTContext().getLangOpts().CPlusPlus ||
         !isTranslationUnit()))) {
@@ -1533,7 +1530,7 @@ void DeclContext::makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal,
     buildLookup();
     makeDeclVisibleInContextImpl(D, Internal);
   } else {
-    LookupPtr.setInt(true);
+    HasLazyLocalLexicalLookups = true;
   }
 
   // If we are a transparent context or inline namespace, insert into our
@@ -1551,7 +1548,7 @@ void DeclContext::makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal,
 
 void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D, bool Internal) {
   // Find or create the stored declaration map.
-  StoredDeclsMap *Map = LookupPtr.getPointer();
+  StoredDeclsMap *Map = LookupPtr;
   if (!Map) {
     ASTContext *C = &getParentASTContext();
     Map = CreateStoredDeclsMap(*C);
@@ -1613,7 +1610,7 @@ DeclContext::udir_range DeclContext::using_directives() const {
 //===----------------------------------------------------------------------===//
 
 StoredDeclsMap *DeclContext::CreateStoredDeclsMap(ASTContext &C) const {
-  assert(!LookupPtr.getPointer() && "context already has a decls map");
+  assert(!LookupPtr && "context already has a decls map");
   assert(getPrimaryContext() == this &&
          "creating decls map on non-primary context");
 
@@ -1625,7 +1622,7 @@ StoredDeclsMap *DeclContext::CreateStoredDeclsMap(ASTContext &C) const {
     M = new StoredDeclsMap();
   M->Previous = C.LastSDM;
   C.LastSDM = llvm::PointerIntPair<StoredDeclsMap*,1>(M, Dependent);
-  LookupPtr.setPointer(M);
+  LookupPtr = M;
   return M;
 }
 
@@ -1657,11 +1654,11 @@ DependentDiagnostic *DependentDiagnostic::Create(ASTContext &C,
   assert(Parent->isDependentContext()
          && "cannot iterate dependent diagnostics of non-dependent context");
   Parent = Parent->getPrimaryContext();
-  if (!Parent->LookupPtr.getPointer())
+  if (!Parent->LookupPtr)
     Parent->CreateStoredDeclsMap(C);
 
-  DependentStoredDeclsMap *Map
-    = static_cast<DependentStoredDeclsMap*>(Parent->LookupPtr.getPointer());
+  DependentStoredDeclsMap *Map =
+      static_cast<DependentStoredDeclsMap *>(Parent->LookupPtr);
 
   // Allocate the copy of the PartialDiagnostic via the ASTContext's
   // BumpPtrAllocator, rather than the ASTContext itself.