llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Jameson Nash (vtjnash) <details> <summary>Changes</summary> Under -fexperimental-late-parse-attributes, an attribute on a parameter may name a parameter declared after it: template <class T> void f(void (*release)(T) RELEASE(mu), Mutex *mu); A parameter's attributes were instantiated as the parameter was, before any later one, so instantiating this asserted in LocalInstantiationScope::findInstantiationOf, and the late parsing change rejected it in a template. Defer a parameter's late-parsed attributes, with the mechanism a class uses for its members, until every parameter of the prototype is instantiated in TransformFunctionTypeParams. A function whose type needs no substitution instantiated its parameters one by one instead; route it through SubstParmTypes, so there is one place that defers. SubstParmTypes now takes whether to evaluate constraints, which SubstFunctionType passes in both of its calls, so a typedef'd function type also follows the instantiator's setting instead of always evaluating them. With that, remove the error for a later parameter; an attribute in a template still cannot name a pointee's parameter. Assisted-by: Claude (Opus 5.5, Fable 5.1) --- Patch is 31.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/232784.diff 9 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+1-2) - (modified) clang/include/clang/Parse/Parser.h (-1) - (modified) clang/include/clang/Sema/Sema.h (+33-14) - (modified) clang/include/clang/Sema/Template.h (+2-1) - (modified) clang/lib/Parse/ParseCXXInlineMethods.cpp (+6-13) - (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+73-30) - (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+64-35) - (added) clang/test/SemaCXX/warn-thread-safety-late-parsing-instantiation.cpp (+49) - (modified) clang/test/SemaCXX/warn-thread-safety-late-parsing.cpp (+15-17) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 7665688686b5d..f8a3c74f2c2cd 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -325,8 +325,7 @@ def warn_attribute_no_decl : Warning< "attribute %0 ignored, because it is not attached to a declaration">, InGroup<IgnoredAttributes>; def err_late_attribute_param_in_template : Error< - "%0 attribute in a template cannot name " - "%select{pointee function parameter|later parameter}1 %2">; + "%0 attribute in a template cannot name pointee function parameter %1">; def err_ms_attributes_not_enabled : Error< "'__declspec' attributes are not enabled; use '-fdeclspec' or " "'-fms-extensions' to enable support for __declspec attributes">; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 9a1abd4f3872d..a742f82618728 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -1552,7 +1552,6 @@ class Parser : public CodeCompletionHandler { /// Diagnose a late-parsed attribute naming a parameter that it cannot, and /// return true if it must be dropped. bool checkLateAttributeParamRefs(const LateParsedAttribute &LPA, - const Decl *D, ArrayRef<const DeclRefExpr *> ParamRefs, bool ReenteredProtoParams); diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 865defa98a1b3..045f00e9cf6fe 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -13951,21 +13951,43 @@ class Sema final : public SemaBase { FunctionProtoType::ExceptionSpecInfo &ESI, SmallVectorImpl<QualType> &ExceptionStorage, const MultiLevelTemplateArgumentList &Args); + /// An attribute whose instantiation is deferred, together with the + /// instantiation scope to instantiate it in. + struct LateInstantiatedAttribute { + const Attr *TmplAttr; + LocalInstantiationScope *Scope; + Decl *NewDecl; + + LateInstantiatedAttribute(const Attr *A, LocalInstantiationScope *S, + Decl *D) + : TmplAttr(A), Scope(S), NewDecl(D) {} + }; + typedef SmallVector<LateInstantiatedAttribute, 1> LateInstantiatedAttrVec; + + /// Substitute into a function parameter. If \p LateAttrs is given, its + /// late-parsed attributes are deferred there, since they may name a + /// parameter that is not substituted yet; see \ref InstantiateLateAttrs. ParmVarDecl * SubstParmVarDecl(ParmVarDecl *D, const MultiLevelTemplateArgumentList &TemplateArgs, int indexAdjustment, UnsignedOrNone NumExpansions, - bool ExpectParameterPack, bool EvaluateConstraints = true); + bool ExpectParameterPack, bool EvaluateConstraints = true, + LateInstantiatedAttrVec *LateAttrs = nullptr); /// Substitute the given template arguments into the given set of /// parameters, producing the set of parameter types that would be generated - /// from such a substitution. + /// from such a substitution. If there is a parameter with no ParmVarDecl to + /// substitute (a null entry of \p Params, as from a typedef'd function + /// type), its type needs to come from \p OldParamTypes instead, which must + /// then be passed, and \p OutParams gets null for it. bool SubstParmTypes(SourceLocation Loc, ArrayRef<ParmVarDecl *> Params, const FunctionProtoType::ExtParameterInfo *ExtParamInfos, const MultiLevelTemplateArgumentList &TemplateArgs, SmallVectorImpl<QualType> &ParamTypes, SmallVectorImpl<ParmVarDecl *> *OutParams, - ExtParameterInfoBuilder &ParamInfos); + ExtParameterInfoBuilder &ParamInfos, + const QualType *OldParamTypes = nullptr, + bool EvaluateConstraints = true); /// Substitute the given template arguments into the default argument. bool SubstDefaultArgument(SourceLocation Loc, ParmVarDecl *Param, @@ -14317,17 +14339,6 @@ class Sema final : public SemaBase { ExplicitSpecifier instantiateExplicitSpecifier( const MultiLevelTemplateArgumentList &TemplateArgs, ExplicitSpecifier ES); - struct LateInstantiatedAttribute { - const Attr *TmplAttr; - LocalInstantiationScope *Scope; - Decl *NewDecl; - - LateInstantiatedAttribute(const Attr *A, LocalInstantiationScope *S, - Decl *D) - : TmplAttr(A), Scope(S), NewDecl(D) {} - }; - typedef SmallVector<LateInstantiatedAttribute, 1> LateInstantiatedAttrVec; - /// Recheck instantiated thread-safety attributes that could not be validated /// on the dependent pattern declaration. bool checkInstantiatedThreadSafetyAttrs(const Decl *D, const Attr *A); @@ -14337,6 +14348,14 @@ class Sema final : public SemaBase { LateInstantiatedAttrVec *LateAttrs = nullptr, LocalInstantiationScope *OuterMostScope = nullptr); + /// Instantiate the attributes that \ref InstantiateAttrs deferred to \p + /// LateAttrs, and attach them to their declarations. They are instantiated + /// in the scopes saved with them, which extend \p OuterMostScope, the + /// current scope. + void InstantiateLateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs, + LateInstantiatedAttrVec &LateAttrs, + LocalInstantiationScope *OuterMostScope); + /// Update instantiation attributes after template was late parsed. /// /// Some attributes are evaluated based on the body of template. If it is diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h index 50e950e56c6ca..bc84ca45348cc 100644 --- a/clang/include/clang/Sema/Template.h +++ b/clang/include/clang/Sema/Template.h @@ -516,7 +516,8 @@ enum class TemplateSubstitutionKind : char { /// given outermost scope. static void deleteScopes(LocalInstantiationScope *Scope, LocalInstantiationScope *Outermost) { - while (Scope && Scope != Outermost) { + while (Scope != Outermost) { + assert(Scope && "scope does not extend the outermost scope"); LocalInstantiationScope *Out = Scope->Outer; delete Scope; Scope = Out; diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp index 6b478ec2e36f6..a86f628ade4c5 100644 --- a/clang/lib/Parse/ParseCXXInlineMethods.cpp +++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp @@ -723,8 +723,8 @@ static void collectParamRefs(const ParsedAttributes &Attrs, } bool Parser::checkLateAttributeParamRefs( - const LateParsedAttribute &LPA, const Decl *D, - ArrayRef<const DeclRefExpr *> ParamRefs, bool ReenteredProtoParams) { + const LateParsedAttribute &LPA, ArrayRef<const DeclRefExpr *> ParamRefs, + bool ReenteredProtoParams) { auto IsPointeeParam = [&](const ParmVarDecl *PVD) { return ReenteredProtoParams && llvm::is_contained(LPA.ProtoParams, PVD); }; @@ -753,24 +753,17 @@ bool Parser::checkLateAttributeParamRefs( // A name binds the same way in a template, but instantiation cannot yet map a // pointee's parameters, which are instantiated with the declaration's type - // and not kept, nor a parameter declared after the one the attribute is on, - // which is instantiated after that parameter's attributes. Reject the - // attribute rather than instantiate it wrongly. + // and not kept. Reject the attribute rather than instantiate it wrongly. // FIXME: Map these parameters during template instantiation. if (Actions.CurContext->isDependentContext() || getCurScope()->getTemplateParamParent() || Actions.getCurGenericLambda()) { - const auto *DParam = dyn_cast_or_null<ParmVarDecl>(D); for (const DeclRefExpr *DRE : ParamRefs) { const auto *PVD = cast<ParmVarDecl>(DRE->getDecl()); - bool Pointee = IsPointeeParam(PVD); - if (!Pointee && - !(DParam && - DParam->getFunctionScopeDepth() == PVD->getFunctionScopeDepth() && - DParam->getFunctionScopeIndex() < PVD->getFunctionScopeIndex())) + if (!IsPointeeParam(PVD)) continue; Diag(DRE->getLocation(), diag::err_late_attribute_param_in_template) - << &LPA.AttrName << !Pointee << PVD; + << &LPA.AttrName << PVD; return true; } } @@ -831,7 +824,7 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LPA, bool EnterScope, SmallVector<const DeclRefExpr *, 4> ParamRefs; collectParamRefs(Parsed, ParamRefs); if (!ParamRefs.empty() && - checkLateAttributeParamRefs(LPA, D, ParamRefs, HasProtoParams)) + checkLateAttributeParamRefs(LPA, ParamRefs, HasProtoParams)) Parsed.clear(); Attrs.takeAllAppendingFrom(Parsed); diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 036642f22bd8c..f173931f60de4 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -1327,6 +1327,11 @@ namespace { llvm::DenseMap<llvm::FoldingSetNodeID, TemplateArgumentLoc> *CurrentCachedTemplateArgs = nullptr; + // While the parameters of a prototype are being substituted, the vector is + // populated with the late-parsed attributes, and the scope that maps them. + Sema::LateInstantiatedAttrVec *LateParmAttrs = nullptr; + LocalInstantiationScope *LateParmAttrScope = nullptr; + bool instantiateMissingDeclsToScopeForConcepts(Decl *D); public: @@ -1731,6 +1736,15 @@ namespace { UnsignedOrNone NumExpansions, bool ExpectParameterPack); + using inherited::TransformFunctionTypeParams; + bool TransformFunctionTypeParams( + SourceLocation Loc, ArrayRef<ParmVarDecl *> Params, + const QualType *ParamTypes, + const FunctionProtoType::ExtParameterInfo *ParamInfos, + SmallVectorImpl<QualType> &PTypes, + SmallVectorImpl<ParmVarDecl *> *PVars, + Sema::ExtParameterInfoBuilder &PInfos, unsigned *LastParamTransformed); + using inherited::TransformTemplateTypeParmType; /// Transforms a template type parameter type by performing /// substitution of the corresponding template type argument. @@ -2507,14 +2521,54 @@ QualType TemplateInstantiator::TransformFunctionProtoType(TypeLocBuilder &TLB, ParmVarDecl *TemplateInstantiator::TransformFunctionTypeParam( ParmVarDecl *OldParm, int indexAdjustment, UnsignedOrNone NumExpansions, bool ExpectParameterPack) { + // Defer the late-parsed attributes of a parameter of the prototype being + // transformed, when in its scope. A parameter is otherwise transformed on its + // own only on demand, for a constraint that names it (see + // instantiateMissingDeclsToScopeForConcepts). Its attributes are then + // instantiated at once, and a later parameter they name is instantiated on + // demand as well. + Sema::LateInstantiatedAttrVec *LateAttrs = nullptr; + if (LateParmAttrs && SemaRef.CurrentInstantiationScope == LateParmAttrScope) + LateAttrs = LateParmAttrs; + else + assert((SemaRef.inConstraintSubstitution() || + SemaRef.inParameterMappingSubstitution()) && + "parameter transformed outside its prototype"); auto NewParm = SemaRef.SubstParmVarDecl( OldParm, TemplateArgs, indexAdjustment, NumExpansions, - ExpectParameterPack, EvaluateConstraints); + ExpectParameterPack, EvaluateConstraints, LateAttrs); if (NewParm && SemaRef.getLangOpts().OpenCL) SemaRef.deduceOpenCLAddressSpace(NewParm); return NewParm; } +bool TemplateInstantiator::TransformFunctionTypeParams( + SourceLocation Loc, ArrayRef<ParmVarDecl *> Params, + const QualType *ParamTypes, + const FunctionProtoType::ExtParameterInfo *ParamInfos, + SmallVectorImpl<QualType> &PTypes, SmallVectorImpl<ParmVarDecl *> *PVars, + Sema::ExtParameterInfoBuilder &PInfos, unsigned *LastParamTransformed) { + // A late-parsed attribute on a parameter may name a parameter declared after + // it, so transform all parameters then instantiate all late attributes. + Sema::LateInstantiatedAttrVec LateAttrs; + LocalInstantiationScope *Scope = SemaRef.CurrentInstantiationScope; + llvm::SaveAndRestore SaveLateAttrs(LateParmAttrs, &LateAttrs); + llvm::SaveAndRestore SaveLateScope(LateParmAttrScope, Scope); + bool Failed = inherited::TransformFunctionTypeParams( + Loc, Params, ParamTypes, ParamInfos, PTypes, PVars, PInfos, + LastParamTransformed); + // The attributes were deferred in this scope itself, so there is nothing to + // free on failure. + assert( + llvm::all_of(LateAttrs, [&](const Sema::LateInstantiatedAttribute &LA) { + return LA.Scope == Scope; + })); + if (Failed) + return true; + SemaRef.InstantiateLateAttrs(TemplateArgs, LateAttrs, Scope); + return false; +} + QualType TemplateInstantiator::BuildSubstTemplateTypeParmType( TypeLocBuilder &TLB, bool SuppressObjCLifetime, bool Final, Decl *AssociatedDecl, unsigned Index, UnsignedOrNone PackIndex, @@ -3166,11 +3220,10 @@ bool Sema::SubstTypeConstraint( : SourceLocation()); } -ParmVarDecl * -Sema::SubstParmVarDecl(ParmVarDecl *OldParm, - const MultiLevelTemplateArgumentList &TemplateArgs, - int indexAdjustment, UnsignedOrNone NumExpansions, - bool ExpectParameterPack, bool EvaluateConstraint) { +ParmVarDecl *Sema::SubstParmVarDecl( + ParmVarDecl *OldParm, const MultiLevelTemplateArgumentList &TemplateArgs, + int indexAdjustment, UnsignedOrNone NumExpansions, bool ExpectParameterPack, + bool EvaluateConstraint, LateInstantiatedAttrVec *LateAttrs) { TypeSourceInfo *OldTSI = OldParm->getTypeSourceInfo(); TypeSourceInfo *NewTSI = nullptr; @@ -3281,7 +3334,8 @@ Sema::SubstParmVarDecl(ParmVarDecl *OldParm, NewParm->setScopeInfo(OldParm->getFunctionScopeDepth(), OldParm->getFunctionScopeIndex() + indexAdjustment); - InstantiateAttrs(TemplateArgs, OldParm, NewParm); + InstantiateAttrs(TemplateArgs, OldParm, NewParm, LateAttrs, + CurrentInstantiationScope); NewParm->deduceParmAddressSpace(Context); @@ -3294,11 +3348,18 @@ bool Sema::SubstParmTypes( const MultiLevelTemplateArgumentList &TemplateArgs, SmallVectorImpl<QualType> &ParamTypes, SmallVectorImpl<ParmVarDecl *> *OutParams, - ExtParameterInfoBuilder &ParamInfos) { + ExtParameterInfoBuilder &ParamInfos, const QualType *OldParamTypes, + bool EvaluateConstraints) { + // If there is a parameter with no ParmVarDecl to substitute, its type needs + // to come from OldParamTypes instead. + assert((OldParamTypes || !llvm::is_contained(Params, nullptr)) && + "a parameter with no ParmVarDecl needs OldParamTypes to be passed"); TemplateInstantiator Instantiator(*this, TemplateArgs, Loc, DeclarationName()); - return Instantiator.TransformFunctionTypeParams( - Loc, Params, nullptr, ExtParamInfos, ParamTypes, OutParams, ParamInfos); + Instantiator.setEvaluateConstraints(EvaluateConstraints); + return Instantiator.TransformFunctionTypeParams(Loc, Params, OldParamTypes, + ExtParamInfos, ParamTypes, + OutParams, ParamInfos); } bool Sema::SubstDefaultArgument( @@ -3742,27 +3803,9 @@ bool Sema::InstantiateClassImpl( ActOnFinishCXXNonNestedClass(); // Instantiate late parsed attributes, and attach them to their decls. - // See Sema::InstantiateAttrs - for (LateInstantiatedAttrVec::iterator I = LateAttrs.begin(), - E = LateAttrs.end(); I != E; ++I) { - assert(CurrentInstantiationScope == Instantiator.getStartingScope()); - CurrentInstantiationScope = I->Scope; - - // Allow 'this' within late-parsed attributes. - auto *ND = cast<NamedDecl>(I->NewDecl); - auto *ThisContext = dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); - CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(), - ND->isCXXInstanceMember()); - - Attr *NewAttr = - instantiateTemplateAttribute(I->TmplAttr, Context, *this, TemplateArgs); - if (NewAttr && checkInstantiatedThreadSafetyAttrs(I->NewDecl, NewAttr)) - I->NewDecl->addAttr(NewAttr); - LocalInstantiationScope::deleteScopes(I->Scope, - Instantiator.getStartingScope()); - } + InstantiateLateAttrs(TemplateArgs, LateAttrs, + Instantiator.getStartingScope()); Instantiator.disableLateAttributeInstantiation(); - LateAttrs.clear(); ActOnFinishDelayedMemberInitializers(Instantiation); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index b4881d0c6db5c..eb612f4f35e19 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -876,6 +876,24 @@ attrToRetainOwnershipKind(const Attr *A) { } } +/// Instantiate \p TmplAttr for \p New in the current instantiation scope and +/// attach it, if it is still valid there. +static void +instantiateAttrOnDecl(Sema &S, + const MultiLevelTemplateArgumentList &TemplateArgs, + const Attr *TmplAttr, Decl *New) { + // Allow 'this' within late-parsed attributes. + auto *ND = cast<NamedDecl>(New); + auto *ThisContext = dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); + Sema::CXXThisScopeRAII ThisScope(S, ThisContext, Qualifiers(), + ND->isCXXInstanceMember()); + + Attr *NewAttr = + sema::instantiateTemplateAttribute(TmplAttr, S.Context, S, TemplateArgs); + if (NewAttr && S.checkInstantiatedThreadSafetyAttrs(New, NewAttr)) + New->addAttr(NewAttr); +} + // Implementation is down with the rest of the OpenACC Decl instantiations. static void instantiateDependentOpenACCRoutineDeclAttr( Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs, @@ -1068,28 +1086,37 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs, assert(!TmplAttr->isPackExpansion()); if (TmplAttr->isLateParsed() && LateAttrs) { - // Late parsed attributes must be instantiated and attached after the - // enclosing class has been instantiated. See Sema::InstantiateClass. - LocalInstantiationScope *Saved = nullptr; - if (CurrentInstantiationScope) - Saved = CurrentInstantiationScope->cloneScopes(OuterMostScope); + // A late-parsed attribute may name a declaration that is instantiated + // after the one it is on: a class member's names a later member, and a + // parameter's a later parameter. Defer it, with the scope to instantiate + // it in, until they all are; see InstantiateLateAttrs. + assert(CurrentInstantiationScope && "no scope to defer the attribute in"); + LocalInstantiationScope *Saved = CurrentInstantiationScope->cloneScopes(OuterMostScope); LateAttrs->push_back(LateInstantiatedAttribute(TmplAttr, Saved, New)); } else { - // Allow 'this' within late-parsed attributes. - auto *ND = cast<NamedDecl>(New); - auto *ThisContext = dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); - CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(), - ND->isCXXInstanceMember()); - - Attr *NewAttr = sema::instantiateTemplateAttribute(TmplAttr, Context, - *this, TemplateArgs); - if (NewAttr && isRelevantAttr(*this, New, TmplAttr) && - checkInstantiatedThreadSafetyAttrs(New, NewAttr)) - New->addAttr(NewAttr); + instantiateAttrOnDecl(*this, TemplateArgs, TmplAttr, New); } } } +void Sema::InstantiateLateAttrs( + const MultiLevelTemplateArgumentList &TemplateArgs, + Lat... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/232784 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
