From a60786b46eaa4766bb57fb3ca4e0191b3f73e42a Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Fri, 20 Aug 2010 23:35:55 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@111698 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/DeclBase.h | 9 +++- include/clang/AST/ExternalASTSource.h | 12 +++++ include/clang/Serialization/ASTReader.h | 2 + lib/AST/DeclBase.cpp | 28 +++++++++++ lib/Serialization/ASTReader.cpp | 67 +++++++++++++++++++++++++ lib/Serialization/ASTWriter.cpp | 5 +- test/PCH/reinclude.cpp | 8 +++ test/PCH/reinclude1.h | 4 ++ test/PCH/reinclude2.h | 1 + 9 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 test/PCH/reinclude.cpp create mode 100644 test/PCH/reinclude1.h create mode 100644 test/PCH/reinclude2.h diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h index 34092c7993..999e45a94a 100644 --- a/include/clang/AST/DeclBase.h +++ b/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; diff --git a/include/clang/AST/ExternalASTSource.h b/include/clang/AST/ExternalASTSource.h index 24d9bc5bca..6bd72529e6 100644 --- a/include/clang/AST/ExternalASTSource.h +++ b/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 &Decls); }; /// \brief A lazy pointer to an AST node (of base type T) that resides diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index 3f7480cee7..f28eb26d93 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/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. /// diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp index 30d540a00a..3dd7abab0c 100644 --- a/lib/AST/DeclBase.cpp +++ b/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 &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 diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index b24b35309a..6e5638b3c8 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/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 ReadKeyDataLength(const unsigned char*& d) { using namespace clang::io; @@ -3197,6 +3235,35 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC, return const_cast(DC)->lookup(Name); } +void ASTReader::MaterializeVisibleDecls(const DeclContext *DC) { + assert(DC->hasExternalVisibleStorage() && + "DeclContext has no visible decls in storage"); + + llvm::SmallVector 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(GetDecl(*Data.first))); + MaterializeVisibleDeclsForName(DC, Val.first, Decls); + } + } +} + void ASTReader::PassInterestingDeclsToConsumer() { assert(Consumer); while (!InterestingDecls.empty()) { diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 5deaf3d475..f47ad3c4d7 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/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 diff --git a/test/PCH/reinclude.cpp b/test/PCH/reinclude.cpp new file mode 100644 index 0000000000..6ab10027c9 --- /dev/null +++ b/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; diff --git a/test/PCH/reinclude1.h b/test/PCH/reinclude1.h new file mode 100644 index 0000000000..4c8ccaef61 --- /dev/null +++ b/test/PCH/reinclude1.h @@ -0,0 +1,4 @@ +namespace A { + int x; + int y; +} diff --git a/test/PCH/reinclude2.h b/test/PCH/reinclude2.h new file mode 100644 index 0000000000..2aa6d31e83 --- /dev/null +++ b/test/PCH/reinclude2.h @@ -0,0 +1 @@ +int q1 = A::x; -- 2.40.0