<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Jul 13, 2015 at 1:45 AM Richard Smith <<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Sun Jul 12 18:43:21 2015<br>
New Revision: 241999<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D241999-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=P0K9mGk31ljPF50voOxt73tmL9C6j6kfy9tBPgrd2PA&s=9wyS_gPitiI0Q9rhqqykdjjc4iEru6rDHbLzmPQyN0c&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=241999&view=rev</a><br>
Log:<br>
[modules] Improve performance when there is a local declaration of an entity<br>
before the first imported declaration.<br>
<br>
We don't need to track all formerly-canonical declarations of an entity; it's sufficient to track those ones for which no other formerly-canonical declaration was imported into the same module. We call those ones "key declarations", and use them as our starting points for collecting redeclarations and performing namespace lookups.<br></blockquote><div><br></div><div>Do you have any numbers on how much this speeds it up?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
cfe/trunk/include/clang/Serialization/ASTReader.h<br>
cfe/trunk/include/clang/Serialization/ASTWriter.h<br>
cfe/trunk/lib/Serialization/ASTReader.cpp<br>
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_include_clang_Serialization_ASTReader.h-3Frev-3D241999-26r1-3D241998-26r2-3D241999-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=P0K9mGk31ljPF50voOxt73tmL9C6j6kfy9tBPgrd2PA&s=ItceRHM-EgIwGtaZU2y51Rypg9UCnoFm5oWqp2cYz3k&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=241999&r1=241998&r2=241999&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)<br>
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Sun Jul 12 18:43:21 2015<br>
@@ -976,12 +976,14 @@ private:<br>
MergedLookups;<br>
<br>
typedef llvm::DenseMap<Decl *, SmallVector<serialization::DeclID, 2> ><br>
- MergedDeclsMap;<br>
+ KeyDeclsMap;<br>
<br>
- /// \brief A mapping from canonical declarations to the set of additional<br>
- /// (global, previously-canonical) declaration IDs that have been merged with<br>
- /// that canonical declaration.<br>
- MergedDeclsMap MergedDecls;<br>
+ /// \brief A mapping from canonical declarations to the set of global<br>
+ /// declaration IDs for key declaration that have been merged with that<br>
+ /// canonical declaration. A key declaration is a formerly-canonical<br>
+ /// declaration whose module did not import any other key declaration for that<br>
+ /// entity. These are the IDs that we use as keys when finding redecl chains.<br>
+ KeyDeclsMap KeyDecls;<br>
<br>
/// \brief A mapping from DeclContexts to the semantic DeclContext that we<br>
/// are treating as the definition of the entity. This is used, for instance,<br>
@@ -1054,6 +1056,36 @@ public:<br>
void ResolveImportedPath(ModuleFile &M, std::string &Filename);<br>
static void ResolveImportedPath(std::string &Filename, StringRef Prefix);<br>
<br>
+ /// \brief Returns the first key declaration for the given declaration. This<br>
+ /// is one that is formerly-canonical (or still canonical) and whose module<br>
+ /// did not import any other key declaration of the entity.<br>
+ Decl *getKeyDeclaration(Decl *D) {<br></blockquote><div><br></div><div>Should the comment say "Returns the first imported key declaration"? Otherwise it sounds like there'd be multiple key decls in the current context.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ D = D->getCanonicalDecl();<br>
+ if (D->isFromASTFile())<br>
+ return D;<br></blockquote><div><br></div><div>I think I asked you that in person already 2 months back, and I forgot again, but why is the isFromASTFile check enough? (I assume the invariant is that merging makes sure that there is only one canonical decl loaded from a module, and it's the first one? but then how do we know we never call this during merging?)</div><div>Also, why is the canonical decl that comes from the AST file not also in the KeyDecls?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ auto I = KeyDecls.find(D);<br>
+ if (I == KeyDecls.end() || I->second.empty())<br>
+ return D;<br>
+ return GetExistingDecl(I->second[0]);<br>
+ }<br>
+ const Decl *getKeyDeclaration(const Decl *D) {<br>
+ return getKeyDeclaration(const_cast<Decl*>(D));<br>
+ }<br>
+<br>
+ /// \brief Run a callback on each imported key declaration of \p D.<br>
+ template <typename Fn><br>
+ void forEachImportedKeyDecl(const Decl *D, Fn Visit) {<br>
+ D = D->getCanonicalDecl();<br>
+ if (D->isFromASTFile())<br>
+ Visit(D);<br>
+<br>
+ auto It = KeyDecls.find(const_cast<Decl*>(D));<br>
+ if (It != KeyDecls.end())<br>
+ for (auto ID : It->second)<br>
+ Visit(GetExistingDecl(ID));<br>
+ }<br>
+<br>
private:<br>
struct ImportedModule {<br>
ModuleFile *Mod;<br>
@@ -1124,18 +1156,6 @@ private:<br>
/// merged into its redecl chain.<br>
Decl *getMostRecentExistingDecl(Decl *D);<br>
<br>
- template <typename Fn><br>
- void forEachFormerlyCanonicalImportedDecl(const Decl *D, Fn Visit) {<br>
- D = D->getCanonicalDecl();<br>
- if (D->isFromASTFile())<br>
- Visit(D);<br>
-<br>
- auto It = MergedDecls.find(const_cast<Decl*>(D));<br>
- if (It != MergedDecls.end())<br>
- for (auto ID : It->second)<br>
- Visit(GetExistingDecl(ID));<br>
- }<br>
-<br>
RecordLocation DeclCursorForID(serialization::DeclID ID,<br>
unsigned &RawLocation);<br>
void loadDeclUpdateRecords(serialization::DeclID ID, Decl *D);<br>
<br>
Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_include_clang_Serialization_ASTWriter.h-3Frev-3D241999-26r1-3D241998-26r2-3D241999-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=P0K9mGk31ljPF50voOxt73tmL9C6j6kfy9tBPgrd2PA&s=QziiL50O-8a4ft7sKoNnP5r20KsDNHy6aCokUdFmE7k&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=241999&r1=241998&r2=241999&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)<br>
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Sun Jul 12 18:43:21 2015<br>
@@ -398,7 +398,7 @@ private:<br>
<br>
/// \brief The set of declarations that may have redeclaration chains that<br>
/// need to be serialized.<br>
- llvm::SmallSetVector<Decl *, 4> Redeclarations;<br>
+ llvm::SmallVector<const Decl *, 16> Redeclarations;<br>
<br>
/// \brief Statements that we've encountered while serializing a<br>
/// declaration or type.<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Serialization_ASTReader.cpp-3Frev-3D241999-26r1-3D241998-26r2-3D241999-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=P0K9mGk31ljPF50voOxt73tmL9C6j6kfy9tBPgrd2PA&s=jULnWkOFrdUeP1uZ-J9fzX9o33eaxNzO7y3gzvGor08&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=241999&r1=241998&r2=241999&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Sun Jul 12 18:43:21 2015<br>
@@ -6071,7 +6071,7 @@ Decl *ASTReader::GetExistingDecl(DeclID<br>
if (D) {<br>
// Track that we have merged the declaration with ID \p ID into the<br>
// pre-existing predefined declaration \p D.<br>
- auto &Merged = MergedDecls[D->getCanonicalDecl()];<br>
+ auto &Merged = KeyDecls[D->getCanonicalDecl()];<br>
if (Merged.empty())<br>
Merged.push_back(ID);<br>
}<br>
@@ -6413,10 +6413,10 @@ ASTReader::FindExternalVisibleDeclsByNam<br>
Contexts.push_back(DC);<br>
<br>
if (DC->isNamespace()) {<br>
- auto Merged = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));<br>
- if (Merged != MergedDecls.end()) {<br>
- for (unsigned I = 0, N = Merged->second.size(); I != N; ++I)<br>
- Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));<br>
+ auto Key = KeyDecls.find(const_cast<Decl *>(cast<Decl>(DC)));<br>
+ if (Key != KeyDecls.end()) {<br>
+ for (unsigned I = 0, N = Key->second.size(); I != N; ++I)<br>
+ Contexts.push_back(cast<DeclContext>(GetDecl(Key->second[I])));<br>
}<br>
}<br>
<br>
@@ -6533,11 +6533,11 @@ void ASTReader::completeVisibleDeclsMap(<br>
Contexts.push_back(DC);<br>
<br>
if (DC->isNamespace()) {<br>
- MergedDeclsMap::iterator Merged<br>
- = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));<br>
- if (Merged != MergedDecls.end()) {<br>
- for (unsigned I = 0, N = Merged->second.size(); I != N; ++I)<br>
- Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));<br>
+ KeyDeclsMap::iterator Key =<br>
+ KeyDecls.find(const_cast<Decl *>(cast<Decl>(DC)));<br>
+ if (Key != KeyDecls.end()) {<br>
+ for (unsigned I = 0, N = Key->second.size(); I != N; ++I)<br>
+ Contexts.push_back(cast<DeclContext>(GetDecl(Key->second[I])));<br>
}<br>
}<br>
<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Serialization_ASTReaderDecl.cpp-3Frev-3D241999-26r1-3D241998-26r2-3D241999-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=P0K9mGk31ljPF50voOxt73tmL9C6j6kfy9tBPgrd2PA&s=c6M8wFQgK1c4OeFAICZXkEsSvChVdkiyp4ivUKoLqfI&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=241999&r1=241998&r2=241999&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Sun Jul 12 18:43:21 2015<br>
@@ -128,20 +128,22 @@ namespace clang {<br>
GlobalDeclID FirstID;<br>
Decl *MergeWith;<br>
mutable bool Owning;<br>
+ bool IsKeyDecl;<br>
Decl::Kind DeclKind;<br>
<br>
void operator=(RedeclarableResult &) = delete;<br>
<br>
public:<br>
RedeclarableResult(ASTReader &Reader, GlobalDeclID FirstID,<br>
- Decl *MergeWith, Decl::Kind DeclKind)<br>
+ Decl *MergeWith, Decl::Kind DeclKind,<br>
+ bool IsKeyDecl)<br>
: Reader(Reader), FirstID(FirstID), MergeWith(MergeWith),<br>
- Owning(true), DeclKind(DeclKind) {}<br>
+ Owning(true), IsKeyDecl(IsKeyDecl), DeclKind(DeclKind) {}<br>
<br>
RedeclarableResult(RedeclarableResult &&Other)<br>
: Reader(Other.Reader), FirstID(Other.FirstID),<br>
MergeWith(Other.MergeWith), Owning(Other.Owning),<br>
- DeclKind(Other.DeclKind) {<br>
+ IsKeyDecl(Other.IsKeyDecl), DeclKind(Other.DeclKind) {<br>
Other.Owning = false;<br>
}<br>
<br>
@@ -156,6 +158,9 @@ namespace clang {<br>
/// \brief Retrieve the first ID.<br>
GlobalDeclID getFirstID() const { return FirstID; }<br>
<br>
+ /// \brief Is this declaration the key declaration?<br>
+ bool isKeyDecl() const { return IsKeyDecl; }<br>
+<br>
/// \brief Get a known declaration that this should be merged with, if<br>
/// any.<br>
Decl *getKnownMergeTarget() const { return MergeWith; }<br>
@@ -348,7 +353,7 @@ namespace clang {<br>
<br>
void mergeTemplatePattern(RedeclarableTemplateDecl *D,<br>
RedeclarableTemplateDecl *Existing,<br>
- DeclID DsID);<br>
+ DeclID DsID, bool IsKeyDecl);<br>
<br>
ObjCTypeParamList *ReadObjCTypeParamList();<br>
<br>
@@ -2173,12 +2178,16 @@ ASTDeclReader::RedeclarableResult<br>
ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {<br>
DeclID FirstDeclID = ReadDeclID(Record, Idx);<br>
Decl *MergeWith = nullptr;<br>
+ bool IsKeyDecl = ThisDeclID == FirstDeclID;<br>
<br>
// 0 indicates that this declaration was the only declaration of its entity,<br>
// and is used for space optimization.<br>
- if (FirstDeclID == 0)<br>
+ if (FirstDeclID == 0) {<br>
FirstDeclID = ThisDeclID;<br>
- else if (unsigned N = Record[Idx++]) {<br>
+ IsKeyDecl = true;<br>
+ } else if (unsigned N = Record[Idx++]) {<br>
+ IsKeyDecl = false;<br>
+<br>
// We have some declarations that must be before us in our redeclaration<br>
// chain. Read them now, and remember that we ought to merge with one of<br>
// them.<br>
@@ -2204,7 +2213,7 @@ ASTDeclReader::VisitRedeclarable(Redecla<br>
// The result structure takes care to note that we need to load the<br>
// other declaration chains for this ID.<br>
return RedeclarableResult(Reader, FirstDeclID, MergeWith,<br>
- static_cast<T *>(D)->getKind());<br>
+ static_cast<T *>(D)->getKind(), IsKeyDecl);<br>
}<br>
<br>
/// \brief Attempts to merge the given declaration (D) with another declaration<br>
@@ -2243,11 +2252,12 @@ template<typename T> static T assert_cas<br>
/// declarations.<br>
void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D,<br>
RedeclarableTemplateDecl *Existing,<br>
- DeclID DsID) {<br>
+ DeclID DsID, bool IsKeyDecl) {<br>
auto *DPattern = D->getTemplatedDecl();<br>
auto *ExistingPattern = Existing->getTemplatedDecl();<br>
RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(),<br>
- /*MergeWith*/ExistingPattern, DPattern->getKind());<br>
+ /*MergeWith*/ExistingPattern, DPattern->getKind(),<br>
+ IsKeyDecl);<br>
<br>
if (auto *DClass = dyn_cast<CXXRecordDecl>(DPattern)) {<br>
// Merge with any existing definition.<br>
@@ -2310,11 +2320,11 @@ void ASTDeclReader::mergeRedeclarable(Re<br>
if (auto *DTemplate = dyn_cast<RedeclarableTemplateDecl>(D))<br>
mergeTemplatePattern(<br>
DTemplate, assert_cast<RedeclarableTemplateDecl*>(ExistingCanon),<br>
- TemplatePatternID);<br>
+ TemplatePatternID, Redecl.isKeyDecl());<br>
<br>
- // If this declaration was the canonical declaration, make a note of that.<br>
- if (DCanon == D) {<br>
- Reader.MergedDecls[ExistingCanon].push_back(Redecl.getFirstID());<br>
+ // If this declaration is a key declaration, make a note of that.<br>
+ if (Redecl.isKeyDecl()) {<br>
+ Reader.KeyDecls[ExistingCanon].push_back(Redecl.getFirstID());<br>
if (Reader.PendingDeclChainsKnown.insert(ExistingCanon).second)<br>
Reader.PendingDeclChains.push_back(ExistingCanon);<br>
}<br>
@@ -3468,7 +3478,7 @@ namespace {<br>
<br>
return;<br>
}<br>
-<br>
+<br>
// Dig out all of the redeclarations.<br>
unsigned Offset = Result->Offset;<br>
unsigned N = M.RedeclarationChains[Offset];<br>
@@ -3531,9 +3541,9 @@ void ASTReader::loadPendingDeclChain(Dec<br>
GlobalDeclID CanonID = CanonDecl->getGlobalID();<br>
if (CanonID)<br>
SearchDecls.push_back(CanonDecl->getGlobalID()); // Always first.<br>
- MergedDeclsMap::iterator MergedPos = MergedDecls.find(CanonDecl);<br>
- if (MergedPos != MergedDecls.end())<br>
- SearchDecls.append(MergedPos->second.begin(), MergedPos->second.end());<br>
+ KeyDeclsMap::iterator KeyPos = KeyDecls.find(CanonDecl);<br>
+ if (KeyPos != KeyDecls.end())<br>
+ SearchDecls.append(KeyPos->second.begin(), KeyPos->second.end());<br>
<br>
// Build up the list of redeclarations.<br>
RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized, CanonID);<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Serialization_ASTWriter.cpp-3Frev-3D241999-26r1-3D241998-26r2-3D241999-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=P0K9mGk31ljPF50voOxt73tmL9C6j6kfy9tBPgrd2PA&s=L7j7Pf7H70gg4d9Y3gYcMSD0foUeFyAyaVzwV4ljU7M&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=241999&r1=241998&r2=241999&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Sun Jul 12 18:43:21 2015<br>
@@ -3634,6 +3634,55 @@ ASTWriter::GenerateNameLookupTable(const<br>
/// bitstream, or 0 if no block was written.<br>
uint64_t ASTWriter::WriteDeclContextVisibleBlock(ASTContext &Context,<br>
DeclContext *DC) {<br>
+ // If we imported a key declaration of this namespace, write the visible<br>
+ // lookup results as an update record for it rather than including them<br>
+ // on this declaration. We will only look at key declarations on reload.<br>
+ if (isa<NamespaceDecl>(DC) && Chain &&<br>
+ Chain->getKeyDeclaration(cast<Decl>(DC))->isFromASTFile()) {<br>
+ // Only do this once, for the first local declaration of the namespace.<br>
+ for (NamespaceDecl *Prev = cast<NamespaceDecl>(DC)->getPreviousDecl(); Prev;<br>
+ Prev = Prev->getPreviousDecl())<br>
+ if (!Prev->isFromASTFile())<br>
+ return 0;<br>
+<br>
+ // Note that we need to emit an update record for the primary context.<br>
+ UpdatedDeclContexts.insert(DC->getPrimaryContext());<br>
+<br>
+ // Make sure all visible decls are written. They will be recorded later. We<br>
+ // do this using a side data structure so we can sort the names into<br>
+ // a deterministic order.<br>
+ StoredDeclsMap *Map = DC->getPrimaryContext()->buildLookup();<br>
+ SmallVector<std::pair<DeclarationName, DeclContext::lookup_result>, 16><br>
+ LookupResults;<br>
+ if (Map) {<br>
+ LookupResults.reserve(Map->size());<br>
+ for (auto &Entry : *Map)<br>
+ LookupResults.push_back(<br>
+ std::make_pair(Entry.first, Entry.second.getLookupResult()));<br>
+ }<br>
+<br>
+ std::sort(LookupResults.begin(), LookupResults.end(), llvm::less_first());<br>
+ for (auto &NameAndResult : LookupResults) {<br>
+ DeclarationName Name = NameAndResult.first;<br>
+ DeclContext::lookup_result Result = NameAndResult.second;<br>
+ if (Name.getNameKind() == DeclarationName::CXXConstructorName ||<br>
+ Name.getNameKind() == DeclarationName::CXXConversionFunctionName) {<br>
+ // We have to work around a name lookup bug here where negative lookup<br>
+ // results for these names get cached in namespace lookup tables (these<br>
+ // names should never be looked up in a namespace).<br>
+ assert(Result.empty() && "Cannot have a constructor or conversion "<br>
+ "function name in a namespace!");<br>
+ continue;<br>
+ }<br>
+<br>
+ for (NamedDecl *ND : Result)<br>
+ if (!ND->isFromASTFile())<br>
+ GetDeclRef(ND);<br>
+ }<br>
+<br>
+ return 0;<br>
+ }<br>
+<br>
if (DC->getPrimaryContext() != DC)<br>
return 0;<br>
<br>
@@ -3685,6 +3734,11 @@ void ASTWriter::WriteDeclContextVisibleU<br>
SmallString<4096> LookupTable;<br>
uint32_t BucketOffset = GenerateNameLookupTable(DC, LookupTable);<br>
<br>
+ // If we're updating a namespace, select a key declaration as the key for the<br>
+ // update record; those are the only ones that will be checked on reload.<br>
+ if (isa<NamespaceDecl>(DC))<br>
+ DC = cast<DeclContext>(Chain->getKeyDeclaration(cast<Decl>(DC)));<br>
+<br>
// Write the lookup table<br>
RecordData Record;<br>
Record.push_back(UPDATE_VISIBLE);<br>
@@ -3717,11 +3771,17 @@ void ASTWriter::WriteRedeclarations() {<br>
SmallVector<serialization::LocalRedeclarationsInfo, 2> LocalRedeclsMap;<br>
<br>
for (unsigned I = 0, N = Redeclarations.size(); I != N; ++I) {<br>
- Decl *First = Redeclarations[I];<br>
- assert(First->isFirstDecl() && "Not the first declaration?");<br>
-<br>
- Decl *MostRecent = First->getMostRecentDecl();<br>
-<br>
+ const Decl *Key = Redeclarations[I];<br>
+ assert((Chain ? Chain->getKeyDeclaration(Key) == Key<br>
+ : Key->isFirstDecl()) &&<br>
+ "not the key declaration");<br>
+<br>
+ const Decl *First = Key->getCanonicalDecl();<br>
+ const Decl *MostRecent = First->getMostRecentDecl();<br>
+<br>
+ assert((getDeclID(First) >= NUM_PREDEF_DECL_IDS || First == Key) &&<br>
+ "should not have imported key decls for predefined decl");<br>
+<br>
// If we only have a single declaration, there is no point in storing<br>
// a redeclaration chain.<br>
if (First == MostRecent)<br>
@@ -3730,34 +3790,34 @@ void ASTWriter::WriteRedeclarations() {<br>
unsigned Offset = LocalRedeclChains.size();<br>
unsigned Size = 0;<br>
LocalRedeclChains.push_back(0); // Placeholder for the size.<br>
-<br>
+<br>
// Collect the set of local redeclarations of this declaration.<br>
- for (Decl *Prev = MostRecent; Prev != First;<br>
+ for (const Decl *Prev = MostRecent; Prev;<br>
Prev = Prev->getPreviousDecl()) {<br>
- if (!Prev->isFromASTFile()) {<br>
+ if (!Prev->isFromASTFile() && Prev != Key) {<br>
AddDeclRef(Prev, LocalRedeclChains);<br>
++Size;<br>
}<br>
}<br>
<br>
LocalRedeclChains[Offset] = Size;<br>
-<br>
+<br>
// Reverse the set of local redeclarations, so that we store them in<br>
// order (since we found them in reverse order).<br>
std::reverse(LocalRedeclChains.end() - Size, LocalRedeclChains.end());<br>
-<br>
+<br>
// Add the mapping from the first ID from the AST to the set of local<br>
// declarations.<br>
- LocalRedeclarationsInfo Info = { getDeclID(First), Offset };<br>
+ LocalRedeclarationsInfo Info = { getDeclID(Key), Offset };<br>
LocalRedeclsMap.push_back(Info);<br>
-<br>
+<br>
assert(N == Redeclarations.size() &&<br>
"Deserialized a declaration we shouldn't have");<br>
}<br>
-<br>
+<br>
if (LocalRedeclChains.empty())<br>
return;<br>
-<br>
+<br>
// Sort the local redeclarations map by the first declaration ID,<br>
// since the reader will be performing binary searches on this information.<br>
llvm::array_pod_sort(LocalRedeclsMap.begin(), LocalRedeclsMap.end());<br>
@@ -4053,25 +4113,27 @@ void ASTWriter::WriteASTCore(Sema &SemaR<br>
Preprocessor &PP = SemaRef.PP;<br>
<br>
// Set up predefined declaration IDs.<br>
- DeclIDs[Context.getTranslationUnitDecl()] = PREDEF_DECL_TRANSLATION_UNIT_ID;<br>
- if (Context.ObjCIdDecl)<br>
- DeclIDs[Context.ObjCIdDecl] = PREDEF_DECL_OBJC_ID_ID;<br>
- if (Context.ObjCSelDecl)<br>
- DeclIDs[Context.ObjCSelDecl] = PREDEF_DECL_OBJC_SEL_ID;<br>
- if (Context.ObjCClassDecl)<br>
- DeclIDs[Context.ObjCClassDecl] = PREDEF_DECL_OBJC_CLASS_ID;<br>
- if (Context.ObjCProtocolClassDecl)<br>
- DeclIDs[Context.ObjCProtocolClassDecl] = PREDEF_DECL_OBJC_PROTOCOL_ID;<br>
- if (Context.Int128Decl)<br>
- DeclIDs[Context.Int128Decl] = PREDEF_DECL_INT_128_ID;<br>
- if (Context.UInt128Decl)<br>
- DeclIDs[Context.UInt128Decl] = PREDEF_DECL_UNSIGNED_INT_128_ID;<br>
- if (Context.ObjCInstanceTypeDecl)<br>
- DeclIDs[Context.ObjCInstanceTypeDecl] = PREDEF_DECL_OBJC_INSTANCETYPE_ID;<br>
- if (Context.BuiltinVaListDecl)<br>
- DeclIDs[Context.getBuiltinVaListDecl()] = PREDEF_DECL_BUILTIN_VA_LIST_ID;<br>
- if (Context.ExternCContext)<br>
- DeclIDs[Context.ExternCContext] = PREDEF_DECL_EXTERN_C_CONTEXT_ID;<br>
+ auto RegisterPredefDecl = [&] (Decl *D, PredefinedDeclIDs ID) {<br>
+ if (D) {<br>
+ assert(D->isCanonicalDecl() && "predefined decl is not canonical");<br>
+ DeclIDs[D] = ID;<br>
+ if (D->getMostRecentDecl() != D)<br>
+ Redeclarations.push_back(D);<br>
+ }<br>
+ };<br>
+ RegisterPredefDecl(Context.getTranslationUnitDecl(),<br>
+ PREDEF_DECL_TRANSLATION_UNIT_ID);<br>
+ RegisterPredefDecl(Context.ObjCIdDecl, PREDEF_DECL_OBJC_ID_ID);<br>
+ RegisterPredefDecl(Context.ObjCSelDecl, PREDEF_DECL_OBJC_SEL_ID);<br>
+ RegisterPredefDecl(Context.ObjCClassDecl, PREDEF_DECL_OBJC_CLASS_ID);<br>
+ RegisterPredefDecl(Context.ObjCProtocolClassDecl,<br>
+ PREDEF_DECL_OBJC_PROTOCOL_ID);<br>
+ RegisterPredefDecl(Context.Int128Decl, PREDEF_DECL_INT_128_ID);<br>
+ RegisterPredefDecl(Context.UInt128Decl, PREDEF_DECL_UNSIGNED_INT_128_ID);<br>
+ RegisterPredefDecl(Context.ObjCInstanceTypeDecl,<br>
+ PREDEF_DECL_OBJC_INSTANCETYPE_ID);<br>
+ RegisterPredefDecl(Context.BuiltinVaListDecl, PREDEF_DECL_BUILTIN_VA_LIST_ID);<br>
+ RegisterPredefDecl(Context.ExternCContext, PREDEF_DECL_EXTERN_C_CONTEXT_ID);<br>
<br>
// Build a record containing all of the tentative definitions in this file, in<br>
// TentativeDefinitions order. Generally, this record will be empty for<br>
@@ -5675,7 +5737,7 @@ void ASTWriter::AddedCXXTemplateSpeciali<br>
void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {<br>
assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");<br>
if (!Chain) return;<br>
- Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) {<br>
+ Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) {<br>
// If we don't already know the exception specification for this redecl<br>
// chain, add an update record for it.<br>
if (isUnresolvedExceptionSpec(cast<FunctionDecl>(D)<br>
@@ -5689,7 +5751,7 @@ void ASTWriter::ResolvedExceptionSpec(co<br>
void ASTWriter::DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) {<br>
assert(!WritingAST && "Already writing the AST!");<br>
if (!Chain) return;<br>
- Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) {<br>
+ Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) {<br>
DeclUpdates[D].push_back(<br>
DeclUpdate(UPD_CXX_DEDUCED_RETURN_TYPE, ReturnType));<br>
});<br>
@@ -5700,7 +5762,7 @@ void ASTWriter::ResolvedOperatorDelete(c<br>
assert(!WritingAST && "Already writing the AST!");<br>
assert(Delete && "Not given an operator delete");<br>
if (!Chain) return;<br>
- Chain->forEachFormerlyCanonicalImportedDecl(DD, [&](const Decl *D) {<br>
+ Chain->forEachImportedKeyDecl(DD, [&](const Decl *D) {<br>
DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_RESOLVED_DTOR_DELETE, Delete));<br>
});<br>
}<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Serialization_ASTWriterDecl.cpp-3Frev-3D241999-26r1-3D241998-26r2-3D241999-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=P0K9mGk31ljPF50voOxt73tmL9C6j6kfy9tBPgrd2PA&s=BXSC0BUwPQh7AUzdstgls9lLecHjzzaPY62bztCFwQ8&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=241999&r1=241998&r2=241999&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Sun Jul 12 18:43:21 2015<br>
@@ -254,6 +254,8 @@ void ASTDeclWriter::VisitDecl(Decl *D) {<br>
//<br>
// This happens when we instantiate a class with a friend declaration or a<br>
// function with a local extern declaration, for instance.<br>
+ //<br>
+ // FIXME: Can we handle this in AddedVisibleDecl instead?<br>
if (D->isOutOfLine()) {<br>
auto *DC = D->getDeclContext();<br>
while (auto *NS = dyn_cast<NamespaceDecl>(DC->getRedeclContext())) {<br>
@@ -1005,42 +1007,6 @@ void ASTDeclWriter::VisitNamespaceDecl(N<br>
Writer.AddDeclRef(D->getAnonymousNamespace(), Record);<br>
Code = serialization::DECL_NAMESPACE;<br>
<br>
- if (Writer.hasChain() && !D->isOriginalNamespace() &&<br>
- D->getOriginalNamespace()->isFromASTFile()) {<br>
- NamespaceDecl *NS = D->getOriginalNamespace();<br>
- Writer.UpdatedDeclContexts.insert(NS);<br>
-<br>
- // Make sure all visible decls are written. They will be recorded later. We<br>
- // do this using a side data structure so we can sort the names into<br>
- // a deterministic order.<br>
- StoredDeclsMap *Map = NS->buildLookup();<br>
- SmallVector<std::pair<DeclarationName, DeclContext::lookup_result>, 16><br>
- LookupResults;<br>
- if (Map) {<br>
- LookupResults.reserve(Map->size());<br>
- for (auto &Entry : *Map)<br>
- LookupResults.push_back(<br>
- std::make_pair(Entry.first, Entry.second.getLookupResult()));<br>
- }<br>
-<br>
- std::sort(LookupResults.begin(), LookupResults.end(), llvm::less_first());<br>
- for (auto &NameAndResult : LookupResults) {<br>
- DeclarationName Name = NameAndResult.first;<br>
- DeclContext::lookup_result Result = NameAndResult.second;<br>
- if (Name.getNameKind() == DeclarationName::CXXConstructorName ||<br>
- Name.getNameKind() == DeclarationName::CXXConversionFunctionName) {<br>
- // We have to work around a name lookup bug here where negative lookup<br>
- // results for these names get cached in namespace lookup tables.<br>
- assert(Result.empty() && "Cannot have a constructor or conversion "<br>
- "function name in a namespace!");<br>
- continue;<br>
- }<br>
-<br>
- for (NamedDecl *ND : Result)<br>
- Writer.GetDeclRef(ND);<br>
- }<br>
- }<br>
-<br>
if (Writer.hasChain() && D->isAnonymousNamespace() &&<br>
D == D->getMostRecentDecl()) {<br>
// This is a most recent reopening of the anonymous namespace. If its parent<br>
@@ -1512,6 +1478,17 @@ void ASTDeclWriter::VisitDeclContext(Dec<br>
Record.push_back(VisibleOffset);<br>
}<br>
<br>
+/// Determine whether D is the first declaration in its redeclaration chain that<br>
+/// is not from an AST file.<br>
+template <typename T><br>
+static bool isFirstLocalDecl(Redeclarable<T> *D) {<br>
+ assert(D && !static_cast<T*>(D)->isFromASTFile());<br>
+ do<br>
+ D = D->getPreviousDecl();<br>
+ while (D && static_cast<T*>(D)->isFromASTFile());<br>
+ return !D;<br>
+}<br>
+<br>
template <typename T><br>
void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {<br>
T *First = D->getFirstDecl();<br>
@@ -1520,41 +1497,30 @@ void ASTDeclWriter::VisitRedeclarable(Re<br>
assert(isRedeclarableDeclKind(static_cast<T *>(D)->getKind()) &&<br>
"Not considered redeclarable?");<br>
<br>
- // There is more than one declaration of this entity, so we will need to<br>
- // write a redeclaration chain.<br>
Writer.AddDeclRef(First, Record);<br>
- Writer.Redeclarations.insert(First);<br>
<br>
- auto *Previous = D->getPreviousDecl();<br>
-<br>
- // In a modules build, we can have imported declarations after a local<br>
- // canonical declaration. If this is the first local declaration, emit<br>
- // a list of all such imported declarations so that we can ensure they<br>
- // are loaded before we are. This allows us to rebuild the redecl chain<br>
- // in the right order on reload (all declarations imported by a module<br>
- // should be before all declarations provided by that module).<br>
- bool EmitImportedMergedCanonicalDecls = false;<br>
+ // In a modules build, emit a list of all imported key declarations<br>
+ // (excluding First, if it was imported), so that we can be sure that all<br>
+ // redeclarations visible to this module are before D in the redecl chain.<br>
+ unsigned I = Record.size();<br>
+ Record.push_back(0);<br>
if (Context.getLangOpts().Modules && Writer.Chain) {<br>
- auto *PreviousLocal = Previous;<br>
- while (PreviousLocal && PreviousLocal->isFromASTFile())<br>
- PreviousLocal = PreviousLocal->getPreviousDecl();<br>
- if (!PreviousLocal)<br>
- EmitImportedMergedCanonicalDecls = true;<br>
+ if (isFirstLocalDecl(D)) {<br>
+ Writer.Chain->forEachImportedKeyDecl(First, [&](const Decl *D) {<br>
+ if (D != First)<br>
+ Writer.AddDeclRef(D, Record);<br>
+ });<br>
+ Record[I] = Record.size() - I - 1;<br>
+<br>
+ // Write a redeclaration chain, attached to the first key decl.<br>
+ Writer.Redeclarations.push_back(Writer.Chain->getKeyDeclaration(First));<br>
+ }<br>
+ } else if (D == First || D->getPreviousDecl()->isFromASTFile()) {<br>
+ assert(isFirstLocalDecl(D) && "imported decl after local decl");<br>
+<br>
+ // Write a redeclaration chain attached to the first decl.<br>
+ Writer.Redeclarations.push_back(First);<br>
}<br>
- if (EmitImportedMergedCanonicalDecls) {<br>
- llvm::SmallMapVector<ModuleFile*, Decl*, 16> FirstInModule;<br>
- for (auto *Redecl = MostRecent; Redecl;<br>
- Redecl = Redecl->getPreviousDecl())<br>
- if (Redecl->isFromASTFile())<br>
- FirstInModule[Writer.Chain->getOwningModuleFile(Redecl)] = Redecl;<br>
- // FIXME: If FirstInModule has entries for modules A and B, and B imports<br>
- // A (directly or indirectly), we don't need to write the entry for A.<br>
- Record.push_back(FirstInModule.size());<br>
- for (auto I = FirstInModule.rbegin(), E = FirstInModule.rend();<br>
- I != E; ++I)<br>
- Writer.AddDeclRef(I->second, Record);<br>
- } else<br>
- Record.push_back(0);<br>
<br>
// Make sure that we serialize both the previous and the most-recent<br>
// declarations, which (transitively) ensures that all declarations in the<br>
@@ -1562,7 +1528,7 @@ void ASTDeclWriter::VisitRedeclarable(Re<br>
//<br>
// FIXME: This is not correct; when we reach an imported declaration we<br>
// won't emit its previous declaration.<br>
- (void)Writer.GetDeclRef(Previous);<br>
+ (void)Writer.GetDeclRef(D->getPreviousDecl());<br>
(void)Writer.GetDeclRef(MostRecent);<br>
} else {<br>
// We use the sentinel value 0 to indicate an only declaration.<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>