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
