r244192 - [modules] Defer setting up the lookup table for a DeclContext until we can
Richard Smith
richard-llvm at metafoo.co.uk
Wed Aug 5 21:23:48 PDT 2015
Author: rsmith
Date: Wed Aug 5 23:23:48 2015
New Revision: 244192
URL: http://llvm.org/viewvc/llvm-project?rev=244192&view=rev
Log:
[modules] Defer setting up the lookup table for a DeclContext until we can
determine the primary context, rather than sometimes registering the lookup
table on the wrong context.
This exposed a couple of bugs:
* the odr violation check didn't deal properly with mergeable declarations
if the declaration retained by name lookup wasn't in the canonical
definition of the class
* the (broken) RewriteDecl mechanism would emit two name lookup tables for
the same DeclContext into the same module file (one as part of the
rewritten declaration and one as a visible update for the old declaration)
These are both fixed too.
Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=244192&r1=244191&r2=244192&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Aug 5 23:23:48 2015
@@ -495,19 +495,21 @@ private:
llvm::DenseMap<FileID, FileDeclsInfo> FileDeclIDs;
// Updates for visible decls can occur for other contexts than just the
- // TU, and when we read those update records, the actual context will not
- // be available yet (unless it's the TU), so have this pending map using the
- // ID as a key. It will be realized when the context is actually loaded.
- typedef
- SmallVector<std::pair<serialization::reader::ASTDeclContextNameLookupTable *,
- ModuleFile*>, 1> DeclContextVisibleUpdates;
- typedef llvm::DenseMap<serialization::DeclID, DeclContextVisibleUpdates>
- DeclContextVisibleUpdatesPending;
+ // TU, and when we read those update records, the actual context may not
+ // be available yet, so have this pending map using the ID as a key. It
+ // will be realized when the context is actually loaded.
+ struct PendingVisibleUpdate {
+ ModuleFile *Mod;
+ const unsigned char *Data;
+ unsigned BucketOffset;
+ };
+ typedef SmallVector<PendingVisibleUpdate, 1> DeclContextVisibleUpdates;
/// \brief Updates to the visible declarations of declaration contexts that
/// haven't been loaded yet.
- DeclContextVisibleUpdatesPending PendingVisibleUpdates;
-
+ llvm::DenseMap<serialization::DeclID, DeclContextVisibleUpdates>
+ PendingVisibleUpdates;
+
/// \brief The set of C++ or Objective-C classes that have forward
/// declarations that have not yet been linked to their definitions.
llvm::SmallPtrSet<Decl *, 4> PendingDefinitions;
@@ -524,11 +526,14 @@ private:
/// performed deduplication.
llvm::SetVector<NamedDecl*> PendingMergedDefinitionsToDeduplicate;
- /// \brief Read the records that describe the contents of declcontexts.
- bool ReadDeclContextStorage(ModuleFile &M,
- llvm::BitstreamCursor &Cursor,
- const std::pair<uint64_t, uint64_t> &Offsets,
- serialization::DeclContextInfo &Info);
+ /// \brief Read the record that describes the lexical contents of a DC.
+ bool ReadLexicalDeclContextStorage(ModuleFile &M,
+ llvm::BitstreamCursor &Cursor,
+ uint64_t Offset, DeclContext *DC);
+ /// \brief Read the record that describes the visible contents of a DC.
+ bool ReadVisibleDeclContextStorage(ModuleFile &M,
+ llvm::BitstreamCursor &Cursor,
+ uint64_t Offset, serialization::DeclID ID);
/// \brief A vector containing identifiers that have already been
/// loaded.
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=244192&r1=244191&r2=244192&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Aug 5 23:23:48 2015
@@ -955,48 +955,54 @@ ASTDeclContextNameLookupTrait::ReadData(
return std::make_pair(Start, Start + NumDecls);
}
-bool ASTReader::ReadDeclContextStorage(ModuleFile &M,
- BitstreamCursor &Cursor,
- const std::pair<uint64_t, uint64_t> &Offsets,
- DeclContextInfo &Info) {
- SavedStreamPosition SavedPosition(Cursor);
- // First the lexical decls.
- if (Offsets.first != 0) {
- Cursor.JumpToBit(Offsets.first);
+bool ASTReader::ReadLexicalDeclContextStorage(ModuleFile &M,
+ BitstreamCursor &Cursor,
+ uint64_t Offset,
+ DeclContext *DC) {
+ assert(Offset != 0);
- RecordData Record;
- StringRef Blob;
- unsigned Code = Cursor.ReadCode();
- unsigned RecCode = Cursor.readRecord(Code, Record, &Blob);
- if (RecCode != DECL_CONTEXT_LEXICAL) {
- Error("Expected lexical block");
- return true;
- }
+ SavedStreamPosition SavedPosition(Cursor);
+ Cursor.JumpToBit(Offset);
- Info.LexicalDecls = llvm::makeArrayRef(
- reinterpret_cast<const KindDeclIDPair *>(Blob.data()),
- Blob.size() / sizeof(KindDeclIDPair));
+ RecordData Record;
+ StringRef Blob;
+ unsigned Code = Cursor.ReadCode();
+ unsigned RecCode = Cursor.readRecord(Code, Record, &Blob);
+ if (RecCode != DECL_CONTEXT_LEXICAL) {
+ Error("Expected lexical block");
+ return true;
}
- // Now the lookup table.
- if (Offsets.second != 0) {
- Cursor.JumpToBit(Offsets.second);
+ M.DeclContextInfos[DC].LexicalDecls = llvm::makeArrayRef(
+ reinterpret_cast<const KindDeclIDPair *>(Blob.data()),
+ Blob.size() / sizeof(KindDeclIDPair));
+ DC->setHasExternalLexicalStorage(true);
+ return false;
+}
- RecordData Record;
- StringRef Blob;
- unsigned Code = Cursor.ReadCode();
- unsigned RecCode = Cursor.readRecord(Code, Record, &Blob);
- if (RecCode != DECL_CONTEXT_VISIBLE) {
- Error("Expected visible lookup table block");
- return true;
- }
- Info.NameLookupTableData = ASTDeclContextNameLookupTable::Create(
- (const unsigned char *)Blob.data() + Record[0],
- (const unsigned char *)Blob.data() + sizeof(uint32_t),
- (const unsigned char *)Blob.data(),
- ASTDeclContextNameLookupTrait(*this, M));
+bool ASTReader::ReadVisibleDeclContextStorage(ModuleFile &M,
+ BitstreamCursor &Cursor,
+ uint64_t Offset,
+ DeclID ID) {
+ assert(Offset != 0);
+
+ SavedStreamPosition SavedPosition(Cursor);
+ Cursor.JumpToBit(Offset);
+
+ RecordData Record;
+ StringRef Blob;
+ unsigned Code = Cursor.ReadCode();
+ unsigned RecCode = Cursor.readRecord(Code, Record, &Blob);
+ if (RecCode != DECL_CONTEXT_VISIBLE) {
+ Error("Expected visible lookup table block");
+ return true;
}
+ // We can't safely determine the primary context yet, so delay attaching the
+ // lookup table until we're done with recursive deserialization.
+ unsigned BucketOffset = Record[0];
+ PendingVisibleUpdates[ID].push_back(PendingVisibleUpdate{
+ &M, (const unsigned char *)Blob.data(), BucketOffset});
return false;
}
@@ -2503,20 +2509,14 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
case UPDATE_VISIBLE: {
unsigned Idx = 0;
serialization::DeclID ID = ReadDeclID(F, Record, Idx);
- ASTDeclContextNameLookupTable *Table =
- ASTDeclContextNameLookupTable::Create(
- (const unsigned char *)Blob.data() + Record[Idx++],
- (const unsigned char *)Blob.data() + sizeof(uint32_t),
- (const unsigned char *)Blob.data(),
- ASTDeclContextNameLookupTrait(*this, F));
- if (Decl *D = GetExistingDecl(ID)) {
- auto *DC = cast<DeclContext>(D);
- DC->getPrimaryContext()->setHasExternalVisibleStorage(true);
- auto *&LookupTable = F.DeclContextInfos[DC].NameLookupTableData;
- delete LookupTable;
- LookupTable = Table;
- } else
- PendingVisibleUpdates[ID].push_back(std::make_pair(Table, &F));
+ auto *Data = (const unsigned char*)Blob.data();
+ unsigned BucketOffset = Record[Idx++];
+ PendingVisibleUpdates[ID].push_back(
+ PendingVisibleUpdate{&F, Data, BucketOffset});
+ // If we've already loaded the decl, perform the updates when we finish
+ // loading this block.
+ if (Decl *D = GetExistingDecl(ID))
+ PendingUpdateRecords.push_back(std::make_pair(ID, D));
break;
}
@@ -8331,21 +8331,26 @@ void ASTReader::diagnoseOdrViolations()
if (Found)
continue;
+ // Quick check failed, time to do the slow thing. Note, we can't just
+ // look up the name of D in CanonDef here, because the member that is
+ // in CanonDef might not be found by name lookup (it might have been
+ // replaced by a more recent declaration in the lookup table), and we
+ // can't necessarily find it in the redeclaration chain because it might
+ // be merely mergeable, not redeclarable.
llvm::SmallVector<const NamedDecl*, 4> Candidates;
- DeclContext::lookup_result R = CanonDef->lookup(D->getDeclName());
- for (DeclContext::lookup_iterator I = R.begin(), E = R.end();
- !Found && I != E; ++I) {
- for (auto RI : (*I)->redecls()) {
- if (RI->getLexicalDeclContext() == CanonDef) {
- // This declaration is present in the canonical definition. If it's
- // in the same redecl chain, it's the one we're looking for.
- if (RI->getCanonicalDecl() == DCanon)
- Found = true;
- else
- Candidates.push_back(cast<NamedDecl>(RI));
- break;
- }
+ for (auto *CanonMember : CanonDef->decls()) {
+ if (CanonMember->getCanonicalDecl() == DCanon) {
+ // This can happen if the declaration is merely mergeable and not
+ // actually redeclarable (we looked for redeclarations earlier).
+ //
+ // FIXME: We should be able to detect this more efficiently, without
+ // pulling in all of the members of CanonDef.
+ Found = true;
+ break;
}
+ if (auto *ND = dyn_cast<NamedDecl>(CanonMember))
+ if (ND->getDeclName() == D->getDeclName())
+ Candidates.push_back(ND);
}
if (!Found) {
@@ -8454,11 +8459,11 @@ void ASTReader::FinishedDeserializing()
}
}
- diagnoseOdrViolations();
-
if (ReadTimer)
ReadTimer->stopTimer();
+ diagnoseOdrViolations();
+
// We are not in recursive loading, so it's safe to pass the "interesting"
// decls to the consumer.
if (Consumer)
@@ -8527,14 +8532,4 @@ ASTReader::ASTReader(Preprocessor &PP, A
ASTReader::~ASTReader() {
if (OwnsDeserializationListener)
delete DeserializationListener;
-
- for (DeclContextVisibleUpdatesPending::iterator
- I = PendingVisibleUpdates.begin(),
- E = PendingVisibleUpdates.end();
- I != E; ++I) {
- for (DeclContextVisibleUpdates::iterator J = I->second.begin(),
- F = I->second.end();
- J != F; ++J)
- delete J->first;
- }
}
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=244192&r1=244191&r2=244192&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Aug 5 23:23:48 2015
@@ -3339,37 +3339,13 @@ Decl *ASTReader::ReadDeclRecord(DeclID I
// If this declaration is also a declaration context, get the
// offsets for its tables of lexical and visible declarations.
if (DeclContext *DC = dyn_cast<DeclContext>(D)) {
- // FIXME: This should really be
- // DeclContext *LookupDC = DC->getPrimaryContext();
- // but that can walk the redeclaration chain, which might not work yet.
- DeclContext *LookupDC = DC;
- if (isa<NamespaceDecl>(DC))
- LookupDC = DC->getPrimaryContext();
std::pair<uint64_t, uint64_t> Offsets = Reader.VisitDeclContext(DC);
- if (Offsets.first || Offsets.second) {
- if (Offsets.first != 0)
- DC->setHasExternalLexicalStorage(true);
- if (Offsets.second != 0)
- LookupDC->setHasExternalVisibleStorage(true);
- if (ReadDeclContextStorage(*Loc.F, DeclsCursor, Offsets,
- Loc.F->DeclContextInfos[DC]))
- return nullptr;
- }
-
- // Now add the pending visible updates for this decl context, if it has any.
- DeclContextVisibleUpdatesPending::iterator I =
- PendingVisibleUpdates.find(ID);
- if (I != PendingVisibleUpdates.end()) {
- // There are updates. This means the context has external visible
- // storage, even if the original stored version didn't.
- LookupDC->setHasExternalVisibleStorage(true);
- for (const auto &Update : I->second) {
- DeclContextInfo &Info = Update.second->DeclContextInfos[DC];
- delete Info.NameLookupTableData;
- Info.NameLookupTableData = Update.first;
- }
- PendingVisibleUpdates.erase(I);
- }
+ if (Offsets.first &&
+ ReadLexicalDeclContextStorage(*Loc.F, DeclsCursor, Offsets.first, DC))
+ return nullptr;
+ if (Offsets.second &&
+ ReadVisibleDeclContextStorage(*Loc.F, DeclsCursor, Offsets.second, ID))
+ return nullptr;
}
assert(Idx == Record.size());
@@ -3392,17 +3368,37 @@ Decl *ASTReader::ReadDeclRecord(DeclID I
}
void ASTReader::loadDeclUpdateRecords(serialization::DeclID ID, Decl *D) {
+ // Load the pending visible updates for this decl context, if it has any.
+ auto I = PendingVisibleUpdates.find(ID);
+ if (I != PendingVisibleUpdates.end()) {
+ auto VisibleUpdates = std::move(I->second);
+ PendingVisibleUpdates.erase(I);
+
+ auto *DC = cast<DeclContext>(D)->getPrimaryContext();
+ for (const PendingVisibleUpdate &Update : VisibleUpdates) {
+ auto *&LookupTable = Update.Mod->DeclContextInfos[DC].NameLookupTableData;
+ assert(!LookupTable && "multiple lookup tables for DC in module");
+ LookupTable = reader::ASTDeclContextNameLookupTable::Create(
+ Update.Data + Update.BucketOffset,
+ Update.Data + sizeof(uint32_t),
+ Update.Data,
+ reader::ASTDeclContextNameLookupTrait(*this, *Update.Mod));
+ }
+ DC->setHasExternalVisibleStorage(true);
+ }
+
// The declaration may have been modified by files later in the chain.
// If this is the case, read the record containing the updates from each file
// and pass it to ASTDeclReader to make the modifications.
DeclUpdateOffsetsMap::iterator UpdI = DeclUpdateOffsets.find(ID);
if (UpdI != DeclUpdateOffsets.end()) {
- FileOffsetsTy &UpdateOffsets = UpdI->second;
+ auto UpdateOffsets = std::move(UpdI->second);
+ DeclUpdateOffsets.erase(UpdI);
+
bool WasInteresting = isConsumerInterestedIn(D, false);
- for (FileOffsetsTy::iterator
- I = UpdateOffsets.begin(), E = UpdateOffsets.end(); I != E; ++I) {
- ModuleFile *F = I->first;
- uint64_t Offset = I->second;
+ for (auto &FileAndOffset : UpdateOffsets) {
+ ModuleFile *F = FileAndOffset.first;
+ uint64_t Offset = FileAndOffset.second;
llvm::BitstreamCursor &Cursor = F->DeclsCursor;
SavedStreamPosition SavedPosition(Cursor);
Cursor.JumpToBit(Offset);
@@ -3817,10 +3813,8 @@ void ASTDeclReader::UpdateDecl(Decl *D,
// Visible update is handled separately.
uint64_t LexicalOffset = Record[Idx++];
if (!HadRealDefinition && LexicalOffset) {
- RD->setHasExternalLexicalStorage(true);
- Reader.ReadDeclContextStorage(ModuleFile, ModuleFile.DeclsCursor,
- std::make_pair(LexicalOffset, 0),
- ModuleFile.DeclContextInfos[RD]);
+ Reader.ReadLexicalDeclContextStorage(ModuleFile, ModuleFile.DeclsCursor,
+ LexicalOffset, RD);
Reader.PendingFakeDefinitionData.erase(OldDD);
}
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=244192&r1=244191&r2=244192&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Aug 5 23:23:48 2015
@@ -3754,6 +3754,9 @@ uint64_t ASTWriter::WriteDeclContextVisi
/// (in C++), for namespaces, and for classes with forward-declared unscoped
/// enumeration members (in C++11).
void ASTWriter::WriteDeclContextVisibleUpdate(const DeclContext *DC) {
+ if (isRewritten(cast<Decl>(DC)))
+ return;
+
StoredDeclsMap *Map = DC->getLookupPtr();
if (!Map || Map->empty())
return;
@@ -5705,6 +5708,7 @@ void ASTWriter::AddedVisibleDecl(const D
if (!(!D->isFromASTFile() && cast<Decl>(DC)->isFromASTFile()))
return; // Not a source decl added to a DeclContext from PCH.
+ assert(DC == DC->getPrimaryContext() && "added to non-primary context");
assert(!getDefinitiveDeclContext(DC) && "DeclContext not definitive!");
assert(!WritingAST && "Already writing the AST!");
UpdatedDeclContexts.insert(DC);
More information about the cfe-commits
mailing list