Fix an issue with writing to PCH another included PCH, introduced by the "using an AST on-disk hash table for name lookup" commit.

When including a PCH and later re-emitting to another PCH, the name lookup tables of DeclContexts
may be incomplete, since we now lazily deserialize the visible decls of a particular name.
Fix the issue by iterating over the un-deserialized visible decls and completing the lookup tables
of DeclContexts before writing them out.

llvm-svn: 111698

clang/include/clang/AST/DeclBase.h

@@ -1095,6 +1095,14 @@ public:
   /// the declaration chains.
   void makeDeclVisibleInContext(NamedDecl *D, bool Recoverable = true);
 
+  /// \brief Deserialize all the visible declarations from external storage.
+  ///
+  /// Name lookup deserializes visible declarations lazily, thus a DeclContext
+  /// may not have a complete name lookup table. This function deserializes
+  /// the rest of visible declarations from the external storage and completes
+  /// the name lookup table.
+  void MaterializeVisibleDeclsFromExternalStorage();
+
   /// udir_iterator - Iterates through the using-directives stored
   /// within this context.
   typedef UsingDirectiveDecl * const * udir_iterator;
@@ -1152,7 +1160,6 @@ public:
 
 private:
   void LoadLexicalDeclsFromExternalStorage() const;
-  void LoadVisibleDeclsFromExternalStorage() const;
 
   friend class DependentDiagnostic;
   StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const;

clang/include/clang/AST/ExternalASTSource.h

@@ -95,6 +95,14 @@ public:
   FindExternalVisibleDeclsByName(const DeclContext *DC,
                                  DeclarationName Name) = 0;
 
+  /// \brief Deserialize all the visible declarations from external storage.
+  ///
+  /// Name lookup deserializes visible declarations lazily, thus a DeclContext
+  /// may not have a complete name lookup table. This function deserializes
+  /// the rest of visible declarations from the external storage and completes
+  /// the name lookup table of the DeclContext.
+  virtual void MaterializeVisibleDecls(const DeclContext *DC) = 0;
+
   /// \brief Finds all declarations lexically contained within the given
   /// DeclContext.
   ///
@@ -136,6 +144,10 @@ protected:
   static DeclContext::lookup_result
   SetNoExternalVisibleDeclsForName(const DeclContext *DC,
                                    DeclarationName Name);
+
+  void MaterializeVisibleDeclsForName(const DeclContext *DC,
+                                 DeclarationName Name,
+                                 llvm::SmallVectorImpl<NamedDecl*> &Decls);
 };
 
 /// \brief A lazy pointer to an AST node (of base type T) that resides

clang/include/clang/Serialization/ASTReader.h

@@ -763,6 +763,8 @@ public:
   FindExternalVisibleDeclsByName(const DeclContext *DC,
                                  DeclarationName Name);
 
+  virtual void MaterializeVisibleDecls(const DeclContext *DC);
+
   /// \brief Read all of the declarations lexically stored in a
   /// declaration context.
   ///

clang/lib/AST/DeclBase.cpp

@@ -667,6 +667,25 @@ ExternalASTSource::SetExternalVisibleDeclsForName(const DeclContext *DC,
   return List.getLookupResult();
 }
 
+void ExternalASTSource::MaterializeVisibleDeclsForName(const DeclContext *DC,
+                                                       DeclarationName Name,
+                                     llvm::SmallVectorImpl<NamedDecl*> &Decls) {
+  assert(DC->LookupPtr);
+  StoredDeclsMap &Map = *DC->LookupPtr;
+
+  // If there's an entry in the table the visible decls for this name have
+  // already been deserialized.
+  if (Map.find(Name) == Map.end()) {
+    StoredDeclsList &List = Map[Name];
+    for (unsigned I = 0, N = Decls.size(); I != N; ++I) {
+      if (List.isNull())
+        List.setOnlyValue(Decls[I]);
+      else
+        List.AddSubsequentDecl(Decls[I]);
+    }
+  }
+}
+
 DeclContext::decl_iterator DeclContext::noload_decls_begin() const {
   return decl_iterator(FirstDecl);
 }
@@ -916,6 +935,15 @@ void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D) {
   DeclNameEntries.AddSubsequentDecl(D);
 }
 
+void DeclContext::MaterializeVisibleDeclsFromExternalStorage() {
+  ExternalASTSource *Source = getParentASTContext().getExternalSource();
+  assert(hasExternalVisibleStorage() && Source && "No external storage?");
+
+  if (!LookupPtr)
+    CreateStoredDeclsMap(getParentASTContext());
+  Source->MaterializeVisibleDecls(this);
+}
+
 /// Returns iterator range [First, Last) of UsingDirectiveDecls stored within
 /// this context.
 DeclContext::udir_iterator_range

clang/lib/Serialization/ASTReader.cpp

@@ -790,6 +790,44 @@ public:
     return Key;
   }
 
+  external_key_type GetExternalKey(const internal_key_type& Key) const {
+    ASTContext *Context = Reader.getContext();
+    switch (Key.Kind) {
+    case DeclarationName::Identifier:
+      return DeclarationName((IdentifierInfo*)Key.Data);
+
+    case DeclarationName::ObjCZeroArgSelector:
+    case DeclarationName::ObjCOneArgSelector:
+    case DeclarationName::ObjCMultiArgSelector:
+      return DeclarationName(Selector(Key.Data));
+
+    case DeclarationName::CXXConstructorName:
+      return Context->DeclarationNames.getCXXConstructorName(
+                           Context->getCanonicalType(Reader.GetType(Key.Data)));
+
+    case DeclarationName::CXXDestructorName:
+      return Context->DeclarationNames.getCXXDestructorName(
+                           Context->getCanonicalType(Reader.GetType(Key.Data)));
+
+    case DeclarationName::CXXConversionFunctionName:
+      return Context->DeclarationNames.getCXXConversionFunctionName(
+                           Context->getCanonicalType(Reader.GetType(Key.Data)));
+
+    case DeclarationName::CXXOperatorName:
+      return Context->DeclarationNames.getCXXOperatorName(
+                                         (OverloadedOperatorKind)Key.Data);
+
+    case DeclarationName::CXXLiteralOperatorName:
+      return Context->DeclarationNames.getCXXLiteralOperatorName(
+                                                     (IdentifierInfo*)Key.Data);
+
+    case DeclarationName::CXXUsingDirective:
+      return DeclarationName::getUsingDirectiveName();
+    }
+
+    llvm_unreachable("Invalid Name Kind ?");
+  }
+
   static std::pair<unsigned, unsigned>
   ReadKeyDataLength(const unsigned char*& d) {
     using namespace clang::io;
@@ -3197,6 +3235,35 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
   return const_cast<DeclContext*>(DC)->lookup(Name);
 }
 
+void ASTReader::MaterializeVisibleDecls(const DeclContext *DC) {
+  assert(DC->hasExternalVisibleStorage() &&
+         "DeclContext has no visible decls in storage");
+
+  llvm::SmallVector<NamedDecl *, 64> Decls;
+  // There might be visible decls in multiple parts of the chain, for the TU
+  // and namespaces.
+  DeclContextInfos &Infos = DeclContextOffsets[DC];
+  for (DeclContextInfos::iterator I = Infos.begin(), E = Infos.end();
+       I != E; ++I) {
+    if (!I->NameLookupTableData)
+      continue;
+
+    ASTDeclContextNameLookupTable *LookupTable =
+        (ASTDeclContextNameLookupTable*)I->NameLookupTableData;
+    for (ASTDeclContextNameLookupTable::item_iterator
+           ItemI = LookupTable->item_begin(),
+           ItemEnd = LookupTable->item_end() ; ItemI != ItemEnd; ++ItemI) {
+      ASTDeclContextNameLookupTable::item_iterator::value_type Val
+          = *ItemI;
+      ASTDeclContextNameLookupTrait::data_type Data = Val.second;
+      Decls.clear();
+      for (; Data.first != Data.second; ++Data.first)
+        Decls.push_back(cast<NamedDecl>(GetDecl(*Data.first)));
+      MaterializeVisibleDeclsForName(DC, Val.first, Decls);
+    }
+  }
+}
+
 void ASTReader::PassInterestingDeclsToConsumer() {
   assert(Consumer);
   while (!InterestingDecls.empty()) {

clang/lib/Serialization/ASTWriter.cpp

@@ -2032,7 +2032,10 @@ uint64_t ASTWriter::WriteDeclContextVisibleBlock(ASTContext &Context,
     return 0;
 
   // Force the DeclContext to build a its name-lookup table.
-  DC->lookup(DeclarationName());
+  if (DC->hasExternalVisibleStorage())
+    DC->MaterializeVisibleDeclsFromExternalStorage();
+  else
+    DC->lookup(DeclarationName());
 
   // Serialize the contents of the mapping used for lookup. Note that,
   // although we have two very different code paths, the serialized

clang/test/PCH/reinclude.cpp

@@ -0,0 +1,8 @@
+// Test without PCH
+// RUN: %clang_cc1 %s -include %S/reinclude1.h -include %S/reinclude2.h -fsyntax-only -verify
+
+// RUN: %clang_cc1 -x c++-header %S/reinclude1.h -emit-pch -o %t1
+// RUN: %clang_cc1 -x c++-header %S/reinclude2.h -include-pch %t1 -emit-pch -o %t2
+// RUN: %clang_cc1 %s -include-pch %t2 -fsyntax-only -verify
+
+int q2 = A::y;

clang/test/PCH/reinclude1.h

@@ -0,0 +1,4 @@
+namespace A {
+  int x;
+  int y;
+}

clang/test/PCH/reinclude2.h

@@ -0,0 +1 @@
+int q1 = A::x;