Skip to content

[clang] fix template argument conversion #124386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 28, 2025
Merged

[clang] fix template argument conversion #124386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,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.
- Clang now rejects declaring an alias template with the same name as its template parameter. (#GH123423)
- Fixed the rejection of valid code when referencing an enumerator of an unscoped enum member with a prior declaration. (#GH124405)
- Fixed immediate escalation of non-dependent expressions. (#GH123405)
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11780,6 +11780,9 @@ class Sema final : public SemaBase {
/// declaration and the type of its corresponding non-type template
/// parameter, produce an expression that properly refers to that
/// declaration.
/// FIXME: This is used in some contexts where the resulting expression
/// doesn't need to live too long. It would be useful if this function
/// could return a temporary expression.
ExprResult BuildExpressionFromDeclTemplateArgument(
const TemplateArgument &Arg, QualType ParamType, SourceLocation Loc,
NamedDecl *TemplateParam = nullptr);
Expand Down
102 changes: 56 additions & 46 deletions clang/lib/Sema/SemaTemplate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -5252,63 +5253,73 @@ 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:
// 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:
case TemplateArgument::StructuralValue:
case TemplateArgument::Declaration:
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()));
case TemplateArgument::StructuralValue: {
// FIXME: StructuralValue is untested here.
ExprResult R =
BuildExpressionFromNonTypeTemplateArgument(Arg, SourceLocation());
assert(R.isUsable());
if (!checkExpr(R.get()))
return true;
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.
Expand All @@ -5319,8 +5330,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;
}
Expand All @@ -5340,8 +5351,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;
Expand All @@ -5357,8 +5368,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
Expand Down Expand Up @@ -5409,34 +5420,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:
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Sema/TreeTransform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
20 changes: 0 additions & 20 deletions clang/test/SemaTemplate/cwg2398.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down