llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) <details> <summary>Changes</summary> This fixes partial ordering of pack expansions of NTTPs, by procedding with the check using the pattern of the NTTP through the rules of the non-pack case. This also unifies almost all of the different versions of FinishTemplateArgumentDeduction (except the function template case). This makes sure they all follow the rules consistently, instantiating the parameters and comparing those with the argument. Fixes #<!-- -->132562 --- Patch is 66.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/134461.diff 15 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+1) - (modified) clang/include/clang/AST/ExprCXX.h (+3-3) - (modified) clang/include/clang/Sema/Sema.h (+9-8) - (modified) clang/lib/AST/ASTContext.cpp (+1-2) - (modified) clang/lib/AST/ASTImporter.cpp (+3-4) - (modified) clang/lib/Sema/SemaOverload.cpp (+4-4) - (modified) clang/lib/Sema/SemaTemplate.cpp (+161-111) - (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+107-174) - (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+4-4) - (modified) clang/lib/Sema/SemaTemplateVariadic.cpp (+1-2) - (modified) clang/lib/Sema/TreeTransform.h (+1-2) - (modified) clang/test/Import/pack-expansion-expr/test.cpp (+1-1) - (modified) clang/test/SemaTemplate/attributes.cpp (+11-11) - (modified) clang/test/SemaTemplate/partial-order.cpp (+38-3) - (modified) clang/test/SemaTemplate/temp_arg_nontype.cpp (+9-7) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index c521b56a98606..6756b0d924be6 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -385,6 +385,7 @@ Bug Fixes to C++ Support - Improved fix for an issue with pack expansions of type constraints, where this now also works if the constraint has non-type or template template parameters. (#GH131798) +- Fixes to partial ordering of non-type template parameter packs. (#GH132562) - Fix crash when evaluating the trailing requires clause of generic lambdas which are part of a pack expansion. - Fixes matching of nested template template parameters. (#GH130362) diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h index ac78d2faefe42..c613ce162a6a4 100644 --- a/clang/include/clang/AST/ExprCXX.h +++ b/clang/include/clang/AST/ExprCXX.h @@ -4209,10 +4209,10 @@ class PackExpansionExpr : public Expr { Stmt *Pattern; public: - PackExpansionExpr(QualType T, Expr *Pattern, SourceLocation EllipsisLoc, + PackExpansionExpr(Expr *Pattern, SourceLocation EllipsisLoc, UnsignedOrNone NumExpansions) - : Expr(PackExpansionExprClass, T, Pattern->getValueKind(), - Pattern->getObjectKind()), + : Expr(PackExpansionExprClass, Pattern->getType(), + Pattern->getValueKind(), Pattern->getObjectKind()), EllipsisLoc(EllipsisLoc), NumExpansions(NumExpansions ? *NumExpansions + 1 : 0), Pattern(Pattern) { diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b835697f99670..0f4377540bb51 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -10126,13 +10126,14 @@ class Sema final : public SemaBase { /// Contexts in which a converted constant expression is required. enum CCEKind { - CCEK_CaseValue, ///< Expression in a case label. - CCEK_Enumerator, ///< Enumerator value with fixed underlying type. - CCEK_TemplateArg, ///< Value of a non-type template parameter. - CCEK_InjectedTTP, ///< Injected parameter of a template template parameter. - CCEK_ArrayBound, ///< Array bound in array declarator or new-expression. - CCEK_ExplicitBool, ///< Condition in an explicit(bool) specifier. - CCEK_Noexcept, ///< Condition in a noexcept(bool) specifier. + CCEK_CaseValue, ///< Expression in a case label. + CCEK_Enumerator, ///< Enumerator value with fixed underlying type. + CCEK_TemplateArg, ///< Value of a non-type template parameter. + CCEK_TempArgStrict, ///< As above, but applies strict template checking + ///< rules. + CCEK_ArrayBound, ///< Array bound in array declarator or new-expression. + CCEK_ExplicitBool, ///< Condition in an explicit(bool) specifier. + CCEK_Noexcept, ///< Condition in a noexcept(bool) specifier. CCEK_StaticAssertMessageSize, ///< Call to size() in a static assert ///< message. CCEK_StaticAssertMessageData, ///< Call to data() in a static assert @@ -11894,7 +11895,7 @@ class Sema final : public SemaBase { QualType InstantiatedParamType, Expr *Arg, TemplateArgument &SugaredConverted, TemplateArgument &CanonicalConverted, - bool MatchingTTP, + bool StrictCheck, CheckTemplateArgumentKind CTAK); /// Check a template argument against its corresponding diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 1b6b3d06ddc1e..2123a3397f125 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -5852,8 +5852,7 @@ TemplateArgument ASTContext::getInjectedTemplateArg(NamedDecl *Param) const { T, VK, NTTP->getLocation()); if (NTTP->isParameterPack()) - E = new (*this) - PackExpansionExpr(DependentTy, E, NTTP->getLocation(), std::nullopt); + E = new (*this) PackExpansionExpr(E, NTTP->getLocation(), std::nullopt); Arg = TemplateArgument(E); } else { auto *TTP = cast<TemplateTemplateParmDecl>(Param); diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 8c91cce22f78e..f4b977d1d14b3 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -8273,14 +8273,13 @@ ASTNodeImporter::VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E) { ExpectedStmt ASTNodeImporter::VisitPackExpansionExpr(PackExpansionExpr *E) { Error Err = Error::success(); - auto ToType = importChecked(Err, E->getType()); - auto ToPattern = importChecked(Err, E->getPattern()); + auto *ToPattern = importChecked(Err, E->getPattern()); auto ToEllipsisLoc = importChecked(Err, E->getEllipsisLoc()); if (Err) return std::move(Err); - return new (Importer.getToContext()) PackExpansionExpr( - ToType, ToPattern, ToEllipsisLoc, E->getNumExpansions()); + return new (Importer.getToContext()) + PackExpansionExpr(ToPattern, ToEllipsisLoc, E->getNumExpansions()); } ExpectedStmt ASTNodeImporter::VisitSizeOfPackExpr(SizeOfPackExpr *E) { diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 0564557738170..f46ef2c7f5bd6 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -6201,7 +6201,7 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From, Sema::CCEKind CCE, NamedDecl *Dest, APValue &PreNarrowingValue) { - assert((S.getLangOpts().CPlusPlus11 || CCE == Sema::CCEK_InjectedTTP) && + assert((S.getLangOpts().CPlusPlus11 || CCE == Sema::CCEK_TempArgStrict) && "converted constant expression outside C++11 or TTP matching"); if (checkPlaceholderForOverload(S, From)) @@ -6272,7 +6272,7 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From, // class type. ExprResult Result; bool IsTemplateArgument = - CCE == Sema::CCEK_TemplateArg || CCE == Sema::CCEK_InjectedTTP; + CCE == Sema::CCEK_TemplateArg || CCE == Sema::CCEK_TempArgStrict; if (T->isRecordType()) { assert(IsTemplateArgument && "unexpected class type converted constant expr"); @@ -6325,7 +6325,7 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From, // value-dependent so we can't tell whether it's actually narrowing. // For matching the parameters of a TTP, the conversion is ill-formed // if it may narrow. - if (CCE != Sema::CCEK_InjectedTTP) + if (CCE != Sema::CCEK_TempArgStrict) break; [[fallthrough]]; case NK_Type_Narrowing: @@ -6400,7 +6400,7 @@ Sema::EvaluateConvertedConstantExpression(Expr *E, QualType T, APValue &Value, Expr::EvalResult Eval; Eval.Diag = &Notes; - assert(CCE != Sema::CCEK_InjectedTTP && "unnexpected CCE Kind"); + assert(CCE != Sema::CCEK_TempArgStrict && "unnexpected CCE Kind"); ConstantExprKind Kind; if (CCE == Sema::CCEK_TemplateArg && T->isRecordType()) diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 153f44f8ec67a..a7bcac50e561c 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -5263,9 +5263,9 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc, auto checkExpr = [&](Expr *E) -> Expr * { TemplateArgument SugaredResult, CanonicalResult; unsigned CurSFINAEErrors = NumSFINAEErrors; - ExprResult Res = - CheckTemplateArgument(NTTP, NTTPType, E, SugaredResult, - CanonicalResult, CTAI.MatchingTTP, CTAK); + ExprResult Res = CheckTemplateArgument( + NTTP, NTTPType, E, SugaredResult, CanonicalResult, + /*StrictCheck=*/CTAI.MatchingTTP || CTAI.PartialOrdering, CTAK); // If the current template argument causes an error, give up now. if (Res.isInvalid() || CurSFINAEErrors < NumSFINAEErrors) return nullptr; @@ -5344,9 +5344,9 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc, } TemplateArgument SugaredResult, CanonicalResult; - E = CheckTemplateArgument(NTTP, NTTPType, E.get(), SugaredResult, - CanonicalResult, /*PartialOrderingTTP=*/false, - CTAK_Specified); + E = CheckTemplateArgument( + NTTP, NTTPType, E.get(), SugaredResult, CanonicalResult, + /*StrictCheck=*/CTAI.PartialOrdering, CTAK_Specified); if (E.isInvalid()) return true; @@ -6757,9 +6757,21 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, QualType ParamType, Expr *Arg, TemplateArgument &SugaredConverted, TemplateArgument &CanonicalConverted, - bool PartialOrderingTTP, + bool StrictCheck, CheckTemplateArgumentKind CTAK) { SourceLocation StartLoc = Arg->getBeginLoc(); + auto *ArgPE = dyn_cast<PackExpansionExpr>(Arg); + Expr *DeductionArg = ArgPE ? ArgPE->getPattern() : Arg; + auto setDeductionArg = [&](Expr *NewDeductionArg) { + DeductionArg = NewDeductionArg; + if (ArgPE) { + // Recreate a pack expansion if we unwrapped one. + Arg = new (Context) PackExpansionExpr( + DeductionArg, ArgPE->getEllipsisLoc(), ArgPE->getNumExpansions()); + } else { + Arg = DeductionArg; + } + }; // If the parameter type somehow involves auto, deduce the type now. DeducedType *DeducedT = ParamType->getContainedDeducedType(); @@ -6769,7 +6781,7 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, // FIXME: The language rules don't say what happens in this case. // FIXME: We get an opaque dependent type out of decltype(auto) if the // expression is merely instantiation-dependent; is this enough? - if (Arg->isTypeDependent()) { + if (DeductionArg->isTypeDependent()) { auto *AT = dyn_cast<AutoType>(DeducedT); if (AT && AT->isDecltypeAuto()) { SugaredConverted = TemplateArgument(Arg); @@ -6782,9 +6794,6 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, // When checking a deduced template argument, deduce from its type even if // the type is dependent, in order to check the types of non-type template // arguments line up properly in partial ordering. - Expr *DeductionArg = Arg; - if (auto *PE = dyn_cast<PackExpansionExpr>(DeductionArg)) - DeductionArg = PE->getPattern(); TypeSourceInfo *TSI = Context.getTrivialTypeSourceInfo(ParamType, Param->getLocation()); if (isa<DeducedTemplateSpecializationType>(DeducedT)) { @@ -6837,64 +6846,55 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, assert(!ParamType.hasQualifiers() && "non-type template parameter type cannot be qualified"); + // If either the parameter has a dependent type or the argument is + // type-dependent, there's nothing we can check now. + if (ParamType->isDependentType() || DeductionArg->isTypeDependent()) { + // Force the argument to the type of the parameter to maintain invariants. + ExprResult E = ImpCastExprToType( + DeductionArg, ParamType.getNonLValueExprType(Context), CK_Dependent, + ParamType->isLValueReferenceType() ? VK_LValue + : ParamType->isRValueReferenceType() ? VK_XValue + : VK_PRValue); + if (E.isInvalid()) + return ExprError(); + setDeductionArg(E.get()); + SugaredConverted = TemplateArgument(Arg); + CanonicalConverted = TemplateArgument( + Context.getCanonicalTemplateArgument(SugaredConverted)); + return Arg; + } + // FIXME: When Param is a reference, should we check that Arg is an lvalue? - if (CTAK == CTAK_Deduced && + if (CTAK == CTAK_Deduced && !StrictCheck && (ParamType->isReferenceType() ? !Context.hasSameType(ParamType.getNonReferenceType(), - Arg->getType()) - : !Context.hasSameUnqualifiedType(ParamType, Arg->getType()))) { - // FIXME: If either type is dependent, we skip the check. This isn't - // correct, since during deduction we're supposed to have replaced each - // template parameter with some unique (non-dependent) placeholder. - // FIXME: If the argument type contains 'auto', we carry on and fail the - // type check in order to force specific types to be more specialized than - // 'auto'. It's not clear how partial ordering with 'auto' is supposed to - // work. Similarly for CTAD, when comparing 'A<x>' against 'A'. - if ((ParamType->isDependentType() || Arg->isTypeDependent()) && - !Arg->getType()->getContainedDeducedType()) { - SugaredConverted = TemplateArgument(Arg); - CanonicalConverted = TemplateArgument( - Context.getCanonicalTemplateArgument(SugaredConverted)); - return Arg; - } + DeductionArg->getType()) + : !Context.hasSameUnqualifiedType(ParamType, + DeductionArg->getType()))) { // FIXME: This attempts to implement C++ [temp.deduct.type]p17. Per DR1770, // we should actually be checking the type of the template argument in P, // not the type of the template argument deduced from A, against the // template parameter type. Diag(StartLoc, diag::err_deduced_non_type_template_arg_type_mismatch) - << Arg->getType() - << ParamType.getUnqualifiedType(); + << Arg->getType() << ParamType.getUnqualifiedType(); NoteTemplateParameterLocation(*Param); return ExprError(); } - // If either the parameter has a dependent type or the argument is - // type-dependent, there's nothing we can check now. - if (ParamType->isDependentType() || Arg->isTypeDependent()) { - // Force the argument to the type of the parameter to maintain invariants. - auto *PE = dyn_cast<PackExpansionExpr>(Arg); - if (PE) - Arg = PE->getPattern(); - ExprResult E = ImpCastExprToType( - Arg, ParamType.getNonLValueExprType(Context), CK_Dependent, - ParamType->isLValueReferenceType() ? VK_LValue - : ParamType->isRValueReferenceType() ? VK_XValue - : VK_PRValue); - if (E.isInvalid()) - return ExprError(); - if (PE) { - // Recreate a pack expansion if we unwrapped one. - E = new (Context) - PackExpansionExpr(E.get()->getType(), E.get(), PE->getEllipsisLoc(), - PE->getNumExpansions()); - } - SugaredConverted = TemplateArgument(E.get()); + // If the argument is a pack expansion, we don't know how many times it would + // expand. If we continue checking the argument, this will make the template + // definition ill-formed if it would be ill-formed for any number of + // expansions during insteantiation time. When partial ordering or matching + // template template parameters, this is exactly what we want. Otherwise, the + // normal template rules apply: we accept the template if it would be valid + // for any number of expansions (i.e. none). + if (ArgPE && !StrictCheck) { + SugaredConverted = TemplateArgument(Arg); CanonicalConverted = TemplateArgument( Context.getCanonicalTemplateArgument(SugaredConverted)); - return E; + return Arg; } - QualType CanonParamType = Context.getCanonicalType(ParamType); // Avoid making a copy when initializing a template parameter of class type // from a template parameter object of the same type. This is going beyond // the standard, but is required for soundness: in @@ -6903,15 +6903,15 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, // // Similarly, don't inject a call to a copy constructor when initializing // from a template parameter of the same type. - Expr *InnerArg = Arg->IgnoreParenImpCasts(); + Expr *InnerArg = DeductionArg->IgnoreParenImpCasts(); if (ParamType->isRecordType() && isa<DeclRefExpr>(InnerArg) && Context.hasSameUnqualifiedType(ParamType, InnerArg->getType())) { NamedDecl *ND = cast<DeclRefExpr>(InnerArg)->getDecl(); if (auto *TPO = dyn_cast<TemplateParamObjectDecl>(ND)) { SugaredConverted = TemplateArgument(TPO, ParamType); - CanonicalConverted = - TemplateArgument(TPO->getCanonicalDecl(), CanonParamType); + CanonicalConverted = TemplateArgument(TPO->getCanonicalDecl(), + ParamType.getCanonicalType()); return Arg; } if (isa<NonTypeTemplateParmDecl>(ND)) { @@ -6928,10 +6928,10 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, *this, Sema::ExpressionEvaluationContext::ConstantEvaluated); bool IsConvertedConstantExpression = true; - if (isa<InitListExpr>(Arg) || ParamType->isRecordType()) { + if (isa<InitListExpr>(DeductionArg) || ParamType->isRecordType()) { InitializationKind Kind = InitializationKind::CreateForInit( - Arg->getBeginLoc(), /*DirectInit=*/false, Arg); - Expr *Inits[1] = {Arg}; + StartLoc, /*DirectInit=*/false, DeductionArg); + Expr *Inits[1] = {DeductionArg}; InitializedEntity Entity = InitializedEntity::InitializeTemplateParameter(ParamType, Param); InitializationSequence InitSeq(*this, Entity, Kind, Inits); @@ -6941,14 +6941,15 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, Result = ActOnConstantExpression(Result.get()); if (Result.isInvalid() || !Result.get()) return ExprError(); - Arg = ActOnFinishFullExpr(Result.get(), Arg->getBeginLoc(), - /*DiscardedValue=*/false, - /*IsConstexpr=*/true, /*IsTemplateArgument=*/true) - .get(); + setDeductionArg(ActOnFinishFullExpr(Result.get(), Arg->getBeginLoc(), + /*DiscardedValue=*/false, + /*IsConstexpr=*/true, + /*IsTemplateArgument=*/true) + .get()); IsConvertedConstantExpression = false; } - if (getLangOpts().CPlusPlus17 || PartialOrderingTTP) { + if (getLangOpts().CPlusPlus17 || StrictCheck) { // C++17 [temp.arg.nontype]p1: // A template-argument for a non-type template parameter shall be // a converted constant expression of the type of the template-parameter. @@ -6956,24 +6957,25 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, ExprResult ArgResult; if (IsConvertedConstantExpression) { ArgResult = BuildConvertedConstantExpression( - Arg, ParamType, - PartialOrderingTTP ? CCEK_InjectedTTP : CCEK_TemplateArg, Param); + DeductionArg, ParamType, + StrictCheck ? CCEK_TempArgStrict : CCEK_TemplateArg, Param); assert(!ArgResult.isUnset()); if (ArgResult.isInvalid()) { NoteTemplateParameterLocation(*Param); return ExprError(); } } else { - ArgResult = Arg; + ArgResult = DeductionArg; } // For a value-dependent argument, CheckConvertedConstantExpression is // permitted (and expected) to be unable to determine a value. if (ArgResult.get()->isValueDependent()) { - SugaredConverted = TemplateArgument(ArgResult.get()); + setDeductionArg(ArgResult.get()); + SugaredConverted = TemplateArgument(Arg); CanonicalConverted = Context.getCanonicalTemplateArgument(SugaredConverted); - return ArgResult; + return Arg; } APValue PreNarrowingValue; @@ -6982,6 +6984,7 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, false, PreNarrowingValue); if (ArgResult.isInvalid()) return ExprError(); + setDeductionArg(ArgResult.get()); if (Value.isLValue()) {... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/134461 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
