From d91747980a97f8f6bc4ae365823d707ba972ef65 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Tue, 11 Mar 2014 03:10:46 +0000 Subject: [PATCH] If a visibility update record is found for a DeclContext after that Decl has already been loaded, apply that update record to the Decl immediately, rather than adding it to a pending list and never applying it. llvm-svn: 203534 --- clang/include/clang/Serialization/Module.h | 14 ++++++++--- clang/lib/Serialization/ASTReader.cpp | 24 ++++++++++++------- clang/lib/Serialization/ASTReaderDecl.cpp | 10 +++----- clang/lib/Serialization/Module.cpp | 7 ------ .../test/Modules/Inputs/update-after-load/a.h | 1 + .../test/Modules/Inputs/update-after-load/b.h | 2 ++ .../Inputs/update-after-load/module.map | 1 + .../update-after-load/modules.timestamp | 0 clang/test/Modules/update-after-load.cpp | 8 +++++++ 9 files changed, 42 insertions(+), 25 deletions(-) create mode 100644 clang/test/Modules/Inputs/update-after-load/a.h create mode 100644 clang/test/Modules/Inputs/update-after-load/b.h create mode 100644 clang/test/Modules/Inputs/update-after-load/module.map create mode 100644 clang/test/Modules/Inputs/update-after-load/modules.timestamp create mode 100644 clang/test/Modules/update-after-load.cpp diff --git a/clang/include/clang/Serialization/Module.h b/clang/include/clang/Serialization/Module.h index 469785de7c45..caa2c286c4fb 100644 --- a/clang/include/clang/Serialization/Module.h +++ b/clang/include/clang/Serialization/Module.h @@ -44,13 +44,21 @@ enum ModuleKind { MK_MainFile ///< File is a PCH file treated as the actual main file. }; +/// A custom deleter for DeclContextInfo::NameLookupTableData, to allow +/// an incomplete type to be used there. +struct NameLookupTableDataDeleter { + void operator()( + OnDiskChainedHashTable *Ptr) const; +}; + /// \brief Information about the contents of a DeclContext. struct DeclContextInfo { DeclContextInfo() - : NameLookupTableData(), LexicalDecls(), NumLexicalDecls() {} + : NameLookupTableData(), LexicalDecls(), NumLexicalDecls() {} - OnDiskChainedHashTable - *NameLookupTableData; // an ASTDeclContextNameLookupTable. + /// An ASTDeclContextNameLookupTable. + std::unique_ptr, + NameLookupTableDataDeleter> NameLookupTableData; const KindDeclIDPair *LexicalDecls; unsigned NumLexicalDecls; }; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 6f4dc9bfb580..487283c421cd 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -457,6 +457,11 @@ ASTReader::setDeserializationListener(ASTDeserializationListener *Listener) { } +void NameLookupTableDataDeleter:: +operator()(ASTDeclContextNameLookupTable *Ptr) const { + delete Ptr; +} + unsigned ASTSelectorLookupTrait::ComputeHash(Selector Sel) { return serialization::ComputeHash(Sel); @@ -836,11 +841,10 @@ bool ASTReader::ReadDeclContextStorage(ModuleFile &M, Error("Expected visible lookup table block"); return true; } - Info.NameLookupTableData - = ASTDeclContextNameLookupTable::Create( - (const unsigned char *)Blob.data() + Record[0], - (const unsigned char *)Blob.data(), - ASTDeclContextNameLookupTrait(*this, M)); + Info.NameLookupTableData.reset(ASTDeclContextNameLookupTable::Create( + (const unsigned char *)Blob.data() + Record[0], + (const unsigned char *)Blob.data(), + ASTDeclContextNameLookupTrait(*this, M))); } return false; @@ -2493,8 +2497,12 @@ bool ASTReader::ReadASTBlock(ModuleFile &F) { ASTDeclContextNameLookupTrait(*this, F)); if (ID == PREDEF_DECL_TRANSLATION_UNIT_ID) { // Is it the TU? DeclContext *TU = Context.getTranslationUnitDecl(); - F.DeclContextInfos[TU].NameLookupTableData = Table; + F.DeclContextInfos[TU].NameLookupTableData.reset(Table); TU->setHasExternalVisibleStorage(true); + } else if (Decl *D = DeclsLoaded[ID - NUM_PREDEF_DECL_IDS]) { + auto *DC = cast(D); + DC->getPrimaryContext()->setHasExternalVisibleStorage(true); + F.DeclContextInfos[DC].NameLookupTableData.reset(Table); } else PendingVisibleUpdates[ID].push_back(std::make_pair(Table, &F)); break; @@ -6078,7 +6086,7 @@ namespace { return false; // Look for this name within this module. - ASTDeclContextNameLookupTable *LookupTable = + const auto &LookupTable = Info->second.NameLookupTableData; ASTDeclContextNameLookupTable::iterator Pos = LookupTable->find(This->Name); @@ -6208,7 +6216,7 @@ namespace { if (!FoundInfo) return false; - ASTDeclContextNameLookupTable *LookupTable = + const auto &LookupTable = Info->second.NameLookupTableData; bool FoundAnything = false; for (ASTDeclContextNameLookupTable::data_iterator diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index b9034f951f51..fef5f7b88901 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -2609,13 +2609,9 @@ Decl *ASTReader::ReadDeclRecord(DeclID ID) { // There are updates. This means the context has external visible // storage, even if the original stored version didn't. LookupDC->setHasExternalVisibleStorage(true); - DeclContextVisibleUpdates &U = I->second; - for (DeclContextVisibleUpdates::iterator UI = U.begin(), UE = U.end(); - UI != UE; ++UI) { - DeclContextInfo &Info = UI->second->DeclContextInfos[DC]; - delete Info.NameLookupTableData; - Info.NameLookupTableData = UI->first; - } + for (const auto &Update : I->second) + Update.second->DeclContextInfos[DC].NameLookupTableData.reset( + Update.first); PendingVisibleUpdates.erase(I); } } diff --git a/clang/lib/Serialization/Module.cpp b/clang/lib/Serialization/Module.cpp index 2eb397176a12..77dcc4f99e3b 100644 --- a/clang/lib/Serialization/Module.cpp +++ b/clang/lib/Serialization/Module.cpp @@ -45,13 +45,6 @@ ModuleFile::ModuleFile(ModuleKind Kind, unsigned Generation) {} ModuleFile::~ModuleFile() { - for (DeclContextInfosMap::iterator I = DeclContextInfos.begin(), - E = DeclContextInfos.end(); - I != E; ++I) { - if (I->second.NameLookupTableData) - delete I->second.NameLookupTableData; - } - delete static_cast(IdentifierLookupTable); delete static_cast(HeaderFileInfoTable); delete static_cast(SelectorLookupTable); diff --git a/clang/test/Modules/Inputs/update-after-load/a.h b/clang/test/Modules/Inputs/update-after-load/a.h new file mode 100644 index 000000000000..0ebcf3e34a70 --- /dev/null +++ b/clang/test/Modules/Inputs/update-after-load/a.h @@ -0,0 +1 @@ +namespace llvm {} diff --git a/clang/test/Modules/Inputs/update-after-load/b.h b/clang/test/Modules/Inputs/update-after-load/b.h new file mode 100644 index 000000000000..64e9bfd4bc79 --- /dev/null +++ b/clang/test/Modules/Inputs/update-after-load/b.h @@ -0,0 +1,2 @@ +#include "a.h" +namespace llvm { void f(); } diff --git a/clang/test/Modules/Inputs/update-after-load/module.map b/clang/test/Modules/Inputs/update-after-load/module.map new file mode 100644 index 000000000000..21e160ea051b --- /dev/null +++ b/clang/test/Modules/Inputs/update-after-load/module.map @@ -0,0 +1 @@ +module a { header "a.h" } module b { header "b.h" } diff --git a/clang/test/Modules/Inputs/update-after-load/modules.timestamp b/clang/test/Modules/Inputs/update-after-load/modules.timestamp new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/clang/test/Modules/update-after-load.cpp b/clang/test/Modules/update-after-load.cpp new file mode 100644 index 000000000000..f497ea47945b --- /dev/null +++ b/clang/test/Modules/update-after-load.cpp @@ -0,0 +1,8 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -I %S/Inputs/update-after-load -verify -fmodules-cache-path=%t %s + +// expected-no-diagnostics +#include "a.h" +namespace llvm {} +#include "b.h" +void llvm::f() {}