From: Richard Smith Date: Sun, 12 Jul 2015 23:43:21 +0000 (+0000) Subject: [modules] Improve performance when there is a local declaration of an entity X-Git-Tag: llvmorg-3.7.0-rc1~233 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5fc18a9a1f8930dbf44b2c1b8f53e14f3142659c;p=platform%2Fupstream%2Fllvm.git [modules] Improve performance when there is a local declaration of an entity before the first imported declaration. 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. llvm-svn: 241999 --- diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index ef5b107..6fa7ffa 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -976,12 +976,14 @@ private: MergedLookups; typedef llvm::DenseMap > - MergedDeclsMap; + KeyDeclsMap; - /// \brief A mapping from canonical declarations to the set of additional - /// (global, previously-canonical) declaration IDs that have been merged with - /// that canonical declaration. - MergedDeclsMap MergedDecls; + /// \brief A mapping from canonical declarations to the set of global + /// declaration IDs for key declaration that have been merged with that + /// canonical declaration. A key declaration is a formerly-canonical + /// declaration whose module did not import any other key declaration for that + /// entity. These are the IDs that we use as keys when finding redecl chains. + KeyDeclsMap KeyDecls; /// \brief A mapping from DeclContexts to the semantic DeclContext that we /// are treating as the definition of the entity. This is used, for instance, @@ -1054,6 +1056,36 @@ public: void ResolveImportedPath(ModuleFile &M, std::string &Filename); static void ResolveImportedPath(std::string &Filename, StringRef Prefix); + /// \brief Returns the first key declaration for the given declaration. This + /// is one that is formerly-canonical (or still canonical) and whose module + /// did not import any other key declaration of the entity. + Decl *getKeyDeclaration(Decl *D) { + D = D->getCanonicalDecl(); + if (D->isFromASTFile()) + return D; + + auto I = KeyDecls.find(D); + if (I == KeyDecls.end() || I->second.empty()) + return D; + return GetExistingDecl(I->second[0]); + } + const Decl *getKeyDeclaration(const Decl *D) { + return getKeyDeclaration(const_cast(D)); + } + + /// \brief Run a callback on each imported key declaration of \p D. + template + void forEachImportedKeyDecl(const Decl *D, Fn Visit) { + D = D->getCanonicalDecl(); + if (D->isFromASTFile()) + Visit(D); + + auto It = KeyDecls.find(const_cast(D)); + if (It != KeyDecls.end()) + for (auto ID : It->second) + Visit(GetExistingDecl(ID)); + } + private: struct ImportedModule { ModuleFile *Mod; @@ -1124,18 +1156,6 @@ private: /// merged into its redecl chain. Decl *getMostRecentExistingDecl(Decl *D); - template - void forEachFormerlyCanonicalImportedDecl(const Decl *D, Fn Visit) { - D = D->getCanonicalDecl(); - if (D->isFromASTFile()) - Visit(D); - - auto It = MergedDecls.find(const_cast(D)); - if (It != MergedDecls.end()) - for (auto ID : It->second) - Visit(GetExistingDecl(ID)); - } - RecordLocation DeclCursorForID(serialization::DeclID ID, unsigned &RawLocation); void loadDeclUpdateRecords(serialization::DeclID ID, Decl *D); diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index c966d3e..e830fdc 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -398,7 +398,7 @@ private: /// \brief The set of declarations that may have redeclaration chains that /// need to be serialized. - llvm::SmallSetVector Redeclarations; + llvm::SmallVector Redeclarations; /// \brief Statements that we've encountered while serializing a /// declaration or type. diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 3ba1383..80b5c77d 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -6071,7 +6071,7 @@ Decl *ASTReader::GetExistingDecl(DeclID ID) { if (D) { // Track that we have merged the declaration with ID \p ID into the // pre-existing predefined declaration \p D. - auto &Merged = MergedDecls[D->getCanonicalDecl()]; + auto &Merged = KeyDecls[D->getCanonicalDecl()]; if (Merged.empty()) Merged.push_back(ID); } @@ -6413,10 +6413,10 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC, Contexts.push_back(DC); if (DC->isNamespace()) { - auto Merged = MergedDecls.find(const_cast(cast(DC))); - if (Merged != MergedDecls.end()) { - for (unsigned I = 0, N = Merged->second.size(); I != N; ++I) - Contexts.push_back(cast(GetDecl(Merged->second[I]))); + auto Key = KeyDecls.find(const_cast(cast(DC))); + if (Key != KeyDecls.end()) { + for (unsigned I = 0, N = Key->second.size(); I != N; ++I) + Contexts.push_back(cast(GetDecl(Key->second[I]))); } } @@ -6533,11 +6533,11 @@ void ASTReader::completeVisibleDeclsMap(const DeclContext *DC) { Contexts.push_back(DC); if (DC->isNamespace()) { - MergedDeclsMap::iterator Merged - = MergedDecls.find(const_cast(cast(DC))); - if (Merged != MergedDecls.end()) { - for (unsigned I = 0, N = Merged->second.size(); I != N; ++I) - Contexts.push_back(cast(GetDecl(Merged->second[I]))); + KeyDeclsMap::iterator Key = + KeyDecls.find(const_cast(cast(DC))); + if (Key != KeyDecls.end()) { + for (unsigned I = 0, N = Key->second.size(); I != N; ++I) + Contexts.push_back(cast(GetDecl(Key->second[I]))); } } diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 52019fc..0c59ba8 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -128,20 +128,22 @@ namespace clang { GlobalDeclID FirstID; Decl *MergeWith; mutable bool Owning; + bool IsKeyDecl; Decl::Kind DeclKind; void operator=(RedeclarableResult &) = delete; public: RedeclarableResult(ASTReader &Reader, GlobalDeclID FirstID, - Decl *MergeWith, Decl::Kind DeclKind) + Decl *MergeWith, Decl::Kind DeclKind, + bool IsKeyDecl) : Reader(Reader), FirstID(FirstID), MergeWith(MergeWith), - Owning(true), DeclKind(DeclKind) {} + Owning(true), IsKeyDecl(IsKeyDecl), DeclKind(DeclKind) {} RedeclarableResult(RedeclarableResult &&Other) : Reader(Other.Reader), FirstID(Other.FirstID), MergeWith(Other.MergeWith), Owning(Other.Owning), - DeclKind(Other.DeclKind) { + IsKeyDecl(Other.IsKeyDecl), DeclKind(Other.DeclKind) { Other.Owning = false; } @@ -156,6 +158,9 @@ namespace clang { /// \brief Retrieve the first ID. GlobalDeclID getFirstID() const { return FirstID; } + /// \brief Is this declaration the key declaration? + bool isKeyDecl() const { return IsKeyDecl; } + /// \brief Get a known declaration that this should be merged with, if /// any. Decl *getKnownMergeTarget() const { return MergeWith; } @@ -348,7 +353,7 @@ namespace clang { void mergeTemplatePattern(RedeclarableTemplateDecl *D, RedeclarableTemplateDecl *Existing, - DeclID DsID); + DeclID DsID, bool IsKeyDecl); ObjCTypeParamList *ReadObjCTypeParamList(); @@ -2173,12 +2178,16 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitRedeclarable(Redeclarable *D) { DeclID FirstDeclID = ReadDeclID(Record, Idx); Decl *MergeWith = nullptr; + bool IsKeyDecl = ThisDeclID == FirstDeclID; // 0 indicates that this declaration was the only declaration of its entity, // and is used for space optimization. - if (FirstDeclID == 0) + if (FirstDeclID == 0) { FirstDeclID = ThisDeclID; - else if (unsigned N = Record[Idx++]) { + IsKeyDecl = true; + } else if (unsigned N = Record[Idx++]) { + IsKeyDecl = false; + // We have some declarations that must be before us in our redeclaration // chain. Read them now, and remember that we ought to merge with one of // them. @@ -2204,7 +2213,7 @@ ASTDeclReader::VisitRedeclarable(Redeclarable *D) { // The result structure takes care to note that we need to load the // other declaration chains for this ID. return RedeclarableResult(Reader, FirstDeclID, MergeWith, - static_cast(D)->getKind()); + static_cast(D)->getKind(), IsKeyDecl); } /// \brief Attempts to merge the given declaration (D) with another declaration @@ -2243,11 +2252,12 @@ template static T assert_cast(...) { /// declarations. void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D, RedeclarableTemplateDecl *Existing, - DeclID DsID) { + DeclID DsID, bool IsKeyDecl) { auto *DPattern = D->getTemplatedDecl(); auto *ExistingPattern = Existing->getTemplatedDecl(); RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(), - /*MergeWith*/ExistingPattern, DPattern->getKind()); + /*MergeWith*/ExistingPattern, DPattern->getKind(), + IsKeyDecl); if (auto *DClass = dyn_cast(DPattern)) { // Merge with any existing definition. @@ -2310,11 +2320,11 @@ void ASTDeclReader::mergeRedeclarable(Redeclarable *DBase, T *Existing, if (auto *DTemplate = dyn_cast(D)) mergeTemplatePattern( DTemplate, assert_cast(ExistingCanon), - TemplatePatternID); + TemplatePatternID, Redecl.isKeyDecl()); - // If this declaration was the canonical declaration, make a note of that. - if (DCanon == D) { - Reader.MergedDecls[ExistingCanon].push_back(Redecl.getFirstID()); + // If this declaration is a key declaration, make a note of that. + if (Redecl.isKeyDecl()) { + Reader.KeyDecls[ExistingCanon].push_back(Redecl.getFirstID()); if (Reader.PendingDeclChainsKnown.insert(ExistingCanon).second) Reader.PendingDeclChains.push_back(ExistingCanon); } @@ -3468,7 +3478,7 @@ namespace { return; } - + // Dig out all of the redeclarations. unsigned Offset = Result->Offset; unsigned N = M.RedeclarationChains[Offset]; @@ -3531,9 +3541,9 @@ void ASTReader::loadPendingDeclChain(Decl *CanonDecl) { GlobalDeclID CanonID = CanonDecl->getGlobalID(); if (CanonID) SearchDecls.push_back(CanonDecl->getGlobalID()); // Always first. - MergedDeclsMap::iterator MergedPos = MergedDecls.find(CanonDecl); - if (MergedPos != MergedDecls.end()) - SearchDecls.append(MergedPos->second.begin(), MergedPos->second.end()); + KeyDeclsMap::iterator KeyPos = KeyDecls.find(CanonDecl); + if (KeyPos != KeyDecls.end()) + SearchDecls.append(KeyPos->second.begin(), KeyPos->second.end()); // Build up the list of redeclarations. RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized, CanonID); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 22925c4..8b68638 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3634,6 +3634,55 @@ ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC, /// bitstream, or 0 if no block was written. uint64_t ASTWriter::WriteDeclContextVisibleBlock(ASTContext &Context, DeclContext *DC) { + // If we imported a key declaration of this namespace, write the visible + // lookup results as an update record for it rather than including them + // on this declaration. We will only look at key declarations on reload. + if (isa(DC) && Chain && + Chain->getKeyDeclaration(cast(DC))->isFromASTFile()) { + // Only do this once, for the first local declaration of the namespace. + for (NamespaceDecl *Prev = cast(DC)->getPreviousDecl(); Prev; + Prev = Prev->getPreviousDecl()) + if (!Prev->isFromASTFile()) + return 0; + + // Note that we need to emit an update record for the primary context. + UpdatedDeclContexts.insert(DC->getPrimaryContext()); + + // Make sure all visible decls are written. They will be recorded later. We + // do this using a side data structure so we can sort the names into + // a deterministic order. + StoredDeclsMap *Map = DC->getPrimaryContext()->buildLookup(); + SmallVector, 16> + LookupResults; + if (Map) { + LookupResults.reserve(Map->size()); + for (auto &Entry : *Map) + LookupResults.push_back( + std::make_pair(Entry.first, Entry.second.getLookupResult())); + } + + std::sort(LookupResults.begin(), LookupResults.end(), llvm::less_first()); + for (auto &NameAndResult : LookupResults) { + DeclarationName Name = NameAndResult.first; + DeclContext::lookup_result Result = NameAndResult.second; + if (Name.getNameKind() == DeclarationName::CXXConstructorName || + Name.getNameKind() == DeclarationName::CXXConversionFunctionName) { + // We have to work around a name lookup bug here where negative lookup + // results for these names get cached in namespace lookup tables (these + // names should never be looked up in a namespace). + assert(Result.empty() && "Cannot have a constructor or conversion " + "function name in a namespace!"); + continue; + } + + for (NamedDecl *ND : Result) + if (!ND->isFromASTFile()) + GetDeclRef(ND); + } + + return 0; + } + if (DC->getPrimaryContext() != DC) return 0; @@ -3685,6 +3734,11 @@ void ASTWriter::WriteDeclContextVisibleUpdate(const DeclContext *DC) { SmallString<4096> LookupTable; uint32_t BucketOffset = GenerateNameLookupTable(DC, LookupTable); + // If we're updating a namespace, select a key declaration as the key for the + // update record; those are the only ones that will be checked on reload. + if (isa(DC)) + DC = cast(Chain->getKeyDeclaration(cast(DC))); + // Write the lookup table RecordData Record; Record.push_back(UPDATE_VISIBLE); @@ -3717,11 +3771,17 @@ void ASTWriter::WriteRedeclarations() { SmallVector LocalRedeclsMap; for (unsigned I = 0, N = Redeclarations.size(); I != N; ++I) { - Decl *First = Redeclarations[I]; - assert(First->isFirstDecl() && "Not the first declaration?"); - - Decl *MostRecent = First->getMostRecentDecl(); - + const Decl *Key = Redeclarations[I]; + assert((Chain ? Chain->getKeyDeclaration(Key) == Key + : Key->isFirstDecl()) && + "not the key declaration"); + + const Decl *First = Key->getCanonicalDecl(); + const Decl *MostRecent = First->getMostRecentDecl(); + + assert((getDeclID(First) >= NUM_PREDEF_DECL_IDS || First == Key) && + "should not have imported key decls for predefined decl"); + // If we only have a single declaration, there is no point in storing // a redeclaration chain. if (First == MostRecent) @@ -3730,34 +3790,34 @@ void ASTWriter::WriteRedeclarations() { unsigned Offset = LocalRedeclChains.size(); unsigned Size = 0; LocalRedeclChains.push_back(0); // Placeholder for the size. - + // Collect the set of local redeclarations of this declaration. - for (Decl *Prev = MostRecent; Prev != First; + for (const Decl *Prev = MostRecent; Prev; Prev = Prev->getPreviousDecl()) { - if (!Prev->isFromASTFile()) { + if (!Prev->isFromASTFile() && Prev != Key) { AddDeclRef(Prev, LocalRedeclChains); ++Size; } } LocalRedeclChains[Offset] = Size; - + // Reverse the set of local redeclarations, so that we store them in // order (since we found them in reverse order). std::reverse(LocalRedeclChains.end() - Size, LocalRedeclChains.end()); - + // Add the mapping from the first ID from the AST to the set of local // declarations. - LocalRedeclarationsInfo Info = { getDeclID(First), Offset }; + LocalRedeclarationsInfo Info = { getDeclID(Key), Offset }; LocalRedeclsMap.push_back(Info); - + assert(N == Redeclarations.size() && "Deserialized a declaration we shouldn't have"); } - + if (LocalRedeclChains.empty()) return; - + // Sort the local redeclarations map by the first declaration ID, // since the reader will be performing binary searches on this information. llvm::array_pod_sort(LocalRedeclsMap.begin(), LocalRedeclsMap.end()); @@ -4053,25 +4113,27 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, Preprocessor &PP = SemaRef.PP; // Set up predefined declaration IDs. - DeclIDs[Context.getTranslationUnitDecl()] = PREDEF_DECL_TRANSLATION_UNIT_ID; - if (Context.ObjCIdDecl) - DeclIDs[Context.ObjCIdDecl] = PREDEF_DECL_OBJC_ID_ID; - if (Context.ObjCSelDecl) - DeclIDs[Context.ObjCSelDecl] = PREDEF_DECL_OBJC_SEL_ID; - if (Context.ObjCClassDecl) - DeclIDs[Context.ObjCClassDecl] = PREDEF_DECL_OBJC_CLASS_ID; - if (Context.ObjCProtocolClassDecl) - DeclIDs[Context.ObjCProtocolClassDecl] = PREDEF_DECL_OBJC_PROTOCOL_ID; - if (Context.Int128Decl) - DeclIDs[Context.Int128Decl] = PREDEF_DECL_INT_128_ID; - if (Context.UInt128Decl) - DeclIDs[Context.UInt128Decl] = PREDEF_DECL_UNSIGNED_INT_128_ID; - if (Context.ObjCInstanceTypeDecl) - DeclIDs[Context.ObjCInstanceTypeDecl] = PREDEF_DECL_OBJC_INSTANCETYPE_ID; - if (Context.BuiltinVaListDecl) - DeclIDs[Context.getBuiltinVaListDecl()] = PREDEF_DECL_BUILTIN_VA_LIST_ID; - if (Context.ExternCContext) - DeclIDs[Context.ExternCContext] = PREDEF_DECL_EXTERN_C_CONTEXT_ID; + auto RegisterPredefDecl = [&] (Decl *D, PredefinedDeclIDs ID) { + if (D) { + assert(D->isCanonicalDecl() && "predefined decl is not canonical"); + DeclIDs[D] = ID; + if (D->getMostRecentDecl() != D) + Redeclarations.push_back(D); + } + }; + RegisterPredefDecl(Context.getTranslationUnitDecl(), + PREDEF_DECL_TRANSLATION_UNIT_ID); + RegisterPredefDecl(Context.ObjCIdDecl, PREDEF_DECL_OBJC_ID_ID); + RegisterPredefDecl(Context.ObjCSelDecl, PREDEF_DECL_OBJC_SEL_ID); + RegisterPredefDecl(Context.ObjCClassDecl, PREDEF_DECL_OBJC_CLASS_ID); + RegisterPredefDecl(Context.ObjCProtocolClassDecl, + PREDEF_DECL_OBJC_PROTOCOL_ID); + RegisterPredefDecl(Context.Int128Decl, PREDEF_DECL_INT_128_ID); + RegisterPredefDecl(Context.UInt128Decl, PREDEF_DECL_UNSIGNED_INT_128_ID); + RegisterPredefDecl(Context.ObjCInstanceTypeDecl, + PREDEF_DECL_OBJC_INSTANCETYPE_ID); + RegisterPredefDecl(Context.BuiltinVaListDecl, PREDEF_DECL_BUILTIN_VA_LIST_ID); + RegisterPredefDecl(Context.ExternCContext, PREDEF_DECL_EXTERN_C_CONTEXT_ID); // Build a record containing all of the tentative definitions in this file, in // TentativeDefinitions order. Generally, this record will be empty for @@ -5675,7 +5737,7 @@ void ASTWriter::AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD, void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) { assert(!DoneWritingDeclsAndTypes && "Already done writing updates!"); if (!Chain) return; - Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) { + Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) { // If we don't already know the exception specification for this redecl // chain, add an update record for it. if (isUnresolvedExceptionSpec(cast(D) @@ -5689,7 +5751,7 @@ void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) { void ASTWriter::DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) { assert(!WritingAST && "Already writing the AST!"); if (!Chain) return; - Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) { + Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) { DeclUpdates[D].push_back( DeclUpdate(UPD_CXX_DEDUCED_RETURN_TYPE, ReturnType)); }); @@ -5700,7 +5762,7 @@ void ASTWriter::ResolvedOperatorDelete(const CXXDestructorDecl *DD, assert(!WritingAST && "Already writing the AST!"); assert(Delete && "Not given an operator delete"); if (!Chain) return; - Chain->forEachFormerlyCanonicalImportedDecl(DD, [&](const Decl *D) { + Chain->forEachImportedKeyDecl(DD, [&](const Decl *D) { DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_RESOLVED_DTOR_DELETE, Delete)); }); } diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index cdd8487..fd6708d 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -254,6 +254,8 @@ void ASTDeclWriter::VisitDecl(Decl *D) { // // This happens when we instantiate a class with a friend declaration or a // function with a local extern declaration, for instance. + // + // FIXME: Can we handle this in AddedVisibleDecl instead? if (D->isOutOfLine()) { auto *DC = D->getDeclContext(); while (auto *NS = dyn_cast(DC->getRedeclContext())) { @@ -1005,42 +1007,6 @@ void ASTDeclWriter::VisitNamespaceDecl(NamespaceDecl *D) { Writer.AddDeclRef(D->getAnonymousNamespace(), Record); Code = serialization::DECL_NAMESPACE; - if (Writer.hasChain() && !D->isOriginalNamespace() && - D->getOriginalNamespace()->isFromASTFile()) { - NamespaceDecl *NS = D->getOriginalNamespace(); - Writer.UpdatedDeclContexts.insert(NS); - - // Make sure all visible decls are written. They will be recorded later. We - // do this using a side data structure so we can sort the names into - // a deterministic order. - StoredDeclsMap *Map = NS->buildLookup(); - SmallVector, 16> - LookupResults; - if (Map) { - LookupResults.reserve(Map->size()); - for (auto &Entry : *Map) - LookupResults.push_back( - std::make_pair(Entry.first, Entry.second.getLookupResult())); - } - - std::sort(LookupResults.begin(), LookupResults.end(), llvm::less_first()); - for (auto &NameAndResult : LookupResults) { - DeclarationName Name = NameAndResult.first; - DeclContext::lookup_result Result = NameAndResult.second; - if (Name.getNameKind() == DeclarationName::CXXConstructorName || - Name.getNameKind() == DeclarationName::CXXConversionFunctionName) { - // We have to work around a name lookup bug here where negative lookup - // results for these names get cached in namespace lookup tables. - assert(Result.empty() && "Cannot have a constructor or conversion " - "function name in a namespace!"); - continue; - } - - for (NamedDecl *ND : Result) - Writer.GetDeclRef(ND); - } - } - if (Writer.hasChain() && D->isAnonymousNamespace() && D == D->getMostRecentDecl()) { // This is a most recent reopening of the anonymous namespace. If its parent @@ -1512,6 +1478,17 @@ void ASTDeclWriter::VisitDeclContext(DeclContext *DC, uint64_t LexicalOffset, Record.push_back(VisibleOffset); } +/// Determine whether D is the first declaration in its redeclaration chain that +/// is not from an AST file. +template +static bool isFirstLocalDecl(Redeclarable *D) { + assert(D && !static_cast(D)->isFromASTFile()); + do + D = D->getPreviousDecl(); + while (D && static_cast(D)->isFromASTFile()); + return !D; +} + template void ASTDeclWriter::VisitRedeclarable(Redeclarable *D) { T *First = D->getFirstDecl(); @@ -1520,41 +1497,30 @@ void ASTDeclWriter::VisitRedeclarable(Redeclarable *D) { assert(isRedeclarableDeclKind(static_cast(D)->getKind()) && "Not considered redeclarable?"); - // There is more than one declaration of this entity, so we will need to - // write a redeclaration chain. Writer.AddDeclRef(First, Record); - Writer.Redeclarations.insert(First); - - auto *Previous = D->getPreviousDecl(); - // In a modules build, we can have imported declarations after a local - // canonical declaration. If this is the first local declaration, emit - // a list of all such imported declarations so that we can ensure they - // are loaded before we are. This allows us to rebuild the redecl chain - // in the right order on reload (all declarations imported by a module - // should be before all declarations provided by that module). - bool EmitImportedMergedCanonicalDecls = false; + // In a modules build, emit a list of all imported key declarations + // (excluding First, if it was imported), so that we can be sure that all + // redeclarations visible to this module are before D in the redecl chain. + unsigned I = Record.size(); + Record.push_back(0); if (Context.getLangOpts().Modules && Writer.Chain) { - auto *PreviousLocal = Previous; - while (PreviousLocal && PreviousLocal->isFromASTFile()) - PreviousLocal = PreviousLocal->getPreviousDecl(); - if (!PreviousLocal) - EmitImportedMergedCanonicalDecls = true; + if (isFirstLocalDecl(D)) { + Writer.Chain->forEachImportedKeyDecl(First, [&](const Decl *D) { + if (D != First) + Writer.AddDeclRef(D, Record); + }); + Record[I] = Record.size() - I - 1; + + // Write a redeclaration chain, attached to the first key decl. + Writer.Redeclarations.push_back(Writer.Chain->getKeyDeclaration(First)); + } + } else if (D == First || D->getPreviousDecl()->isFromASTFile()) { + assert(isFirstLocalDecl(D) && "imported decl after local decl"); + + // Write a redeclaration chain attached to the first decl. + Writer.Redeclarations.push_back(First); } - if (EmitImportedMergedCanonicalDecls) { - llvm::SmallMapVector FirstInModule; - for (auto *Redecl = MostRecent; Redecl; - Redecl = Redecl->getPreviousDecl()) - if (Redecl->isFromASTFile()) - FirstInModule[Writer.Chain->getOwningModuleFile(Redecl)] = Redecl; - // FIXME: If FirstInModule has entries for modules A and B, and B imports - // A (directly or indirectly), we don't need to write the entry for A. - Record.push_back(FirstInModule.size()); - for (auto I = FirstInModule.rbegin(), E = FirstInModule.rend(); - I != E; ++I) - Writer.AddDeclRef(I->second, Record); - } else - Record.push_back(0); // Make sure that we serialize both the previous and the most-recent // declarations, which (transitively) ensures that all declarations in the @@ -1562,7 +1528,7 @@ void ASTDeclWriter::VisitRedeclarable(Redeclarable *D) { // // FIXME: This is not correct; when we reach an imported declaration we // won't emit its previous declaration. - (void)Writer.GetDeclRef(Previous); + (void)Writer.GetDeclRef(D->getPreviousDecl()); (void)Writer.GetDeclRef(MostRecent); } else { // We use the sentinel value 0 to indicate an only declaration.