llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) <details> <summary>Changes</summary> After implementation of CWG2369, declarations are instantiated on demand when evaluating a concept. This was hinged on TemplateInstantiator, where we intercepted most calls that need an instantiated declaration. This is almost identity to SetupConstraintScope, which tries to re-instantiate any declarations to the current scope. This patch removes SetupConstraintScope because we can instantiate anything on demand. The concept normalization patch brings us more troubles when we have to deal with sugars: declarations instantiated outside of the scope (e.g. a typedef declaration) are never added/instantiated during instantiation of concepts: this patch makes that function also look up outer scopes for those declarations. Note that we couldn't simply make the scope of concepts 'transparent', since we don't want clashes of instantiated function parameters. Also also this added a call to instantiateMissingDeclsToScopeForConcepts for C++26 fold expressions, before checking any pack sizes. Fixes https://github.com/llvm/llvm-project/issues/198052 Fixes https://github.com/llvm/llvm-project/issues/209632 --- Patch is 24.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/221707.diff 7 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+2) - (modified) clang/include/clang/Sema/Sema.h (+7-19) - (modified) clang/include/clang/Sema/Template.h (+2) - (modified) clang/lib/Sema/SemaConcept.cpp (+32-144) - (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+79-35) - (modified) clang/test/SemaCXX/cxx2c-fold-exprs.cpp (+26) - (modified) clang/test/SemaTemplate/concepts-lambda.cpp (+16) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 82772714b5d46..5f265255c5bf7 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -514,6 +514,8 @@ features cannot lower the translation-unit ABI level; - Fixed a crash when module directive export module foo not following a semicolon and there are no rest pp-tokens in current module file. (#GH187771) +- Fixed some bugs related to concepts. (#GH198052), (#GH209632) + - Fixed a crash when a lambda parameter pack was given a default argument that is a pack expansion referencing an enclosing function's parameter pack (e.g. `[](Types... = args...) {}`). Clang now diagnoses the illegal default diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4650bd53775f7..02c2aebd257a1 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -15126,11 +15126,17 @@ class Sema final : public SemaBase { const NamedDecl *D1, ArrayRef<AssociatedConstraint> AC1, const NamedDecl *D2, ArrayRef<AssociatedConstraint> AC2); +private: + friend class ConstraintSatisfactionChecker; + friend class SubstituteParameterMappings; + + UnsignedOrNone EvaluateFoldExpandedConstraintSize( + const Expr *Pattern, const MultiLevelTemplateArgumentList &MLTAL); + /// Cache the satisfaction of an atomic constraint. /// The key is based on the unsubstituted expression and the parameter /// mapping. This lets us not substituting the mapping more than once, /// which is (very!) expensive. - /// FIXME: this should be private. llvm::DenseMap<llvm::FoldingSetNodeID, UnsubstitutedConstraintSatisfactionCacheResult> UnsubstitutedConstraintSatisfactionCache; @@ -15142,7 +15148,6 @@ class Sema final : public SemaBase { llvm::DenseMap<llvm::FoldingSetNodeID, TemplateArgumentLoc> *CurrentCachedTemplateArgs = nullptr; -private: /// Caches pairs of template-like decls whose associated constraints were /// checked for subsumption and whether or not the first's constraints did in /// fact subsume the second's. @@ -15163,23 +15168,6 @@ class Sema final : public SemaBase { // The current stack of constraint satisfactions, so we can exit-early. llvm::SmallVector<SatisfactionStackEntryTy, 10> SatisfactionStack; - /// Used by SetupConstraintCheckingTemplateArgumentsAndScope to set up the - /// LocalInstantiationScope of the current non-lambda function. For lambdas, - /// use LambdaScopeForCallOperatorInstantiationRAII. - bool - SetupConstraintScope(FunctionDecl *FD, - std::optional<ArrayRef<TemplateArgument>> TemplateArgs, - const MultiLevelTemplateArgumentList &MLTAL, - LocalInstantiationScope &Scope); - - /// Used during constraint checking, sets up the constraint template argument - /// lists, and calls SetupConstraintScope to set up the - /// LocalInstantiationScope to have the proper set of ParVarDecls configured. - std::optional<MultiLevelTemplateArgumentList> - SetupConstraintCheckingTemplateArgumentsAndScope( - FunctionDecl *FD, std::optional<ArrayRef<TemplateArgument>> TemplateArgs, - LocalInstantiationScope &Scope); - ///@} // diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h index 50e950e56c6ca..818528744511d 100644 --- a/clang/include/clang/Sema/Template.h +++ b/clang/include/clang/Sema/Template.h @@ -540,6 +540,8 @@ enum class TemplateSubstitutionKind : char { llvm::PointerUnion<Decl *, DeclArgumentPack *> * getInstantiationOfIfExists(const Decl *D); + LocalInstantiationScope *getOuterScope() const { return Outer; } + void InstantiatedLocal(const Decl *D, Decl *Inst); void InstantiatedLocalPackArg(const Decl *D, VarDecl *Inst); void MakeInstantiatedLocalArgPack(const Decl *D); diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 1fb9485b411bd..780ce17afcd5f 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -530,7 +530,9 @@ class HashParameterMapping : public RecursiveASTVisitor<HashParameterMapping> { } } }; +} // namespace +namespace clang { class ConstraintSatisfactionChecker { Sema &S; const NamedDecl *Template; @@ -583,10 +585,6 @@ class ConstraintSatisfactionChecker { EvaluateAtomicConstraint(const Expr *AtomicExpr, const MultiLevelTemplateArgumentList &MLTAL); - UnsignedOrNone EvaluateFoldExpandedConstraintSize( - const FoldExpandedConstraint &FE, - const MultiLevelTemplateArgumentList &MLTAL); - // XXX: It is SLOW! Use it very carefully. std::optional<MultiLevelTemplateArgumentList> SubstitutionInTemplateArguments( const NormalizedConstraintWithParamMapping &Constraint, @@ -631,7 +629,7 @@ class ConstraintSatisfactionChecker { const MultiLevelTemplateArgumentList &MLTAL); }; -} // namespace +} // namespace clang ExprResult ConstraintSatisfactionChecker::EvaluateAtomicConstraint( const Expr *AtomicExpr, const MultiLevelTemplateArgumentList &MLTAL) { @@ -903,31 +901,6 @@ ExprResult ConstraintSatisfactionChecker::Evaluate( return E; } -UnsignedOrNone -ConstraintSatisfactionChecker::EvaluateFoldExpandedConstraintSize( - const FoldExpandedConstraint &FE, - const MultiLevelTemplateArgumentList &MLTAL) { - - Expr *Pattern = const_cast<Expr *>(FE.getPattern()); - - SmallVector<UnexpandedParameterPack, 2> Unexpanded; - S.collectUnexpandedParameterPacks(Pattern, Unexpanded); - assert(!Unexpanded.empty() && "Pack expansion without parameter packs?"); - bool Expand = true; - bool RetainExpansion = false; - UnsignedOrNone NumExpansions(std::nullopt); - if (S.CheckParameterPacksForExpansion( - Pattern->getExprLoc(), Pattern->getSourceRange(), Unexpanded, MLTAL, - /*FailOnPackProducingTemplates=*/false, Expand, RetainExpansion, - NumExpansions, /*Diagnose=*/false) || - !Expand || RetainExpansion) - return std::nullopt; - - if (NumExpansions && S.getLangOpts().BracketDepth < *NumExpansions) - return std::nullopt; - return NumExpansions; -} - ExprResult ConstraintSatisfactionChecker::EvaluateSlow( const FoldExpandedConstraint &Constraint, const MultiLevelTemplateArgumentList &MLTAL) { @@ -948,9 +921,15 @@ ExprResult ConstraintSatisfactionChecker::EvaluateSlow( return ExprError(); } - ExprResult Out; - UnsignedOrNone NumExpansions = - EvaluateFoldExpandedConstraintSize(Constraint, *SubstitutedArgs); + UnsignedOrNone NumExpansions(std::nullopt); + { + Sema::InstantiatingTemplate InstTemplate( + S, TemplateNameLoc, + Sema::InstantiatingTemplate::ConstraintSubstitution{}, + const_cast<NamedDecl *>(Template), Constraint.getSourceRange()); + NumExpansions = S.EvaluateFoldExpandedConstraintSize( + Constraint.getPattern(), *SubstitutedArgs); + } if (!NumExpansions) return ExprEmpty(); @@ -959,6 +938,7 @@ ExprResult ConstraintSatisfactionChecker::EvaluateSlow( return ExprEmpty(); } + ExprResult Out; for (unsigned I = 0; I < *NumExpansions; I++) { Sema::ArgPackSubstIndexRAII SubstIndex(S, I); Satisfaction.IsSatisfied = false; @@ -1410,98 +1390,6 @@ SubstituteConceptsInConstraintExpression(Sema &S, const NamedDecl *D, MLTAL); } -bool Sema::SetupConstraintScope( - FunctionDecl *FD, std::optional<ArrayRef<TemplateArgument>> TemplateArgs, - const MultiLevelTemplateArgumentList &MLTAL, - LocalInstantiationScope &Scope) { - assert(!isLambdaCallOperator(FD) && - "Use LambdaScopeForCallOperatorInstantiationRAII to handle lambda " - "instantiations"); - if (FD->isTemplateInstantiation() && FD->getPrimaryTemplate()) { - FunctionTemplateDecl *PrimaryTemplate = FD->getPrimaryTemplate(); - InstantiatingTemplate Inst( - *this, FD->getPointOfInstantiation(), - Sema::InstantiatingTemplate::ConstraintsCheck{}, PrimaryTemplate, - TemplateArgs ? *TemplateArgs : ArrayRef<TemplateArgument>{}, - SourceRange()); - if (Inst.isInvalid()) - return true; - - // addInstantiatedParametersToScope creates a map of 'uninstantiated' to - // 'instantiated' parameters and adds it to the context. For the case where - // this function is a template being instantiated NOW, we also need to add - // the list of current template arguments to the list so that they also can - // be picked out of the map. - if (auto *SpecArgs = FD->getTemplateSpecializationArgs()) { - MultiLevelTemplateArgumentList JustTemplArgs(FD, SpecArgs->asArray(), - /*Final=*/false); - if (addInstantiatedParametersToScope( - FD, PrimaryTemplate->getTemplatedDecl(), Scope, JustTemplArgs)) - return true; - } - - // If this is a member function, make sure we get the parameters that - // reference the original primary template. - if (FunctionTemplateDecl *FromMemTempl = - PrimaryTemplate->getInstantiatedFromMemberTemplate()) { - if (addInstantiatedParametersToScope(FD, FromMemTempl->getTemplatedDecl(), - Scope, MLTAL)) - return true; - } - - return false; - } - - if (FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization || - FD->getTemplatedKind() == FunctionDecl::TK_DependentNonTemplate) { - FunctionDecl *InstantiatedFrom = - FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization - ? FD->getInstantiatedFromMemberFunction() - : FD->getInstantiatedFromDecl(); - - InstantiatingTemplate Inst( - *this, FD->getPointOfInstantiation(), - Sema::InstantiatingTemplate::ConstraintsCheck{}, InstantiatedFrom, - TemplateArgs ? *TemplateArgs : ArrayRef<TemplateArgument>{}, - SourceRange()); - if (Inst.isInvalid()) - return true; - - // Case where this was not a template, but instantiated as a - // child-function. - if (addInstantiatedParametersToScope(FD, InstantiatedFrom, Scope, MLTAL)) - return true; - } - - return false; -} - -// This function collects all of the template arguments for the purposes of -// constraint-instantiation and checking. -std::optional<MultiLevelTemplateArgumentList> -Sema::SetupConstraintCheckingTemplateArgumentsAndScope( - FunctionDecl *FD, std::optional<ArrayRef<TemplateArgument>> TemplateArgs, - LocalInstantiationScope &Scope) { - MultiLevelTemplateArgumentList MLTAL; - - // Collect the list of template arguments relative to the 'primary' template. - // We need the entire list, since the constraint is completely uninstantiated - // at this point. - MLTAL = - getTemplateInstantiationArgs(FD, FD->getLexicalDeclContext(), - /*Final=*/false, /*Innermost=*/std::nullopt, - /*RelativeToPrimary=*/true, - /*Pattern=*/nullptr, - /*ForConstraintInstantiation=*/true); - // Lambdas are handled by LambdaScopeForCallOperatorInstantiationRAII. - if (isLambdaCallOperator(FD)) - return MLTAL; - if (SetupConstraintScope(FD, TemplateArgs, MLTAL, Scope)) - return std::nullopt; - - return MLTAL; -} - bool Sema::CheckFunctionConstraints(const FunctionDecl *FD, ConstraintSatisfaction &Satisfaction, SourceLocation UsageLoc, @@ -1541,12 +1429,12 @@ bool Sema::CheckFunctionConstraints(const FunctionDecl *FD, ContextRAII SavedContext{*this, CtxToSave}; LocalInstantiationScope Scope(*this, !ForOverloadResolution); - std::optional<MultiLevelTemplateArgumentList> MLTAL = - SetupConstraintCheckingTemplateArgumentsAndScope( - const_cast<FunctionDecl *>(FD), {}, Scope); - - if (!MLTAL) - return true; + MultiLevelTemplateArgumentList MLTAL = + getTemplateInstantiationArgs(FD, FD->getLexicalDeclContext(), + /*Final=*/false, /*Innermost=*/std::nullopt, + /*RelativeToPrimary=*/true, + /*Pattern=*/nullptr, + /*ForConstraintInstantiation=*/true); Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; @@ -1557,11 +1445,11 @@ bool Sema::CheckFunctionConstraints(const FunctionDecl *FD, CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr); LambdaScopeForCallOperatorInstantiationRAII LambdaScope( - *this, const_cast<FunctionDecl *>(FD), *MLTAL, Scope, + *this, const_cast<FunctionDecl *>(FD), MLTAL, Scope, ForOverloadResolution); return CheckConstraintSatisfaction( - FD, FD->getTrailingRequiresClause(), *MLTAL, + FD, FD->getTrailingRequiresClause(), MLTAL, SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()), Satisfaction); } @@ -1794,12 +1682,12 @@ bool Sema::CheckFunctionTemplateConstraints( Sema::ContextRAII savedContext(*this, Decl); LocalInstantiationScope Scope(*this); - std::optional<MultiLevelTemplateArgumentList> MLTAL = - SetupConstraintCheckingTemplateArgumentsAndScope(Decl, TemplateArgs, - Scope); - - if (!MLTAL) - return true; + MultiLevelTemplateArgumentList MLTAL = + getTemplateInstantiationArgs(Decl, Decl->getLexicalDeclContext(), + /*Final=*/false, /*Innermost=*/std::nullopt, + /*RelativeToPrimary=*/true, + /*Pattern=*/nullptr, + /*ForConstraintInstantiation=*/true); Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; @@ -1809,10 +1697,10 @@ bool Sema::CheckFunctionTemplateConstraints( } CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr); - LambdaScopeForCallOperatorInstantiationRAII LambdaScope(*this, Decl, *MLTAL, + LambdaScopeForCallOperatorInstantiationRAII LambdaScope(*this, Decl, MLTAL, Scope); - return CheckConstraintSatisfaction(Template, TemplateAC, *MLTAL, + return CheckConstraintSatisfaction(Template, TemplateAC, MLTAL, PointOfInstantiation, Satisfaction); } @@ -2085,7 +1973,7 @@ void Sema::DiagnoseUnsatisfiedConstraint( ConstraintExpr->getBeginLoc(), First); } -namespace { +namespace clang { class SubstituteParameterMappings { Sema &SemaRef; @@ -2123,6 +2011,8 @@ class SubstituteParameterMappings { bool substitute(NormalizedConstraint &N); }; +} // namespace clang + void SubstituteParameterMappings::buildParameterMapping( NormalizedConstraintWithParamMapping &N) { TemplateParameterList *TemplateParams = @@ -2416,8 +2306,6 @@ bool SubstituteParameterMappings::substitute(NormalizedConstraint &N) { llvm_unreachable("Unknown ConstraintKind enum"); } -} // namespace - NormalizedConstraint *NormalizedConstraint::fromAssociatedConstraints( Sema &S, const NamedDecl *D, ArrayRef<AssociatedConstraint> ACs) { assert(ACs.size() != 0); diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index ffbe8bb0506bc..8975eeba37363 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -1324,11 +1324,10 @@ namespace { bool BailOutOnIncomplete; std::optional<llvm::FoldingSetNodeID> TemplateArgsHashValue; + llvm::DenseMap<llvm::FoldingSetNodeID, TemplateArgumentLoc> + *CurrentCachedTemplateArgs = nullptr; - // CWG2770: Function parameters should be instantiated when they are - // needed by a satisfaction check of an atomic constraint or - // (recursively) by another function parameter. - bool maybeInstantiateFunctionParameterToScope(ParmVarDecl *OldParm); + bool instantiateMissingDeclsToScopeForConcepts(Decl *OldParm); public: typedef TreeTransform<TemplateInstantiator> inherited; @@ -1358,12 +1357,14 @@ namespace { inline static struct ForConstraintSubstitution_t { } ForConstraintSubstitution; - TemplateInstantiator(ForParameterMappingSubstitution_t, Sema &SemaRef, - SourceLocation Loc, - const MultiLevelTemplateArgumentList &TemplateArgs) + TemplateInstantiator( + ForParameterMappingSubstitution_t, Sema &SemaRef, SourceLocation Loc, + const MultiLevelTemplateArgumentList &TemplateArgs, + llvm::DenseMap<llvm::FoldingSetNodeID, TemplateArgumentLoc> *Cache) : inherited(SemaRef), TemplateArgs(TemplateArgs), Loc(Loc), - EvaluateLambdaConstraint(true), BailOutOnIncomplete(false) { - if (!SemaRef.CurrentCachedTemplateArgs) + EvaluateLambdaConstraint(true), BailOutOnIncomplete(false), + CurrentCachedTemplateArgs(Cache) { + if (!Cache) return; auto &V = TemplateArgsHashValue.emplace(); for (auto &Level : TemplateArgs) @@ -1410,22 +1411,18 @@ namespace { ArrayRef<UnexpandedParameterPack> Unexpanded, bool FailOnPackProducingTemplates, bool &ShouldExpand, bool &RetainExpansion, - UnsignedOrNone &NumExpansions) { - if (SemaRef.CurrentInstantiationScope && - (SemaRef.inConstraintSubstitution() || - SemaRef.inParameterMappingSubstitution())) { - for (UnexpandedParameterPack ParmPack : Unexpanded) { - NamedDecl *VD = ParmPack.first.dyn_cast<NamedDecl *>(); - if (auto *PVD = dyn_cast_if_present<ParmVarDecl>(VD); - PVD && maybeInstantiateFunctionParameterToScope(PVD)) - return true; - } + UnsignedOrNone &NumExpansions, + bool Diagnose = true) { + for (UnexpandedParameterPack ParmPack : Unexpanded) { + if (instantiateMissingDeclsToScopeForConcepts( + dyn_cast<NamedDecl *>(ParmPack.first))) + return true; } return getSema().CheckParameterPacksForExpansion( EllipsisLoc, PatternRange, Unexpanded, TemplateArgs, FailOnPackProducingTemplates, ShouldExpand, RetainExpansion, - NumExpansions); + NumExpansions, Diagnose); } void ExpandingFunctionParameterPack(ParmVarDecl *Pack) { @@ -1637,7 +1634,7 @@ namespace { TemplateArgumentLoc &Output, bool Uneval = false) { const TemplateArgument &Arg = Input.getArgument(); - if (auto *Cache = SemaRef.CurrentCachedTemplateArgs; + if (auto *Cache = CurrentCachedTemplateArgs; Cache && TemplateArgsHashValue) { llvm::FoldingSetNodeID ID = *TemplateArgsHashValue; ID.AddInteger(SemaRef.ArgPackSubstIndex.toInternalRepresentation()); @@ -1980,11 +1977,7 @@ Decl *TemplateInstantiator::TransformDecl(SourceLocation Loc, Decl *D) { // template parameter. } - if (ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(D); - PVD && SemaRef.CurrentInstantiationScope && - (SemaRef.inConstraintSubstitution() || - SemaRef.inParameterMappingSubstitution()) && - maybeInstantiateFunctionParameterToScope(PVD)) + if (instantiateMissingDeclsToScopeForConcepts(D)) return nullptr; if (isa<CXXExpansionStmtDecl>(D)) { @@ -1996,11 +1989,41 @@ Decl *TemplateInstantiator::TransformDecl(SourceLocation Loc, Decl *D) { return SemaRef.FindInstantiatedDecl(Loc, cast<NamedDecl>(D), TemplateArgs); } -bool TemplateInstantiator::maybeInstantiateFunctionParameterToScope( - ParmVarDecl *OldParm) { - if (SemaRef.CurrentInstantiationScope->getInstantiationOfIfExists(OldParm)) +bool TemplateInstantiator::instantiateMissingDeclsToScopeForConcepts(Decl *PD) { + if (!(PD && (SemaRef.inConstraintSubstitution() || + SemaRef.inParameterMappingSubstitution()))) + return false; + + auto *Current = SemaRef.CurrentInstantiationScope; + if (!Current) + return false; + if (Current->getInstant... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/221707 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
