From 9b88a4cdf48d714751ed6a02245d7bdf023072f9 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 27 Jul 2015 05:40:23 +0000 Subject: [PATCH] [modules] Add an assert for redeclarations that we never added to their redecl chain and fix the cases where it fires. * Handle the __va_list_tag as a predefined decl. Previously we failed to merge sometimes it because it's not visible to name lookup. (In passing, remove redundant __va_list_tag typedefs that we were creating for some ABIs. These didn't affect the mangling or representation of the type.) * For Decls derived from Redeclarable that are not in fact redeclarable (implicit params, function params, ObjC type parameters), remove them from the list of expected redeclarable decls. llvm-svn: 243259 --- clang/include/clang/AST/ASTContext.h | 8 ++-- clang/include/clang/Serialization/ASTBitCodes.h | 27 +++++++------- clang/lib/AST/ASTContext.cpp | 49 ++++++++++--------------- clang/lib/Serialization/ASTCommon.h | 2 - clang/lib/Serialization/ASTReader.cpp | 9 +++-- clang/lib/Serialization/ASTReaderDecl.cpp | 47 +++++++++++++++--------- clang/lib/Serialization/ASTWriter.cpp | 1 + clang/test/Misc/ast-dump-color.cpp | 2 +- 8 files changed, 74 insertions(+), 71 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index ffcace4..c67fa67 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -843,9 +843,9 @@ public: mutable QualType AutoDeductTy; // Deduction against 'auto'. mutable QualType AutoRRefDeductTy; // Deduction against 'auto &&'. - // Type used to help define __builtin_va_list for some targets. - // The type is built when constructing 'BuiltinVaListDecl'. - mutable QualType VaListTagTy; + // Decl used to help define __builtin_va_list for some targets. + // The decl is built when constructing 'BuiltinVaListDecl'. + mutable Decl *VaListTagDecl; ASTContext(LangOptions &LOpts, SourceManager &SM, IdentifierTable &idents, SelectorTable &sels, Builtin::Context &builtins); @@ -1569,7 +1569,7 @@ public: /// \brief Retrieve the C type declaration corresponding to the predefined /// \c __va_list_tag type used to help define the \c __builtin_va_list type /// for some targets. - QualType getVaListTagType() const; + Decl *getVaListTagDecl() const; /// \brief Return a type with additional \c const, \c volatile, or /// \c restrict qualifiers. diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index f5f0e80..3d46225 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -761,26 +761,24 @@ namespace clang { PREDEF_TYPE_ARC_UNBRIDGED_CAST = 34, /// \brief The pseudo-object placeholder type. PREDEF_TYPE_PSEUDO_OBJECT = 35, - /// \brief The __va_list_tag placeholder type. - PREDEF_TYPE_VA_LIST_TAG = 36, /// \brief The placeholder type for builtin functions. - PREDEF_TYPE_BUILTIN_FN = 37, + PREDEF_TYPE_BUILTIN_FN = 36, /// \brief OpenCL 1d image type. - PREDEF_TYPE_IMAGE1D_ID = 38, + PREDEF_TYPE_IMAGE1D_ID = 37, /// \brief OpenCL 1d image array type. - PREDEF_TYPE_IMAGE1D_ARR_ID = 39, + PREDEF_TYPE_IMAGE1D_ARR_ID = 38, /// \brief OpenCL 1d image buffer type. - PREDEF_TYPE_IMAGE1D_BUFF_ID = 40, + PREDEF_TYPE_IMAGE1D_BUFF_ID = 39, /// \brief OpenCL 2d image type. - PREDEF_TYPE_IMAGE2D_ID = 41, + PREDEF_TYPE_IMAGE2D_ID = 40, /// \brief OpenCL 2d image array type. - PREDEF_TYPE_IMAGE2D_ARR_ID = 42, + PREDEF_TYPE_IMAGE2D_ARR_ID = 41, /// \brief OpenCL 3d image type. - PREDEF_TYPE_IMAGE3D_ID = 43, + PREDEF_TYPE_IMAGE3D_ID = 42, /// \brief OpenCL event type. - PREDEF_TYPE_EVENT_ID = 44, + PREDEF_TYPE_EVENT_ID = 43, /// \brief OpenCL sampler type. - PREDEF_TYPE_SAMPLER_ID = 45 + PREDEF_TYPE_SAMPLER_ID = 44 }; /// \brief The number of predefined type IDs that are reserved for @@ -945,15 +943,18 @@ namespace clang { /// \brief The internal '__builtin_va_list' typedef. PREDEF_DECL_BUILTIN_VA_LIST_ID = 9, + /// \brief The internal '__va_list_tag' struct, if any. + PREDEF_DECL_VA_LIST_TAG = 10, + /// \brief The extern "C" context. - PREDEF_DECL_EXTERN_C_CONTEXT_ID = 10, + PREDEF_DECL_EXTERN_C_CONTEXT_ID = 11, }; /// \brief The number of declaration IDs that are predefined. /// /// For more information about predefined declarations, see the /// \c PredefinedDeclIDs type and the PREDEF_DECL_*_ID constants. - const unsigned int NUM_PREDEF_DECL_IDS = 11; + const unsigned int NUM_PREDEF_DECL_IDS = 12; /// \brief Record codes for each kind of declaration. /// diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index ba4bcbc..34d085e 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -1083,7 +1083,7 @@ void ASTContext::InitBuiltinTypes(const TargetInfo &Target) { InitBuiltinType(HalfTy, BuiltinType::Half); // Builtin type used to help define __builtin_va_list. - VaListTagTy = QualType(); + VaListTagDecl = nullptr; } DiagnosticsEngine &ASTContext::getDiagnostics() const { @@ -6068,8 +6068,8 @@ CreateAArch64ABIBuiltinVaListDecl(const ASTContext *Context) { VaListTagDecl->addDecl(Field); } VaListTagDecl->completeDefinition(); + Context->VaListTagDecl = VaListTagDecl; QualType VaListTagType = Context->getRecordType(VaListTagDecl); - Context->VaListTagTy = VaListTagType; // } __builtin_va_list; return Context->buildImplicitTypedef(VaListTagType, "__builtin_va_list"); @@ -6120,8 +6120,8 @@ static TypedefDecl *CreatePowerABIBuiltinVaListDecl(const ASTContext *Context) { VaListTagDecl->addDecl(Field); } VaListTagDecl->completeDefinition(); + Context->VaListTagDecl = VaListTagDecl; QualType VaListTagType = Context->getRecordType(VaListTagDecl); - Context->VaListTagTy = VaListTagType; // } __va_list_tag; TypedefDecl *VaListTagTypedefDecl = @@ -6140,7 +6140,7 @@ static TypedefDecl *CreatePowerABIBuiltinVaListDecl(const ASTContext *Context) { static TypedefDecl * CreateX86_64ABIBuiltinVaListDecl(const ASTContext *Context) { - // typedef struct __va_list_tag { + // struct __va_list_tag { RecordDecl *VaListTagDecl; VaListTagDecl = Context->buildImplicitRecord("__va_list_tag"); VaListTagDecl->startDefinition(); @@ -6180,21 +6180,15 @@ CreateX86_64ABIBuiltinVaListDecl(const ASTContext *Context) { VaListTagDecl->addDecl(Field); } VaListTagDecl->completeDefinition(); + Context->VaListTagDecl = VaListTagDecl; QualType VaListTagType = Context->getRecordType(VaListTagDecl); - Context->VaListTagTy = VaListTagType; - - // } __va_list_tag; - TypedefDecl *VaListTagTypedefDecl = - Context->buildImplicitTypedef(VaListTagType, "__va_list_tag"); - QualType VaListTagTypedefType = - Context->getTypedefType(VaListTagTypedefDecl); + // }; - // typedef __va_list_tag __builtin_va_list[1]; + // typedef struct __va_list_tag __builtin_va_list[1]; llvm::APInt Size(Context->getTypeSize(Context->getSizeType()), 1); - QualType VaListTagArrayType - = Context->getConstantArrayType(VaListTagTypedefType, - Size, ArrayType::Normal,0); + QualType VaListTagArrayType = + Context->getConstantArrayType(VaListTagType, Size, ArrayType::Normal, 0); return Context->buildImplicitTypedef(VaListTagArrayType, "__builtin_va_list"); } @@ -6249,7 +6243,7 @@ CreateAAPCSABIBuiltinVaListDecl(const ASTContext *Context) { static TypedefDecl * CreateSystemZBuiltinVaListDecl(const ASTContext *Context) { - // typedef struct __va_list_tag { + // struct __va_list_tag { RecordDecl *VaListTagDecl; VaListTagDecl = Context->buildImplicitRecord("__va_list_tag"); VaListTagDecl->startDefinition(); @@ -6289,20 +6283,15 @@ CreateSystemZBuiltinVaListDecl(const ASTContext *Context) { VaListTagDecl->addDecl(Field); } VaListTagDecl->completeDefinition(); + Context->VaListTagDecl = VaListTagDecl; QualType VaListTagType = Context->getRecordType(VaListTagDecl); - Context->VaListTagTy = VaListTagType; - // } __va_list_tag; - TypedefDecl *VaListTagTypedefDecl = - Context->buildImplicitTypedef(VaListTagType, "__va_list_tag"); - QualType VaListTagTypedefType = - Context->getTypedefType(VaListTagTypedefDecl); + // }; // typedef __va_list_tag __builtin_va_list[1]; llvm::APInt Size(Context->getTypeSize(Context->getSizeType()), 1); - QualType VaListTagArrayType - = Context->getConstantArrayType(VaListTagTypedefType, - Size, ArrayType::Normal,0); + QualType VaListTagArrayType = + Context->getConstantArrayType(VaListTagType, Size, ArrayType::Normal, 0); return Context->buildImplicitTypedef(VaListTagArrayType, "__builtin_va_list"); } @@ -6340,13 +6329,13 @@ TypedefDecl *ASTContext::getBuiltinVaListDecl() const { return BuiltinVaListDecl; } -QualType ASTContext::getVaListTagType() const { - // Force the creation of VaListTagTy by building the __builtin_va_list +Decl *ASTContext::getVaListTagDecl() const { + // Force the creation of VaListTagDecl by building the __builtin_va_list // declaration. - if (VaListTagTy.isNull()) - (void) getBuiltinVaListDecl(); + if (!VaListTagDecl) + (void)getBuiltinVaListDecl(); - return VaListTagTy; + return VaListTagDecl; } void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) { diff --git a/clang/lib/Serialization/ASTCommon.h b/clang/lib/Serialization/ASTCommon.h index f21e8a7..e59bc89 100644 --- a/clang/lib/Serialization/ASTCommon.h +++ b/clang/lib/Serialization/ASTCommon.h @@ -62,8 +62,6 @@ TypeID MakeTypeID(ASTContext &Context, QualType T, IdxForTypeTy IdxForType) { return TypeIdx(PREDEF_TYPE_AUTO_DEDUCT).asTypeID(FastQuals); if (T == Context.AutoRRefDeductTy) return TypeIdx(PREDEF_TYPE_AUTO_RREF_DEDUCT).asTypeID(FastQuals); - if (T == Context.VaListTagTy) - return TypeIdx(PREDEF_TYPE_VA_LIST_TAG).asTypeID(FastQuals); return IdxForType(T).asTypeID(FastQuals); } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 520c341..0c4dab40 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -5762,10 +5762,6 @@ QualType ASTReader::GetType(TypeID ID) { T = Context.ARCUnbridgedCastTy; break; - case PREDEF_TYPE_VA_LIST_TAG: - T = Context.getVaListTagType(); - break; - case PREDEF_TYPE_BUILTIN_FN: T = Context.BuiltinFnTy; break; @@ -6078,6 +6074,9 @@ static Decl *getPredefinedDecl(ASTContext &Context, PredefinedDeclIDs ID) { case PREDEF_DECL_BUILTIN_VA_LIST_ID: return Context.getBuiltinVaListDecl(); + case PREDEF_DECL_VA_LIST_TAG: + return Context.getVaListTagDecl(); + case PREDEF_DECL_EXTERN_C_CONTEXT_ID: return Context.getExternCContextDecl(); } @@ -8142,6 +8141,8 @@ void ASTReader::finishPendingActions() { assert(PendingDeclChainsKnown.empty()); PendingDeclChains.clear(); + assert(RedeclsDeserialized.empty() && "some redecls not wired up"); + // Make the most recent of the top-level declarations visible. for (TopLevelDeclsMap::iterator TLD = TopLevelDecls.begin(), TLDEnd = TopLevelDecls.end(); TLD != TLDEnd; ++TLD) { diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index f6858b4..45a3e45 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -126,35 +126,43 @@ namespace clang { class RedeclarableResult { ASTReader &Reader; GlobalDeclID FirstID; + Decl *LoadedDecl; 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, - bool IsKeyDecl) - : Reader(Reader), FirstID(FirstID), MergeWith(MergeWith), - Owning(true), IsKeyDecl(IsKeyDecl), DeclKind(DeclKind) {} + Decl *LoadedDecl, Decl *MergeWith, bool IsKeyDecl) + : Reader(Reader), FirstID(FirstID), LoadedDecl(LoadedDecl), + MergeWith(MergeWith), Owning(true), IsKeyDecl(IsKeyDecl) {} RedeclarableResult(RedeclarableResult &&Other) - : Reader(Other.Reader), FirstID(Other.FirstID), - MergeWith(Other.MergeWith), Owning(Other.Owning), - IsKeyDecl(Other.IsKeyDecl), DeclKind(Other.DeclKind) { + : Reader(Other.Reader), FirstID(Other.FirstID), + LoadedDecl(Other.LoadedDecl), MergeWith(Other.MergeWith), + Owning(Other.Owning), IsKeyDecl(Other.IsKeyDecl) { Other.Owning = false; } ~RedeclarableResult() { - if (FirstID && Owning && isRedeclarableDeclKind(DeclKind)) { + if (FirstID && Owning && + isRedeclarableDeclKind(LoadedDecl->getKind())) { auto Canon = Reader.GetDecl(FirstID)->getCanonicalDecl(); if (Reader.PendingDeclChainsKnown.insert(Canon).second) Reader.PendingDeclChains.push_back(Canon); } } + /// \brief Note that a RedeclarableDecl is not actually redeclarable. + void setNotRedeclarable() { + Owning = false; + Reader.RedeclsDeserialized.erase(LoadedDecl); + assert(FirstID == LoadedDecl->getGlobalID() && !MergeWith && + "non-redeclarable declaration was redeclared?"); + } + /// \brief Retrieve the first ID. GlobalDeclID getFirstID() const { return FirstID; } @@ -908,7 +916,9 @@ void ASTDeclReader::VisitObjCMethodDecl(ObjCMethodDecl *MD) { } void ASTDeclReader::VisitObjCTypeParamDecl(ObjCTypeParamDecl *D) { - VisitTypedefNameDecl(D); + RedeclarableResult Redecl = VisitTypedefNameDecl(D); + Redecl.setNotRedeclarable(); + D->Variance = Record[Idx++]; D->Index = Record[Idx++]; D->VarianceLoc = ReadSourceLocation(Record, Idx); @@ -1208,9 +1218,11 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) { }; switch ((VarKind)Record[Idx++]) { case VarNotTemplate: - // Only true variables (not parameters or implicit parameters) can be merged - if (VD->getKind() != Decl::ParmVar && VD->getKind() != Decl::ImplicitParam && - !isa(VD)) + // Only true variables (not parameters or implicit parameters) can be + // merged; the other kinds are not really redeclarable at all. + if (isa(VD) || isa(VD)) + Redecl.setNotRedeclarable(); + else if (!isa(VD)) mergeRedeclarable(VD, Redecl); break; case VarTemplate: @@ -2070,6 +2082,7 @@ ASTDeclReader::VisitVarTemplateSpecializationDeclImpl( if (writtenAsCanonicalDecl) { VarTemplateDecl *CanonPattern = ReadDeclAs(Record, Idx); if (D->isCanonicalDecl()) { // It's kept in the folding set. + // FIXME: If it's already present, merge it. if (VarTemplatePartialSpecializationDecl *Partial = dyn_cast(D)) { CanonPattern->getCommonPtr()->PartialSpecializations @@ -2212,8 +2225,8 @@ 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(), IsKeyDecl); + return RedeclarableResult(Reader, FirstDeclID, static_cast(D), MergeWith, + IsKeyDecl); } /// \brief Attempts to merge the given declaration (D) with another declaration @@ -2256,8 +2269,7 @@ void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D, auto *DPattern = D->getTemplatedDecl(); auto *ExistingPattern = Existing->getTemplatedDecl(); RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(), - /*MergeWith*/ExistingPattern, DPattern->getKind(), - IsKeyDecl); + DPattern, /*MergeWith*/ ExistingPattern, IsKeyDecl); if (auto *DClass = dyn_cast(DPattern)) { // Merge with any existing definition. @@ -3540,6 +3552,7 @@ void ASTReader::loadPendingDeclChain(Decl *CanonDecl) { // Build up the list of redeclarations. RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized, CanonID); ModuleMgr.visit(Visitor); + RedeclsDeserialized.erase(CanonDecl); // Retrieve the chains. ArrayRef Chain = Visitor.getChain(); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 60c9aa44..ba8e1a8 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4165,6 +4165,7 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, RegisterPredefDecl(Context.ObjCInstanceTypeDecl, PREDEF_DECL_OBJC_INSTANCETYPE_ID); RegisterPredefDecl(Context.BuiltinVaListDecl, PREDEF_DECL_BUILTIN_VA_LIST_ID); + RegisterPredefDecl(Context.VaListTagDecl, PREDEF_DECL_VA_LIST_TAG); RegisterPredefDecl(Context.ExternCContext, PREDEF_DECL_EXTERN_C_CONTEXT_ID); // Build a record containing all of the tentative definitions in this file, in diff --git a/clang/test/Misc/ast-dump-color.cpp b/clang/test/Misc/ast-dump-color.cpp index 479467d..e93274e 100644 --- a/clang/test/Misc/ast-dump-color.cpp +++ b/clang/test/Misc/ast-dump-color.cpp @@ -32,7 +32,7 @@ struct Invalid { //CHECK: {{^}}[[GREEN:.\[0;1;32m]]TranslationUnitDecl[[RESET:.\[0m]][[Yellow:.\[0;33m]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]][[RESET]]> [[Yellow]][[RESET]]{{$}} //CHECK: {{^}}[[Blue:.\[0;34m]]|-[[RESET]][[GREEN]]TypedefDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]][[RESET]]> [[Yellow]][[RESET]] implicit[[CYAN:.\[0;1;36m]] __int128_t[[RESET]] [[Green:.\[0;32m]]'__int128'[[RESET]]{{$}} //CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]TypedefDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]][[RESET]]> [[Yellow]][[RESET]] implicit[[CYAN]] __uint128_t[[RESET]] [[Green]]'unsigned __int128'[[RESET]]{{$}} -//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]TypedefDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]][[RESET]]> [[Yellow]][[RESET]] implicit[[CYAN]] __builtin_va_list[[RESET]] [[Green]]'__va_list_tag [1]'[[RESET]]{{$}} +//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]TypedefDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]][[RESET]]> [[Yellow]][[RESET]] implicit[[CYAN]] __builtin_va_list[[RESET]] [[Green]]'struct __va_list_tag [1]'[[RESET]]{{$}} //CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]VarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]{{.*}}ast-dump-color.cpp:6:1[[RESET]], [[Yellow]]col:5[[RESET]]> [[Yellow]]col:5[[RESET]][[CYAN]] Test[[RESET]] [[Green]]'int'[[RESET]] //CHECK: {{^}}[[Blue]]| |-[[RESET]][[BLUE:.\[0;1;34m]]UnusedAttr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:25[[RESET]]>{{$}} //CHECK: {{^}}[[Blue]]| `-[[RESET]][[Blue]]FullComment[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:4:4[[RESET]], [[Yellow]]line:5:8[[RESET]]>{{$}} -- 2.7.4