diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 5ef4a5cdabd8..9ed1bf97ec7a 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -110,8 +110,8 @@ namespace clang { /// chain and to introduce it into the list of pending redeclaration chains /// on destruction. /// - /// The caller can choose not to introduce this ID into the redeclaration - /// chain by calling \c suppress(). + /// The caller can choose not to introduce this ID into the list of pending + /// redeclaration chains by calling \c suppress(). class RedeclarableResult { ASTReader &Reader; GlobalDeclID FirstID; @@ -1134,8 +1134,6 @@ void ASTDeclReader::VisitNamespaceDecl(NamespaceDecl *D) { D->setInline(Record[Idx++]); D->LocStart = ReadSourceLocation(Record, Idx); D->RBraceLoc = ReadSourceLocation(Record, Idx); - // FIXME: At the point of this call, D->getCanonicalDecl() returns 0. - mergeRedeclarable(D, Redecl); if (Redecl.getFirstID() == ThisDeclID) { // Each module has its own anonymous namespace, which is disjoint from @@ -1149,6 +1147,8 @@ void ASTDeclReader::VisitNamespaceDecl(NamespaceDecl *D) { // been deserialized. D->AnonOrFirstNamespaceAndInline.setPointer(D->getFirstDecl()); } + + mergeRedeclarable(D, Redecl); } void ASTDeclReader::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) { @@ -2009,14 +2009,25 @@ ASTDeclReader::VisitRedeclarable(Redeclarable *D) { /// \brief Attempts to merge the given declaration (D) with another declaration /// of the same entity. template -void ASTDeclReader::mergeRedeclarable(Redeclarable *D, +void ASTDeclReader::mergeRedeclarable(Redeclarable *DBase, RedeclarableResult &Redecl, DeclID TemplatePatternID) { + T *D = static_cast(DBase); + T *DCanon = D->getCanonicalDecl(); + if (D != DCanon && + (!Reader.getContext().getLangOpts().Modules || + Reader.getOwningModuleFile(DCanon) == Reader.getOwningModuleFile(D))) { + // All redeclarations between this declaration and its originally-canonical + // declaration get pulled in when we load DCanon; we don't need to + // perform any more merging now. + Redecl.suppress(); + } + // If modules are not available, there is no reason to perform this merge. if (!Reader.getContext().getLangOpts().Modules) return; - if (FindExistingResult ExistingRes = findExisting(static_cast(D))) + if (FindExistingResult ExistingRes = findExisting(D)) if (T *Existing = ExistingRes) mergeRedeclarable(D, Existing, Redecl, TemplatePatternID); } @@ -2036,7 +2047,8 @@ void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D, DeclID DsID) { auto *DPattern = D->getTemplatedDecl(); auto *ExistingPattern = Existing->getTemplatedDecl(); - RedeclarableResult Result(Reader, DsID, DPattern->getKind()); + RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(), + DPattern->getKind()); if (auto *DClass = dyn_cast(DPattern)) { // Merge with any existing definition. // FIXME: This is duplicated in several places. Refactor. @@ -2076,8 +2088,10 @@ void ASTDeclReader::mergeRedeclarable(Redeclarable *DBase, T *Existing, T *ExistingCanon = Existing->getCanonicalDecl(); T *DCanon = D->getCanonicalDecl(); if (ExistingCanon != DCanon) { + assert(DCanon->getGlobalID() == Redecl.getFirstID()); + // Have our redeclaration link point back at the canonical declaration - // of the existing declaration, so that this declaration has the + // of the existing declaration, so that this declaration has the // appropriate canonical declaration. D->RedeclLink = Redeclarable::PreviousDeclLink(ExistingCanon); @@ -2092,18 +2106,6 @@ void ASTDeclReader::mergeRedeclarable(Redeclarable *DBase, T *Existing, DTemplate, assert_cast(ExistingCanon), TemplatePatternID); - // Don't introduce DCanon into the set of pending declaration chains. - Redecl.suppress(); - - // Introduce ExistingCanon into the set of pending declaration chains, - // if in fact it came from a module file. - if (ExistingCanon->isFromASTFile()) { - GlobalDeclID ExistingCanonID = ExistingCanon->getGlobalID(); - assert(ExistingCanonID && "Unrecorded canonical declaration ID?"); - if (Reader.PendingDeclChainsKnown.insert(ExistingCanonID)) - Reader.PendingDeclChains.push_back(ExistingCanonID); - } - // If this declaration was the canonical declaration, make a note of // that. We accept the linear algorithm here because the number of // unique canonical declarations of an entity should always be tiny. @@ -2112,14 +2114,6 @@ void ASTDeclReader::mergeRedeclarable(Redeclarable *DBase, T *Existing, if (std::find(Merged.begin(), Merged.end(), Redecl.getFirstID()) == Merged.end()) Merged.push_back(Redecl.getFirstID()); - - // If ExistingCanon did not come from a module file, introduce the - // first declaration that *does* come from a module file to the - // set of pending declaration chains, so that we merge this - // declaration. - if (!ExistingCanon->isFromASTFile() && - Reader.PendingDeclChainsKnown.insert(Redecl.getFirstID())) - Reader.PendingDeclChains.push_back(Merged[0]); } } } @@ -2422,6 +2416,8 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { if (!Name) { // Don't bother trying to find unnamed declarations. FindExistingResult Result(Reader, D, /*Existing=*/nullptr); + // FIXME: We may still need to pull in the redeclaration chain; there can + // be redeclarations via 'decltype'. Result.suppress(); return Result; } @@ -3001,6 +2997,8 @@ namespace { // Visit each of the declarations. for (unsigned I = 0, N = SearchDecls.size(); I != N; ++I) searchForID(M, SearchDecls[I]); + // FIXME: If none of the SearchDecls had local IDs in this module, can + // we avoid searching any ancestor module files? return false; } @@ -3308,6 +3306,10 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile, // FIXME: This doesn't send the right notifications if there are // ASTMutationListeners other than an ASTWriter. + // FIXME: We can't both pull in declarations (and thus create new pending + // redeclaration chains) *and* walk redeclaration chains in this function. + // We should defer the updates that require walking redecl chains. + // Maintain AST consistency: any later redeclarations are used too. for (auto *Redecl = D->getMostRecentDecl(); /**/; Redecl = Redecl->getPreviousDecl()) {