Fix handling of module imports adding names to a DeclContext after qualified

name lookup has been performed in that context (this probably only happens in
C++).

1) Whenever we add names to a context, set a flag on it, and if we perform
lookup and discover that the context has had a lookup table built but has the
flag set, update all entries in the lookup table with additional names from
the external source.

2) When marking a DeclContext as having external visible decls, mark the
context in which lookup is performed, not the one we are adding. These won't
be the same if we're adding another copy of a pre-existing namespace.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174577 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 36 insertions, 9 deletions

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index efac2fba64..8d9d4ded54 100644
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -928,19 +928,26 @@ class DeclContext {
   /// \brief Whether this declaration context also has some external
   /// storage that contains additional declarations that are lexically
   /// part of this context.
-  mutable unsigned ExternalLexicalStorage : 1;
+  mutable bool ExternalLexicalStorage : 1;
 
   /// \brief Whether this declaration context also has some external
   /// storage that contains additional declarations that are visible
   /// in this context.
-  mutable unsigned ExternalVisibleStorage : 1;
+  mutable bool ExternalVisibleStorage : 1;
+
+  /// \brief Whether this declaration context has had external visible
+  /// storage added since the last lookup. In this case, \c LookupPtr's
+  /// invariant may not hold and needs to be fixed before we perform
+  /// another lookup.
+  mutable bool NeedToReconcileExternalVisibleStorage : 1;
 
   /// \brief Pointer to the data structure used to lookup declarations
   /// within this context (or a DependentStoredDeclsMap if this is a
   /// dependent context), and a bool indicating whether we have lazily
   /// omitted any declarations from the map. We maintain the invariant
-  /// that, if the map contains an entry for a DeclarationName, then it
-  /// contains all relevant entries for that name.
+  /// that, if the map contains an entry for a DeclarationName (and we
+  /// haven't lazily omitted anything), then it contains all relevant
+  /// entries for that name.
   mutable llvm::PointerIntPair<StoredDeclsMap*, 1, bool> LookupPtr;
 
 protected:
@@ -963,10 +970,11 @@ protected:
   static std::pair<Decl *, Decl *>
   BuildDeclChain(ArrayRef<Decl*> Decls, bool FieldsAlreadyLoaded);
 
-   DeclContext(Decl::Kind K)
-     : DeclKind(K), ExternalLexicalStorage(false),
-       ExternalVisibleStorage(false), LookupPtr(0, false), FirstDecl(0),
-       LastDecl(0) { }
+  DeclContext(Decl::Kind K)
+      : DeclKind(K), ExternalLexicalStorage(false),
+        ExternalVisibleStorage(false),
+        NeedToReconcileExternalVisibleStorage(false), LookupPtr(0, false),
+        FirstDecl(0), LastDecl(0) {}
 
 public:
   ~DeclContext();
@@ -1497,6 +1505,8 @@ public:
   /// declarations visible in this context.
   void setHasExternalVisibleStorage(bool ES = true) {
     ExternalVisibleStorage = ES;
+    if (ES)
+      NeedToReconcileExternalVisibleStorage = true;
   }
 
   /// \brief Determine whether the given declaration is stored in the list of
@@ -1512,6 +1522,7 @@ public:
   LLVM_ATTRIBUTE_USED void dumpDeclContext() const;
 
 private:
+  void reconcileExternalVisibleStorage();
   void LoadLexicalDeclsFromExternalStorage() const;
 
   /// @brief Makes a declaration visible within this context, but
diff --git a/include/clang/AST/DeclContextInternals.h b/include/clang/AST/DeclContextInternals.h
index 6acee7d85c..84f3698d6b 100644
--- a/include/clang/AST/DeclContextInternals.h
+++ b/include/clang/AST/DeclContextInternals.h
@@ -97,6 +97,22 @@ public:
              == Vec.end() && "list still contains decl");
   }
 
+  /// \brief Remove any declarations which were imported from an external
+  /// AST source.
+  void removeExternalDecls() {
+    if (isNull()) {
+      // Nothing to do.
+    } else if (NamedDecl *Singleton = getAsDecl()) {
+      if (Singleton->isFromASTFile())
+        *this = StoredDeclsList();
+    } else {
+      DeclsTy &Vec = *getAsVector();
+      Vec.erase(std::remove_if(Vec.begin(), Vec.end(),
+                               std::mem_fun(&Decl::isFromASTFile)),
+                Vec.end());
+    }
+  }
+
   /// getLookupResult - Return an array of all the decls that this list
   /// represents.
   DeclContext::lookup_result getLookupResult() {
@@ -186,7 +202,7 @@ public:
     // All other declarations go at the end of the list, but before any
     // tag declarations.  But we can be clever about tag declarations
     // because there can only ever be one in a scope.
-    } else if (Vec.back()->hasTagIdentifierNamespace()) {
+    } else if (!Vec.empty() && Vec.back()->hasTagIdentifierNamespace()) {
       NamedDecl *TagD = Vec.back();
       Vec.back() = D;
       Vec.push_back(TagD);