From 3b961266218aadc9051719994278ba0d9c78ca0f Mon Sep 17 00:00:00 2001 From: Richard Smith <richard-llvm@metafoo.co.uk> Date: Sat, 30 Aug 2014 00:04:23 +0000 Subject: [PATCH] [modules] Fix deserialization cycle when loading a tag declaration with a typedef name for linkage purposes. When loading the type, delay loading its typedef until we've finished loading and merging the type. In its place, save out the name of the typedef, which we need for merging purposes. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@216806 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Serialization/ASTReaderDecl.cpp | 107 ++++++++++++----------- lib/Serialization/ASTWriterDecl.cpp | 29 +++--- test/Modules/Inputs/cxx-decls-imported.h | 8 ++ test/Modules/Inputs/cxx-decls-merged.h | 8 ++ test/Modules/cxx-decls.cpp | 3 + 5 files changed, 96 insertions(+), 59 deletions(-) diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index c63d3b06f75..9a5723058a5 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -44,6 +44,8 @@ namespace clang { unsigned &Idx; TypeID TypeIDForTypeDecl; unsigned AnonymousDeclNumber; + GlobalDeclID NamedDeclForTagDecl; + IdentifierInfo *TypedefNameForLinkage; bool HasPendingBody; @@ -165,24 +167,29 @@ namespace clang { NamedDecl *New; NamedDecl *Existing; mutable bool AddResult; + unsigned AnonymousDeclNumber; + IdentifierInfo *TypedefNameForLinkage; void operator=(FindExistingResult&) LLVM_DELETED_FUNCTION; public: FindExistingResult(ASTReader &Reader) : Reader(Reader), New(nullptr), Existing(nullptr), AddResult(false), - AnonymousDeclNumber(0) {} + AnonymousDeclNumber(0), TypedefNameForLinkage(0) {} FindExistingResult(ASTReader &Reader, NamedDecl *New, NamedDecl *Existing, - unsigned AnonymousDeclNumber = 0) + unsigned AnonymousDeclNumber, + IdentifierInfo *TypedefNameForLinkage) : Reader(Reader), New(New), Existing(Existing), AddResult(true), - AnonymousDeclNumber(AnonymousDeclNumber) {} + AnonymousDeclNumber(AnonymousDeclNumber), + TypedefNameForLinkage(TypedefNameForLinkage) {} FindExistingResult(const FindExistingResult &Other) : Reader(Other.Reader), New(Other.New), Existing(Other.Existing), AddResult(Other.AddResult), - AnonymousDeclNumber(Other.AnonymousDeclNumber) { + AnonymousDeclNumber(Other.AnonymousDeclNumber), + TypedefNameForLinkage(Other.TypedefNameForLinkage) { Other.AddResult = false; } @@ -201,13 +208,12 @@ namespace clang { FindExistingResult findExisting(NamedDecl *D); public: - ASTDeclReader(ASTReader &Reader, ModuleFile &F, - DeclID thisDeclID, - unsigned RawLocation, - const RecordData &Record, unsigned &Idx) - : Reader(Reader), F(F), ThisDeclID(thisDeclID), - RawLocation(RawLocation), Record(Record), Idx(Idx), - TypeIDForTypeDecl(0), HasPendingBody(false) { } + ASTDeclReader(ASTReader &Reader, ModuleFile &F, DeclID thisDeclID, + unsigned RawLocation, const RecordData &Record, unsigned &Idx) + : Reader(Reader), F(F), ThisDeclID(thisDeclID), + RawLocation(RawLocation), Record(Record), Idx(Idx), + TypeIDForTypeDecl(0), NamedDeclForTagDecl(0), + TypedefNameForLinkage(nullptr), HasPendingBody(false) {} template <typename DeclT> static void attachPreviousDeclImpl(ASTReader &Reader, @@ -371,6 +377,12 @@ void ASTDeclReader::Visit(Decl *D) { if (TypeDecl *TD = dyn_cast<TypeDecl>(D)) { // We have a fully initialized TypeDecl. Read its type now. TD->setTypeForDecl(Reader.GetType(TypeIDForTypeDecl).getTypePtrOrNull()); + + // If this is a tag declaration with a typedef name for linkage, it's safe + // to load that typedef now. + if (NamedDeclForTagDecl) + cast<TagDecl>(D)->NamedDeclOrQualifier = + cast<NamedDecl>(Reader.GetDecl(NamedDeclForTagDecl)); } else if (ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(D)) { // if we have a fully initialized TypeDecl, we can safely read its type now. ID->TypeForDecl = Reader.GetType(TypeIDForTypeDecl).getTypePtrOrNull(); @@ -509,12 +521,25 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitTagDecl(TagDecl *TD) { TD->setCompleteDefinitionRequired(Record[Idx++]); TD->setRBraceLoc(ReadSourceLocation(Record, Idx)); - if (Record[Idx++]) { // hasExtInfo + switch (Record[Idx++]) { + case 0: + break; + case 1: { // ExtInfo TagDecl::ExtInfo *Info = new (Reader.getContext()) TagDecl::ExtInfo(); ReadQualifierInfo(*Info, Record, Idx); TD->NamedDeclOrQualifier = Info; - } else + break; + } + case 2: // TypedefNameForAnonDecl + NamedDeclForTagDecl = ReadDeclID(Record, Idx); + TypedefNameForLinkage = Reader.GetIdentifierInfo(F, Record, Idx); + break; + case 3: // DeclaratorForAnonDecl TD->NamedDeclOrQualifier = ReadDeclAs<NamedDecl>(Record, Idx); + break; + default: + llvm_unreachable("unexpected tag info kind"); + } if (!isa<CXXRecordDecl>(TD)) mergeRedeclarable(TD, Redecl); @@ -2447,36 +2472,15 @@ static DeclContext *getPrimaryContextForMerging(DeclContext *DC) { return nullptr; } -static DeclarationName -getNameForMerging(NamedDecl *D, bool &IsTypedefNameForLinkage) { - DeclarationName Name = D->getDeclName(); - - if (!Name) { - // If this declaration has a typedef name for linkage purposes, - // look that name up when merging. We may be able to find another - // typedef that names a matching TagDecl. - if (auto *TD = dyn_cast<TagDecl>(D)) { - if (auto *Typedef = TD->getTypedefNameForAnonDecl()) { - Name = Typedef->getDeclName(); - IsTypedefNameForLinkage = true; - } - } - } - - return Name; -} - ASTDeclReader::FindExistingResult::~FindExistingResult() { if (!AddResult || Existing) return; - bool IsTypedefNameForLinkage = false; - DeclarationName Name = getNameForMerging(New, IsTypedefNameForLinkage); - + DeclarationName Name = New->getDeclName(); DeclContext *DC = New->getDeclContext()->getRedeclContext(); - if (IsTypedefNameForLinkage) { + if (TypedefNameForLinkage) { Reader.ImportedTypedefNamesForLinkage.insert( - std::make_pair(std::make_pair(DC, Name.getAsIdentifierInfo()), New)); + std::make_pair(std::make_pair(DC, TypedefNameForLinkage), New)); } else if (!Name) { assert(needsAnonymousDeclarationNumber(New)); setAnonymousDeclForMerging(Reader, New->getLexicalDeclContext(), @@ -2557,13 +2561,14 @@ void ASTDeclReader::setAnonymousDeclForMerging(ASTReader &Reader, } ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { - bool IsTypedefNameForLinkage = false; - DeclarationName Name = getNameForMerging(D, IsTypedefNameForLinkage); + DeclarationName Name = TypedefNameForLinkage ? TypedefNameForLinkage + : D->getDeclName(); if (!Name && !needsAnonymousDeclarationNumber(D)) { // Don't bother trying to find unnamed declarations that are in // unmergeable contexts. - FindExistingResult Result(Reader, D, /*Existing=*/nullptr); + FindExistingResult Result(Reader, D, /*Existing=*/nullptr, + AnonymousDeclNumber, TypedefNameForLinkage); // FIXME: We may still need to pull in the redeclaration chain; there can // be redeclarations via 'decltype'. Result.suppress(); @@ -2574,12 +2579,13 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { // necessary merging already. DeclContext *DC = D->getDeclContext()->getRedeclContext(); - if (IsTypedefNameForLinkage) { + if (TypedefNameForLinkage) { auto It = Reader.ImportedTypedefNamesForLinkage.find( - std::make_pair(DC, Name.getAsIdentifierInfo())); + std::make_pair(DC, TypedefNameForLinkage)); if (It != Reader.ImportedTypedefNamesForLinkage.end()) if (isSameEntity(It->second, D)) - return FindExistingResult(Reader, D, It->second); + return FindExistingResult(Reader, D, It->second, AnonymousDeclNumber, + TypedefNameForLinkage); // Go on to check in other places in case an existing typedef name // was not imported. } @@ -2591,7 +2597,8 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { if (auto *Existing = getAnonymousDeclForMerging( Reader, D->getLexicalDeclContext(), AnonymousDeclNumber)) if (isSameEntity(Existing, D)) - return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber); + return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber, + TypedefNameForLinkage); } else if (DC->isTranslationUnit() && Reader.SemaObj) { IdentifierResolver &IdResolver = Reader.SemaObj->IdResolver; @@ -2621,16 +2628,18 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { for (IdentifierResolver::iterator I = IdResolver.begin(Name), IEnd = IdResolver.end(); I != IEnd; ++I) { - if (NamedDecl *Existing = getDeclForMerging(*I, IsTypedefNameForLinkage)) + if (NamedDecl *Existing = getDeclForMerging(*I, TypedefNameForLinkage)) if (isSameEntity(Existing, D)) - return FindExistingResult(Reader, D, Existing); + return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber, + TypedefNameForLinkage); } } else if (DeclContext *MergeDC = getPrimaryContextForMerging(DC)) { DeclContext::lookup_result R = MergeDC->noload_lookup(Name); for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) { - if (NamedDecl *Existing = getDeclForMerging(*I, IsTypedefNameForLinkage)) + if (NamedDecl *Existing = getDeclForMerging(*I, TypedefNameForLinkage)) if (isSameEntity(Existing, D)) - return FindExistingResult(Reader, D, Existing); + return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber, + TypedefNameForLinkage); } } else { // Not in a mergeable context. @@ -2648,7 +2657,7 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { Reader.PendingOdrMergeChecks.push_back(D); return FindExistingResult(Reader, D, /*Existing=*/nullptr, - AnonymousDeclNumber); + AnonymousDeclNumber, TypedefNameForLinkage); } template<typename DeclT> diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp index dfb5987fe44..674fcfad68d 100644 --- a/lib/Serialization/ASTWriterDecl.cpp +++ b/lib/Serialization/ASTWriterDecl.cpp @@ -254,13 +254,20 @@ void ASTDeclWriter::VisitTagDecl(TagDecl *D) { Record.push_back(D->isFreeStanding()); Record.push_back(D->isCompleteDefinitionRequired()); Writer.AddSourceLocation(D->getRBraceLoc(), Record); - Record.push_back(D->hasExtInfo()); - if (D->hasExtInfo()) + + if (D->hasExtInfo()) { + Record.push_back(1); Writer.AddQualifierInfo(*D->getExtInfo(), Record); - else if (D->hasDeclaratorForAnonDecl()) - Writer.AddDeclRef(D->getDeclaratorForAnonDecl(), Record); - else - Writer.AddDeclRef(D->getTypedefNameForAnonDecl(), Record); + } else if (auto *TD = D->getTypedefNameForAnonDecl()) { + Record.push_back(2); + Writer.AddDeclRef(TD, Record); + Writer.AddIdentifierRef(TD->getDeclName().getAsIdentifierInfo(), Record); + } else if (auto *DD = D->getDeclaratorForAnonDecl()) { + Record.push_back(3); + Writer.AddDeclRef(DD, Record); + } else { + Record.push_back(0); + } } void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { @@ -286,6 +293,8 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { !D->isImplicit() && !D->isUsed(false) && !D->hasExtInfo() && + !D->getTypedefNameForAnonDecl() && + !D->getDeclaratorForAnonDecl() && D->getFirstDecl() == D->getMostRecentDecl() && !D->isInvalidDecl() && !D->isReferenced() && @@ -313,6 +322,8 @@ void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) { !D->isImplicit() && !D->isUsed(false) && !D->hasExtInfo() && + !D->getTypedefNameForAnonDecl() && + !D->getDeclaratorForAnonDecl() && D->getFirstDecl() == D->getMostRecentDecl() && !D->isInvalidDecl() && !D->isReferenced() && @@ -1573,8 +1584,7 @@ void ASTWriter::WriteDeclAbbrevs() { Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFreeStanding Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsCompleteDefinitionRequired Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SourceLocation - Abv->Add(BitCodeAbbrevOp(0)); // hasExtInfo - Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // TypedefNameAnonDecl + Abv->Add(BitCodeAbbrevOp(0)); // ExtInfoKind // EnumDecl Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // AddTypeRef Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // IntegerType @@ -1621,8 +1631,7 @@ void ASTWriter::WriteDeclAbbrevs() { Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFreeStanding Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsCompleteDefinitionRequired Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SourceLocation - Abv->Add(BitCodeAbbrevOp(0)); // hasExtInfo - Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // TypedefNameAnonDecl + Abv->Add(BitCodeAbbrevOp(0)); // ExtInfoKind // RecordDecl Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // FlexibleArrayMember Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // AnonymousStructUnion diff --git a/test/Modules/Inputs/cxx-decls-imported.h b/test/Modules/Inputs/cxx-decls-imported.h index 7677511103c..691b374d0d8 100644 --- a/test/Modules/Inputs/cxx-decls-imported.h +++ b/test/Modules/Inputs/cxx-decls-imported.h @@ -36,3 +36,11 @@ struct OverridesVirtualFunctions : HasVirtualFunctions { void f(); }; extern "C" void ExternCFunction(); + +typedef struct { + struct Inner { + int n; + }; +} NameForLinkage2; +auto name_for_linkage2_inner_a = NameForLinkage2::Inner(); +typedef decltype(name_for_linkage2_inner_a) NameForLinkage2Inner; diff --git a/test/Modules/Inputs/cxx-decls-merged.h b/test/Modules/Inputs/cxx-decls-merged.h index 8ba2df63cae..7f2baf36ee9 100644 --- a/test/Modules/Inputs/cxx-decls-merged.h +++ b/test/Modules/Inputs/cxx-decls-merged.h @@ -14,3 +14,11 @@ struct OverridesVirtualFunctions : HasVirtualFunctions { }; extern OverridesVirtualFunctions overrides_virtual_functions; extern "C" void ExternCFunction(); + +typedef struct { + struct Inner { + int n; + }; +} NameForLinkage2; +auto name_for_linkage2_inner_b = NameForLinkage2::Inner(); +typedef decltype(name_for_linkage2_inner_b) NameForLinkage2Inner; diff --git a/test/Modules/cxx-decls.cpp b/test/Modules/cxx-decls.cpp index 77982e46fba..1fb550ba280 100644 --- a/test/Modules/cxx-decls.cpp +++ b/test/Modules/cxx-decls.cpp @@ -37,6 +37,9 @@ int use_overrides_virtual_functions(OverridesVirtualFunctions ovf) { return 0; } @import cxx_decls_merged; +NameForLinkage2Inner use_name_for_linkage2_inner; +NameForLinkage2 use_name_for_linkage2; + int name_for_linkage_test = use_name_for_linkage(name_for_linkage); int overrides_virtual_functions_test = use_overrides_virtual_functions(overrides_virtual_functions); -- GitLab