https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/124386
>From 4f4e65818c2fd85814efc63f779026853820ad76 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov <mizve...@gmail.com> Date: Fri, 24 Jan 2025 19:25:38 -0300 Subject: [PATCH] [clang] fix template argument conversion Converted template arguments need to be converted again, if the corresponding template parameter changed, as different conversions might apply in that case. --- clang/docs/ReleaseNotes.rst | 3 + clang/include/clang/AST/Expr.h | 10 ++ clang/include/clang/Sema/Sema.h | 2 + clang/lib/AST/TemplateBase.cpp | 12 +-- clang/lib/Sema/SemaTemplate.cpp | 145 +++++++++++++++++++--------- clang/lib/Sema/TreeTransform.h | 6 +- clang/test/SemaTemplate/cwg2398.cpp | 20 ---- 7 files changed, 122 insertions(+), 76 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b89d055304f4a6..27574924a14a92 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -993,6 +993,9 @@ Bug Fixes to C++ Support - Fix immediate escalation not propagating through inherited constructors. (#GH112677) - Fixed assertions or false compiler diagnostics in the case of C++ modules for lambda functions or inline friend functions defined inside templates (#GH122493). +- Fix template argument checking so that converted template arguments are + converted again. This fixes some issues with partial ordering involving + template template parameters with non-type template parameters. Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 708c8656decbe0..919025bec4d6da 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -2262,6 +2262,16 @@ class UnaryOperator final } public: + enum OnStack_t { OnStack }; + UnaryOperator(OnStack_t _, const ASTContext &Ctx, Expr *input, Opcode opc, + QualType type, ExprValueKind VK, ExprObjectKind OK, + SourceLocation l, bool CanOverflow, + FPOptionsOverride FPFeatures) + : UnaryOperator(Ctx, input, opc, type, VK, OK, l, CanOverflow, + FPFeatures) { + assert(!FPFeatures.requiresTrailingStorage()); + } + static UnaryOperator *CreateEmpty(const ASTContext &C, bool hasFPFeatures); static UnaryOperator *Create(const ASTContext &C, Expr *input, Opcode opc, diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 1ea7c62cb36f05..0a7782fe7cc7e7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -11753,6 +11753,8 @@ class Sema final : public SemaBase { /// If an error occurred, it returns ExprError(); otherwise, it /// returns the converted template argument. \p ParamType is the /// type of the non-type template parameter after it has been instantiated. + /// \p Arg is the input template argument expression to be converted. + /// this expression may not necessarily outlive the converted result. ExprResult CheckTemplateArgument(NonTypeTemplateParmDecl *Param, QualType InstantiatedParamType, Expr *Arg, TemplateArgument &SugaredConverted, diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp index 3625b6e435a556..0eef8f305fcb39 100644 --- a/clang/lib/AST/TemplateBase.cpp +++ b/clang/lib/AST/TemplateBase.cpp @@ -515,19 +515,17 @@ void TemplateArgument::print(const PrintingPolicy &Policy, raw_ostream &Out, } case Declaration: { - NamedDecl *ND = getAsDecl(); + ValueDecl *VD = getAsDecl(); if (getParamTypeForDecl()->isRecordType()) { - if (auto *TPO = dyn_cast<TemplateParamObjectDecl>(ND)) { + if (auto *TPO = dyn_cast<TemplateParamObjectDecl>(VD)) { TPO->getType().getUnqualifiedType().print(Out, Policy); TPO->printAsInit(Out, Policy); break; } } - if (auto *VD = dyn_cast<ValueDecl>(ND)) { - if (needsAmpersandOnTemplateArg(getParamTypeForDecl(), VD->getType())) - Out << "&"; - } - ND->printQualifiedName(Out); + if (needsAmpersandOnTemplateArg(getParamTypeForDecl(), VD->getType())) + Out << "&"; + VD->printQualifiedName(Out); break; } diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 210df2836eeb07..7abe0ed4f1c133 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -5199,7 +5199,7 @@ convertTypeTemplateArgumentToTemplate(ASTContext &Context, TypeLoc TLoc) { } bool Sema::CheckTemplateArgument( - NamedDecl *Param, TemplateArgumentLoc &Arg, NamedDecl *Template, + NamedDecl *Param, TemplateArgumentLoc &ArgLoc, NamedDecl *Template, SourceLocation TemplateLoc, SourceLocation RAngleLoc, unsigned ArgumentPackIndex, SmallVectorImpl<TemplateArgument> &SugaredConverted, @@ -5208,9 +5208,10 @@ bool Sema::CheckTemplateArgument( bool PartialOrderingTTP, bool *MatchedPackOnParmToNonPackOnArg) { // Check template type parameters. if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(Param)) - return CheckTemplateTypeArgument(TTP, Arg, SugaredConverted, + return CheckTemplateTypeArgument(TTP, ArgLoc, SugaredConverted, CanonicalConverted); + const TemplateArgument &Arg = ArgLoc.getArgument(); // Check non-type template parameters. if (NonTypeTemplateParmDecl *NTTP =dyn_cast<NonTypeTemplateParmDecl>(Param)) { // Do substitution on the type of the non-type template parameter @@ -5252,63 +5253,114 @@ bool Sema::CheckTemplateArgument( return true; } - switch (Arg.getArgument().getKind()) { - case TemplateArgument::Null: - llvm_unreachable("Should never see a NULL template argument here"); - - case TemplateArgument::Expression: { - Expr *E = Arg.getArgument().getAsExpr(); + auto checkExpr = [&](Expr *E) -> Expr * { TemplateArgument SugaredResult, CanonicalResult; unsigned CurSFINAEErrors = NumSFINAEErrors; ExprResult Res = CheckTemplateArgument(NTTP, NTTPType, E, SugaredResult, CanonicalResult, PartialOrderingTTP, CTAK); - if (Res.isInvalid()) - return true; // If the current template argument causes an error, give up now. - if (CurSFINAEErrors < NumSFINAEErrors) - return true; + if (Res.isInvalid() || CurSFINAEErrors < NumSFINAEErrors) + return nullptr; + SugaredConverted.push_back(SugaredResult); + CanonicalConverted.push_back(CanonicalResult); + return Res.get(); + }; + switch (Arg.getKind()) { + case TemplateArgument::Null: + llvm_unreachable("Should never see a NULL template argument here"); + + case TemplateArgument::Expression: { + Expr *E = Arg.getAsExpr(); + Expr *R = checkExpr(E); + if (!R) + return true; // If the resulting expression is new, then use it in place of the // old expression in the template argument. - if (Res.get() != E) { - TemplateArgument TA(Res.get()); - Arg = TemplateArgumentLoc(TA, Res.get()); + if (R != E) { + TemplateArgument TA(R); + ArgLoc = TemplateArgumentLoc(TA, R); } - - SugaredConverted.push_back(SugaredResult); - CanonicalConverted.push_back(CanonicalResult); break; } - case TemplateArgument::Declaration: - case TemplateArgument::Integral: + // As for the converted NTTP kinds, they still might need another + // conversion, as the new corresponding parameter might be different. + // Ideally, we would always perform substitution starting with sugared types + // and never need these, as we would still have expressions. Since these are + // needed so rarely, it's probably a better tradeoff to just convert them + // back to expressions. + case TemplateArgument::Integral: { + IntegerLiteral ILE(Context, Arg.getAsIntegral(), Arg.getIntegralType(), + SourceLocation()); + if (!checkExpr(&ILE)) + return true; + break; + } + case TemplateArgument::Declaration: { + bool needsAddrOf = false; + ValueDecl *VD = Arg.getAsDecl(); + QualType ParamType = Arg.getParamTypeForDecl(), DeclType = VD->getType(); + QualType DeclRefType; + auto handlePtrType = [&](auto *P) { + QualType PointeeType = P->getPointeeType(); + if (!Context.hasSameType(P->getPointeeType(), DeclType)) + return; + needsAddrOf = true; + DeclRefType = PointeeType; + }; + if (auto *P = ParamType->getAs<PointerType>()) + handlePtrType(P); + else if (auto *P = ParamType->getAs<MemberPointerType>()) + handlePtrType(P); + else + DeclRefType = ParamType->castAs<ReferenceType>()->getPointeeType(); + + DeclRefExpr DRE(Context, Arg.getAsDecl(), + /*RefersToEnclosingVariableOrCapture=*/false, DeclRefType, + VK_LValue, SourceLocation()); + if (needsAddrOf) { + UnaryOperator UOE(UnaryOperator::OnStack, Context, &DRE, UO_AddrOf, + ParamType, VK_PRValue, OK_Ordinary, SourceLocation(), + /*CanOverflow=*/false, /*FPFeatures=*/{}); + if (!checkExpr(&UOE)) + return true; + } else { + if (!checkExpr(&DRE)) + return true; + } + break; + } + case TemplateArgument::NullPtr: { + CXXNullPtrLiteralExpr NPLE(Arg.getNullPtrType(), SourceLocation()); + if (!checkExpr(&NPLE)) + return true; + break; + } case TemplateArgument::StructuralValue: - case TemplateArgument::NullPtr: - // We've already checked this template argument, so just copy - // it to the list of converted arguments. - SugaredConverted.push_back(Arg.getArgument()); - CanonicalConverted.push_back( - Context.getCanonicalTemplateArgument(Arg.getArgument())); + // FIXME: Is it even possible to reach here? + // If this is actually used, this needs to convert the argument again. + SugaredConverted.push_back(Arg); + CanonicalConverted.push_back(Context.getCanonicalTemplateArgument(Arg)); break; case TemplateArgument::Template: case TemplateArgument::TemplateExpansion: // We were given a template template argument. It may not be ill-formed; // see below. - if (DependentTemplateName *DTN - = Arg.getArgument().getAsTemplateOrTemplatePattern() - .getAsDependentTemplateName()) { + if (DependentTemplateName *DTN = Arg.getAsTemplateOrTemplatePattern() + .getAsDependentTemplateName()) { // We have a template argument such as \c T::template X, which we // parsed as a template template argument. However, since we now // know that we need a non-type template argument, convert this // template name into an expression. DeclarationNameInfo NameInfo(DTN->getIdentifier(), - Arg.getTemplateNameLoc()); + ArgLoc.getTemplateNameLoc()); CXXScopeSpec SS; - SS.Adopt(Arg.getTemplateQualifierLoc()); + SS.Adopt(ArgLoc.getTemplateQualifierLoc()); // FIXME: the template-template arg was a DependentTemplateName, // so it was provided with a template keyword. However, its source // location is not stored in the template argument structure. @@ -5319,8 +5371,8 @@ bool Sema::CheckTemplateArgument( // If we parsed the template argument as a pack expansion, create a // pack expansion expression. - if (Arg.getArgument().getKind() == TemplateArgument::TemplateExpansion){ - E = ActOnPackExpansion(E.get(), Arg.getTemplateEllipsisLoc()); + if (Arg.getKind() == TemplateArgument::TemplateExpansion) { + E = ActOnPackExpansion(E.get(), ArgLoc.getTemplateEllipsisLoc()); if (E.isInvalid()) return true; } @@ -5340,8 +5392,8 @@ bool Sema::CheckTemplateArgument( // We have a template argument that actually does refer to a class // template, alias template, or template template parameter, and // therefore cannot be a non-type template argument. - Diag(Arg.getLocation(), diag::err_template_arg_must_be_expr) - << Arg.getSourceRange(); + Diag(ArgLoc.getLocation(), diag::err_template_arg_must_be_expr) + << ArgLoc.getSourceRange(); NoteTemplateParameterLocation(*Param); return true; @@ -5357,8 +5409,8 @@ bool Sema::CheckTemplateArgument( // // We warn specifically about this case, since it can be rather // confusing for users. - QualType T = Arg.getArgument().getAsType(); - SourceRange SR = Arg.getSourceRange(); + QualType T = Arg.getAsType(); + SourceRange SR = ArgLoc.getSourceRange(); if (T->isFunctionType()) Diag(SR.getBegin(), diag::err_template_arg_nontype_ambig) << SR << T; else @@ -5409,34 +5461,33 @@ bool Sema::CheckTemplateArgument( // When [the injected-class-name] is used [...] as a template-argument for // a template template-parameter [...] it refers to the class template // itself. - if (Arg.getArgument().getKind() == TemplateArgument::Type) { + if (Arg.getKind() == TemplateArgument::Type) { TemplateArgumentLoc ConvertedArg = convertTypeTemplateArgumentToTemplate( - Context, Arg.getTypeSourceInfo()->getTypeLoc()); + Context, ArgLoc.getTypeSourceInfo()->getTypeLoc()); if (!ConvertedArg.getArgument().isNull()) - Arg = ConvertedArg; + ArgLoc = ConvertedArg; } - switch (Arg.getArgument().getKind()) { + switch (Arg.getKind()) { case TemplateArgument::Null: llvm_unreachable("Should never see a NULL template argument here"); case TemplateArgument::Template: case TemplateArgument::TemplateExpansion: - if (CheckTemplateTemplateArgument(TempParm, Params, Arg, PartialOrdering, + if (CheckTemplateTemplateArgument(TempParm, Params, ArgLoc, PartialOrdering, MatchedPackOnParmToNonPackOnArg)) return true; - SugaredConverted.push_back(Arg.getArgument()); - CanonicalConverted.push_back( - Context.getCanonicalTemplateArgument(Arg.getArgument())); + SugaredConverted.push_back(Arg); + CanonicalConverted.push_back(Context.getCanonicalTemplateArgument(Arg)); break; case TemplateArgument::Expression: case TemplateArgument::Type: // We have a template template parameter but the template // argument does not refer to a template. - Diag(Arg.getLocation(), diag::err_template_arg_must_be_template) - << getLangOpts().CPlusPlus11; + Diag(ArgLoc.getLocation(), diag::err_template_arg_must_be_template) + << getLangOpts().CPlusPlus11; return true; case TemplateArgument::Declaration: diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 12680843a434a0..81e351af575a96 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -7335,8 +7335,10 @@ QualType TreeTransform<Derived>::TransformTemplateSpecializationType( NewTemplateArgs)) return QualType(); - // FIXME: maybe don't rebuild if all the template arguments are the same. - + // This needs to be rebuilt if either the arguments changed, or if the + // original template changed. If the template changed, and even if the + // arguments didn't change, these arguments might not correspond to their + // respective parameters, therefore needing conversions. QualType Result = getDerived().RebuildTemplateSpecializationType(Template, TL.getTemplateNameLoc(), diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp index 1d35b574176a7e..137b94ba2641de 100644 --- a/clang/test/SemaTemplate/cwg2398.cpp +++ b/clang/test/SemaTemplate/cwg2398.cpp @@ -663,58 +663,38 @@ namespace nttp_auto { namespace nttp_partial_order { namespace t1 { - // FIXME: This should pick the second overload. template<template<short> class TT1> void f(TT1<0>); - // new-note@-1 {{here}} template<template<int> class TT2> void f(TT2<0>) {} - // new-note@-1 {{here}} template<int> struct B {}; template void f<B>(B<0>); - // new-error@-1 {{ambiguous}} } // namespace t1 namespace t2 { - // FIXME: This should pick the second overload. struct A {} a; template<template<A&> class TT1> void f(TT1<a>); - // new-note@-1 {{here}} template<template<const A&> class TT2> void f(TT2<a>) {} - // new-note@-1 {{here}} template<const A&> struct B {}; template void f<B>(B<a>); - // new-error@-1 {{ambiguous}} } // namespace t2 namespace t3 { - // FIXME: This should pick the second overload. enum A {}; template<template<A> class TT1> void f(TT1<{}>); - // new-note@-1 {{here}} template<template<int> class TT2> void f(TT2<{}>) {} - // new-note@-1 {{here}} template<int> struct B {}; template void f<B>(B<{}>); - // new-error@-1 {{ambiguous}} } // namespace t3 namespace t4 { - // FIXME: This should pick the second overload. struct A {} a; template<template<A*> class TT1> void f(TT1<&a>); - // new-note@-1 {{here}} template<template<const A*> class TT2> void f(TT2<&a>) {} - // new-note@-1 {{here}} template<const A*> struct B {}; template void f<B>(B<&a>); - // new-error@-1 {{ambiguous}} } // namespace t4 namespace t5 { - // FIXME: This should pick the second overload. struct A { int m; }; template<template<int A::*> class TT1> void f(TT1<&A::m>); - // new-note@-1 {{here}} template<template<const int A::*> class TT2> void f(TT2<&A::m>) {} - // new-note@-1 {{here}} template<const int A::*> struct B {}; template void f<B>(B<&A::m>); - // new-error@-1 {{ambiguous}} } // namespace t5 namespace t6 { // FIXME: This should pick the second overload. _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits