<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Aug 27, 2015 at 10:47 AM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Richard Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> writes:<br>
> Author: rsmith<br>
> Date: Fri Aug 21 20:47:18 2015<br>
> New Revision: 245779<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=245779&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=245779&view=rev</a><br>
> Log:<br>
> [modules] Rearrange how redeclaration chains are loaded, to remove a walk over<br>
> all modules and reduce the number of declarations we load when loading a<br>
> redeclaration chain.<br>
<br>
</span>It looks like ASTDeclWriter::AddFirstDeclFromEachModule is called with a<br>
null `this` since this change, which has been causing ubsan failures on<br>
76 tests since it went in:<br>
<br>
ASTWriterDecl.cpp:170:18: runtime error: member call on null pointer of type 'clang::ASTReader'<br>
<br>
Logs and other failures here:<br>
<br>
<a href="http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/146/" rel="noreferrer" target="_blank">http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/146/</a><br>
<br>
I guess you must've missed the failure email. Could you please take a<br>
look?</blockquote><div><br></div><div>Thanks, fixed in r246215.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> The new approach is:<br>
> * when loading the first declaration of an entity within a module file, we<br>
> first load all declarations of the entity that were imported into that<br>
> module file, and then load all the other declarations of that entity from<br>
> that module file and build a suitable decl chain from them<br>
> * when loading any other declaration of an entity, we first load the first<br>
> declaration from the same module file<br>
><br>
> As before, we complete redecl chains through name lookup where necessary.<br>
><br>
> To make this work, I also had to change the way that template specializations<br>
> are stored -- it no longer suffices to track only canonical specializations; we<br>
> now emit all "first local" declarations when emitting a list of specializations<br>
> for a template.<br>
><br>
> On one testcase with several thousand imported module files, this reduces the<br>
> total runtime by 72%.<br>
><br>
> Modified:<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>
> cfe/trunk/test/Modules/cxx-templates.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=245779&r1=245778&r2=245779&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)<br>
> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Fri Aug 21 20:47:18 2015<br>
> @@ -405,6 +405,10 @@ private:<br>
> /// \brief The set of declarations that may have redeclaration chains that<br>
> /// need to be serialized.<br>
> llvm::SmallVector<const Decl *, 16> Redeclarations;<br>
> +<br>
> + /// \brief A cache of the first local declaration for "interesting"<br>
> + /// redeclaration chains.<br>
> + llvm::DenseMap<const Decl *, const Decl *> FirstLocalDeclCache;<br>
><br>
> /// \brief Statements that we've encountered while serializing a<br>
> /// declaration or type.<br>
> @@ -676,6 +680,10 @@ public:<br>
> const ASTTemplateArgumentListInfo *ASTTemplArgList,<br>
> RecordDataImpl &Record);<br>
><br>
> + /// \brief Find the first local declaration of a given local redeclarable<br>
> + /// decl.<br>
> + const Decl *getFirstLocalDecl(const Decl *D);<br>
> +<br>
> /// \brief Emit a reference to a declaration.<br>
> void AddDeclRef(const Decl *D, RecordDataImpl &Record);<br>
><br>
> @@ -857,12 +865,6 @@ public:<br>
> void CompletedTagDefinition(const TagDecl *D) override;<br>
> void AddedVisibleDecl(const DeclContext *DC, const Decl *D) override;<br>
> void AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D) override;<br>
> - void AddedCXXTemplateSpecialization(const ClassTemplateDecl *TD,<br>
> - const ClassTemplateSpecializationDecl *D) override;<br>
> - void AddedCXXTemplateSpecialization(const VarTemplateDecl *TD,<br>
> - const VarTemplateSpecializationDecl *D) override;<br>
> - void AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,<br>
> - const FunctionDecl *D) override;<br>
> void ResolvedExceptionSpec(const FunctionDecl *FD) override;<br>
> void DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) override;<br>
> void ResolvedOperatorDelete(const CXXDestructorDecl *DD,<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Aug 21 20:47:18 2015<br>
> @@ -8123,11 +8123,8 @@ void ASTReader::finishPendingActions() {<br>
> PendingIncompleteDeclChains.clear();<br>
><br>
> // Load pending declaration chains.<br>
> - for (unsigned I = 0; I != PendingDeclChains.size(); ++I) {<br>
> - PendingDeclChainsKnown.erase(PendingDeclChains[I]);<br>
> + for (unsigned I = 0; I != PendingDeclChains.size(); ++I)<br>
> loadPendingDeclChain(PendingDeclChains[I]);<br>
> - }<br>
> - assert(PendingDeclChainsKnown.empty());<br>
> PendingDeclChains.clear();<br>
><br>
> assert(RedeclsDeserialized.empty() && "some redecls not wired up");<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Aug 21 20:47:18 2015<br>
> @@ -147,12 +147,6 @@ namespace clang {<br>
> }<br>
><br>
> ~RedeclarableResult() {<br>
> - if (FirstID && Owning &&<br>
> - isRedeclarableDeclKind(LoadedDecl->getKind())) {<br>
> - auto Canon = Reader.GetDecl(FirstID)->getCanonicalDecl();<br>
> - if (Reader.PendingDeclChainsKnown.insert(Canon).second)<br>
> - Reader.PendingDeclChains.push_back(Canon);<br>
> - }<br>
> }<br>
><br>
> /// \brief Note that a RedeclarableDecl is not actually redeclarable.<br>
> @@ -2186,23 +2180,33 @@ ASTDeclReader::RedeclarableResult<br>
> ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {<br>
> DeclID FirstDeclID = ReadDeclID(Record, Idx);<br>
> Decl *MergeWith = nullptr;<br>
> +<br>
> bool IsKeyDecl = ThisDeclID == FirstDeclID;<br>
> + bool IsFirstLocalDecl = false;<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>
> FirstDeclID = ThisDeclID;<br>
> IsKeyDecl = true;<br>
> + IsFirstLocalDecl = true;<br>
> } else if (unsigned N = Record[Idx++]) {<br>
> - IsKeyDecl = false;<br>
> + // This declaration was the first local declaration, but may have imported<br>
> + // other declarations.<br>
> + IsKeyDecl = N == 1;<br>
> + IsFirstLocalDecl = true;<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>
> // FIXME: Provide a known merge target to the second and subsequent such<br>
> // declaration.<br>
> - for (unsigned I = 0; I != N; ++I)<br>
> + for (unsigned I = 0; I != N - 1; ++I)<br>
> MergeWith = ReadDecl(Record, Idx/*, MergeWith*/);<br>
> + } else {<br>
> + // This declaration was not the first local declaration. Read the first<br>
> + // local declaration now, to trigger the import of other redeclarations.<br>
> + (void)ReadDecl(Record, Idx);<br>
> }<br>
><br>
> T *FirstDecl = cast_or_null<T>(Reader.GetDecl(FirstDeclID));<br>
> @@ -2214,13 +2218,21 @@ ASTDeclReader::VisitRedeclarable(Redecla<br>
> D->RedeclLink = Redeclarable<T>::PreviousDeclLink(FirstDecl);<br>
> D->First = FirstDecl->getCanonicalDecl();<br>
> }<br>
> -<br>
> +<br>
> // Note that this declaration has been deserialized.<br>
> - Reader.RedeclsDeserialized.insert(static_cast<T *>(D));<br>
> -<br>
> + T *DAsT = static_cast<T*>(D);<br>
> + Reader.RedeclsDeserialized.insert(DAsT);<br>
> +<br>
> + // Note that we need to load local redeclarations of this decl and build a<br>
> + // decl chain for them. This must happen *after* we perform the preloading<br>
> + // above; this ensures that the redeclaration chain is built in the correct<br>
> + // order.<br>
> + if (IsFirstLocalDecl)<br>
> + Reader.PendingDeclChains.push_back(DAsT);<br>
> +<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, static_cast<T *>(D), MergeWith,<br>
> + return RedeclarableResult(Reader, FirstDeclID, DAsT, MergeWith,<br>
> IsKeyDecl);<br>
> }<br>
><br>
> @@ -2330,11 +2342,8 @@ void ASTDeclReader::mergeRedeclarable(Re<br>
> TemplatePatternID, Redecl.isKeyDecl());<br>
><br>
> // If this declaration is a key declaration, make a note of that.<br>
> - if (Redecl.isKeyDecl()) {<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>
> }<br>
> }<br>
><br>
> @@ -3424,15 +3433,12 @@ namespace {<br>
> ASTReader &Reader;<br>
> SmallVectorImpl<DeclID> &SearchDecls;<br>
> llvm::SmallPtrSetImpl<Decl *> &Deserialized;<br>
> - GlobalDeclID CanonID;<br>
> SmallVector<Decl *, 4> Chain;<br>
><br>
> public:<br>
> RedeclChainVisitor(ASTReader &Reader, SmallVectorImpl<DeclID> &SearchDecls,<br>
> - llvm::SmallPtrSetImpl<Decl *> &Deserialized,<br>
> - GlobalDeclID CanonID)<br>
> - : Reader(Reader), SearchDecls(SearchDecls), Deserialized(Deserialized),<br>
> - CanonID(CanonID) {<br>
> + llvm::SmallPtrSetImpl<Decl *> &Deserialized)<br>
> + : Reader(Reader), SearchDecls(SearchDecls), Deserialized(Deserialized) {<br>
> assert(std::is_sorted(SearchDecls.begin(), SearchDecls.end()));<br>
> }<br>
><br>
> @@ -3518,10 +3524,10 @@ namespace {<br>
> break;<br>
> }<br>
><br>
> - if (LocalSearchDeclID && LocalSearchDeclID != CanonID) {<br>
> + assert(LocalSearchDeclID);<br>
> + if (LocalSearchDeclID) {<br>
> // If the search decl was from this module, add it to the chain.<br>
> // Note, the chain is sorted from newest to oldest, so this goes last.<br>
> - // We exclude the canonical declaration; it implicitly goes at the end.<br>
> addToChain(Reader.GetDecl(LocalSearchDeclID));<br>
> }<br>
><br>
> @@ -3531,26 +3537,14 @@ namespace {<br>
> };<br>
> }<br>
><br>
> -void ASTReader::loadPendingDeclChain(Decl *CanonDecl) {<br>
> - // The decl might have been merged into something else after being added to<br>
> - // our list. If it was, just skip it.<br>
> - if (!CanonDecl->isCanonicalDecl())<br>
> - return;<br>
> -<br>
> +void ASTReader::loadPendingDeclChain(Decl *FirstLocal) {<br>
> // Determine the set of declaration IDs we'll be searching for.<br>
> - SmallVector<DeclID, 16> SearchDecls;<br>
> - GlobalDeclID CanonID = CanonDecl->getGlobalID();<br>
> - if (CanonID)<br>
> - SearchDecls.push_back(CanonDecl->getGlobalID()); // Always first.<br>
> - KeyDeclsMap::iterator KeyPos = KeyDecls.find(CanonDecl);<br>
> - if (KeyPos != KeyDecls.end())<br>
> - SearchDecls.append(KeyPos->second.begin(), KeyPos->second.end());<br>
> - llvm::array_pod_sort(SearchDecls.begin(), SearchDecls.end());<br>
> + SmallVector<DeclID, 1> SearchDecls;<br>
> + SearchDecls.push_back(FirstLocal->getGlobalID());<br>
><br>
> // Build up the list of redeclarations.<br>
> - RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized, CanonID);<br>
> - ModuleMgr.visit(Visitor);<br>
> - RedeclsDeserialized.erase(CanonDecl);<br>
> + RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized);<br>
> + Visitor(*getOwningModuleFile(FirstLocal));<br>
><br>
> // Retrieve the chains.<br>
> ArrayRef<Decl *> Chain = Visitor.getChain();<br>
> @@ -3561,11 +3555,14 @@ void ASTReader::loadPendingDeclChain(Dec<br>
> //<br>
> // FIXME: We have three different dispatches on decl kind here; maybe<br>
> // we should instead generate one loop per kind and dispatch up-front?<br>
> + Decl *CanonDecl = FirstLocal->getCanonicalDecl();<br>
> Decl *MostRecent = ASTDeclReader::getMostRecentDecl(CanonDecl);<br>
> if (!MostRecent)<br>
> MostRecent = CanonDecl;<br>
> for (unsigned I = 0, N = Chain.size(); I != N; ++I) {<br>
> auto *D = Chain[N - I - 1];<br>
> + if (D == CanonDecl)<br>
> + continue;<br>
> ASTDeclReader::attachPreviousDecl(*this, D, MostRecent, CanonDecl);<br>
> MostRecent = D;<br>
> }<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Aug 21 20:47:18 2015<br>
> @@ -3799,41 +3799,39 @@ void ASTWriter::WriteRedeclarations() {<br>
> SmallVector<serialization::LocalRedeclarationsInfo, 2> LocalRedeclsMap;<br>
><br>
> for (unsigned I = 0, N = Redeclarations.size(); I != N; ++I) {<br>
> - const Decl *Key = Redeclarations[I];<br>
> - assert((Chain ? Chain->getKeyDeclaration(Key) == Key<br>
> - : Key->isFirstDecl()) &&<br>
> - "not the key declaration");<br>
> + const Decl *FirstLocal = Redeclarations[I];<br>
> + assert(!FirstLocal->isFromASTFile() &&<br>
> + (!FirstLocal->getPreviousDecl() ||<br>
> + FirstLocal->getPreviousDecl()->isFromASTFile() ||<br>
> + getDeclID(FirstLocal->getPreviousDecl()) < NUM_PREDEF_DECL_IDS) &&<br>
> + "not the first local declaration");<br>
> + assert(getDeclID(FirstLocal) >= NUM_PREDEF_DECL_IDS &&<br>
> + "should not have predefined decl as first decl");<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>
> - continue;<br>
> -<br>
> unsigned Offset = LocalRedeclChains.size();<br>
> unsigned Size = 0;<br>
> LocalRedeclChains.push_back(0); // Placeholder for the size.<br>
><br>
> // Collect the set of local redeclarations of this declaration, from newest<br>
> // to oldest.<br>
> - for (const Decl *Prev = MostRecent; Prev;<br>
> - Prev = Prev->getPreviousDecl()) {<br>
> - if (!Prev->isFromASTFile() && Prev != Key) {<br>
> + for (const Decl *Prev = FirstLocal->getMostRecentDecl(); Prev != FirstLocal;<br>
> + Prev = Prev->getPreviousDecl()) {<br>
> + if (!Prev->isFromASTFile()) {<br>
> AddDeclRef(Prev, LocalRedeclChains);<br>
> ++Size;<br>
> }<br>
> }<br>
><br>
> + // If we only have a single local declaration, there is no point in storing<br>
> + // a redeclaration chain.<br>
> + if (LocalRedeclChains.size() == 1)<br>
> + continue;<br>
> +<br>
> LocalRedeclChains[Offset] = Size;<br>
><br>
> // Add the mapping from the first ID from the AST to the set of local<br>
> // declarations.<br>
> - LocalRedeclarationsInfo Info = { getDeclID(Key), Offset };<br>
> + LocalRedeclarationsInfo Info = { getDeclID(FirstLocal), Offset };<br>
> LocalRedeclsMap.push_back(Info);<br>
><br>
> assert(N == Redeclarations.size() &&<br>
> @@ -4145,8 +4143,6 @@ void ASTWriter::WriteASTCore(Sema &SemaR<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>
> @@ -5730,42 +5726,6 @@ void ASTWriter::AddedCXXImplicitMember(c<br>
> DeclUpdates[RD].push_back(DeclUpdate(UPD_CXX_ADDED_IMPLICIT_MEMBER, D));<br>
> }<br>
><br>
> -void ASTWriter::AddedCXXTemplateSpecialization(const ClassTemplateDecl *TD,<br>
> - const ClassTemplateSpecializationDecl *D) {<br>
> - // The specializations set is kept in the canonical template.<br>
> - TD = TD->getCanonicalDecl();<br>
> - if (!(!D->isFromASTFile() && TD->isFromASTFile()))<br>
> - return; // Not a source specialization added to a template from PCH.<br>
> -<br>
> - assert(!WritingAST && "Already writing the AST!");<br>
> - DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
> - D));<br>
> -}<br>
> -<br>
> -void ASTWriter::AddedCXXTemplateSpecialization(<br>
> - const VarTemplateDecl *TD, const VarTemplateSpecializationDecl *D) {<br>
> - // The specializations set is kept in the canonical template.<br>
> - TD = TD->getCanonicalDecl();<br>
> - if (!(!D->isFromASTFile() && TD->isFromASTFile()))<br>
> - return; // Not a source specialization added to a template from PCH.<br>
> -<br>
> - assert(!WritingAST && "Already writing the AST!");<br>
> - DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
> - D));<br>
> -}<br>
> -<br>
> -void ASTWriter::AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,<br>
> - const FunctionDecl *D) {<br>
> - // The specializations set is kept in the canonical template.<br>
> - TD = TD->getCanonicalDecl();<br>
> - if (!(!D->isFromASTFile() && TD->isFromASTFile()))<br>
> - return; // Not a source specialization added to a template from PCH.<br>
> -<br>
> - assert(!WritingAST && "Already writing the AST!");<br>
> - DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
> - D));<br>
> -}<br>
> -<br>
> void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {<br>
> assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");<br>
> if (!Chain) return;<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Fri Aug 21 20:47:18 2015<br>
> @@ -159,6 +159,19 @@ namespace clang {<br>
> Writer.AddStmt(FD->getBody());<br>
> }<br>
><br>
> + /// Add to the record the first declaration from each module file that<br>
> + /// provides a declaration of D. The intent is to provide a sufficient<br>
> + /// set such that reloading this set will load all current redeclarations.<br>
> + void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {<br>
> + llvm::MapVector<ModuleFile*, const Decl*> Firsts;<br>
> + // FIXME: We can skip entries that we know are implied by others.<br>
> + for (const Decl *R = D->getMostRecentDecl(); R; R = R->getPreviousDecl())<br>
> + if (IncludeLocal || R->isFromASTFile())<br>
> + Firsts[Writer.Chain->getOwningModuleFile(R)] = R;<br>
> + for (const auto &F : Firsts)<br>
> + Writer.AddDeclRef(F.second, Record);<br>
> + }<br>
> +<br>
> /// Get the specialization decl from an entry in the specialization list.<br>
> template <typename EntryType><br>
> typename RedeclarableTemplateDecl::SpecEntryTraits<EntryType>::DeclType *<br>
> @@ -194,20 +207,46 @@ namespace clang {<br>
> if (auto *LS = Common->LazySpecializations)<br>
> LazySpecializations = ArrayRef<DeclID>(LS + 1, LS + 1 + LS[0]);<br>
><br>
> - Record.push_back(Specializations.size() +<br>
> - PartialSpecializations.size() +<br>
> - LazySpecializations.size());<br>
> + // Add a slot to the record for the number of specializations.<br>
> + unsigned I = Record.size();<br>
> + Record.push_back(0);<br>
> +<br>
> for (auto &Entry : Specializations) {<br>
> auto *D = getSpecializationDecl(Entry);<br>
> assert(D->isCanonicalDecl() && "non-canonical decl in set");<br>
> - Writer.AddDeclRef(D, Record);<br>
> + AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);<br>
> }<br>
> for (auto &Entry : PartialSpecializations) {<br>
> auto *D = getSpecializationDecl(Entry);<br>
> assert(D->isCanonicalDecl() && "non-canonical decl in set");<br>
> - Writer.AddDeclRef(D, Record);<br>
> + AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);<br>
> }<br>
> Record.append(LazySpecializations.begin(), LazySpecializations.end());<br>
> +<br>
> + // Update the size entry we added earlier.<br>
> + Record[I] = Record.size() - I - 1;<br>
> + }<br>
> +<br>
> + /// Ensure that this template specialization is associated with the specified<br>
> + /// template on reload.<br>
> + void RegisterTemplateSpecialization(const Decl *Template,<br>
> + const Decl *Specialization) {<br>
> + Template = Template->getCanonicalDecl();<br>
> +<br>
> + // If the canonical template is local, we'll write out this specialization<br>
> + // when we emit it.<br>
> + // FIXME: We can do the same thing if there is any local declaration of<br>
> + // the template, to avoid emitting an update record.<br>
> + if (!Template->isFromASTFile())<br>
> + return;<br>
> +<br>
> + // We only need to associate the first local declaration of the<br>
> + // specialization. The other declarations will get pulled in by it.<br>
> + if (Writer.getFirstLocalDecl(Specialization) != Specialization)<br>
> + return;<br>
> +<br>
> + Writer.DeclUpdates[Template].push_back(ASTWriter::DeclUpdate(<br>
> + UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION, Specialization));<br>
> }<br>
> };<br>
> }<br>
> @@ -479,6 +518,9 @@ void ASTDeclWriter::VisitFunctionDecl(Fu<br>
> case FunctionDecl::TK_FunctionTemplateSpecialization: {<br>
> FunctionTemplateSpecializationInfo *<br>
> FTSInfo = D->getTemplateSpecializationInfo();<br>
> +<br>
> + RegisterTemplateSpecialization(FTSInfo->getTemplate(), D);<br>
> +<br>
> Writer.AddDeclRef(FTSInfo->getTemplate(), Record);<br>
> Record.push_back(FTSInfo->getTemplateSpecializationKind());<br>
><br>
> @@ -1249,6 +1291,8 @@ void ASTDeclWriter::VisitClassTemplateDe<br>
><br>
> void ASTDeclWriter::VisitClassTemplateSpecializationDecl(<br>
> ClassTemplateSpecializationDecl *D) {<br>
> + RegisterTemplateSpecialization(D->getSpecializedTemplate(), D);<br>
> +<br>
> VisitCXXRecordDecl(D);<br>
><br>
> llvm::PointerUnion<ClassTemplateDecl *,<br>
> @@ -1308,6 +1352,8 @@ void ASTDeclWriter::VisitVarTemplateDecl<br>
><br>
> void ASTDeclWriter::VisitVarTemplateSpecializationDecl(<br>
> VarTemplateSpecializationDecl *D) {<br>
> + RegisterTemplateSpecialization(D->getSpecializedTemplate(), D);<br>
> +<br>
> VisitVarDecl(D);<br>
><br>
> llvm::PointerUnion<VarTemplateDecl *, VarTemplatePartialSpecializationDecl *><br>
> @@ -1478,48 +1524,61 @@ 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>
> +/// \brief Is this a local declaration (that is, one that will be written to<br>
> +/// our AST file)? This is the case for declarations that are neither imported<br>
> +/// from another AST file nor predefined.<br>
> +static bool isLocalDecl(ASTWriter &W, const Decl *D) {<br>
> + if (D->isFromASTFile())<br>
> + return false;<br>
> + return W.getDeclID(D) >= NUM_PREDEF_DECL_IDS;<br>
> +}<br>
> +<br>
> +const Decl *ASTWriter::getFirstLocalDecl(const Decl *D) {<br>
> + assert(isLocalDecl(*this, D) && "expected a local declaration");<br>
> +<br>
> + const Decl *Canon = D->getCanonicalDecl();<br>
> + if (isLocalDecl(*this, Canon))<br>
> + return Canon;<br>
> +<br>
> + const Decl *&CacheEntry = FirstLocalDeclCache[Canon];<br>
> + if (CacheEntry)<br>
> + return CacheEntry;<br>
> +<br>
> + for (const Decl *Redecl = D; Redecl; Redecl = Redecl->getPreviousDecl())<br>
> + if (isLocalDecl(*this, Redecl))<br>
> + D = Redecl;<br>
> + return CacheEntry = D;<br>
> }<br>
><br>
> template <typename T><br>
> void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {<br>
> T *First = D->getFirstDecl();<br>
> T *MostRecent = First->getMostRecentDecl();<br>
> + T *DAsT = static_cast<T *>(D);<br>
> if (MostRecent != First) {<br>
> - assert(isRedeclarableDeclKind(static_cast<T *>(D)->getKind()) &&<br>
> + assert(isRedeclarableDeclKind(DAsT->getKind()) &&<br>
> "Not considered redeclarable?");<br>
><br>
> Writer.AddDeclRef(First, Record);<br>
><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>
> - 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>
> + // Write out a list of local redeclarations of this declaration if it's the<br>
> + // first local declaration in the chain.<br>
> + const Decl *FirstLocal = Writer.getFirstLocalDecl(DAsT);<br>
> + if (DAsT == FirstLocal) {<br>
> + Writer.Redeclarations.push_back(DAsT);<br>
> +<br>
> + // Emit a list of all imported first declarations so that we can be sure<br>
> + // that all redeclarations visible to this module are before D in the<br>
> + // redecl chain.<br>
> + unsigned I = Record.size();<br>
> + Record.push_back(0);<br>
> + if (Writer.Chain)<br>
> + AddFirstDeclFromEachModule(DAsT, /*IncludeLocal*/false);<br>
> + // This is the number of imported first declarations + 1.<br>
> + Record[I] = Record.size() - I;<br>
> + } else {<br>
> + Record.push_back(0);<br>
> + Writer.AddDeclRef(FirstLocal, Record);<br>
> }<br>
><br>
> // Make sure that we serialize both the previous and the most-recent<br>
><br>
> Modified: cfe/trunk/test/Modules/cxx-templates.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/cxx-templates.cpp (original)<br>
> +++ cfe/trunk/test/Modules/cxx-templates.cpp Fri Aug 21 20:47:18 2015<br>
> @@ -187,10 +187,10 @@ namespace Std {<br>
><br>
> // CHECK-DUMP: ClassTemplateDecl {{.*}} <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}> col:{{.*}} in cxx_templates_common SomeTemplate<br>
> // CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate<br>
> -// CHECK-DUMP-NEXT: TemplateArgument type 'char [2]'<br>
> -// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition<br>
> -// CHECK-DUMP-NEXT: TemplateArgument type 'char [2]'<br>
> -// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate<br>
> // CHECK-DUMP-NEXT: TemplateArgument type 'char [1]'<br>
> // CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition<br>
> // CHECK-DUMP-NEXT: TemplateArgument type 'char [1]'<br>
> +// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate<br>
> +// CHECK-DUMP-NEXT: TemplateArgument type 'char [2]'<br>
> +// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition<br>
> +// CHECK-DUMP-NEXT: TemplateArgument type 'char [2]'<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>