Skip to content

Commit

Permalink
[clang] disallow narrowing when matching template template parameters
Browse files Browse the repository at this point in the history
This fixes the core issue described in P3579, following the design
intent of P0522 to not introduce any new cases where a template
template parameter match is allowed for a template which is not valid
for all possible uses.

With this patch, narrowing conversions is disallowed for TTP matching.

This reuses the existing machinery for diagnosing narrowing in
a converted constant expression.
Since P0522 is a DR and we apply it all the way back to C++98,
this brings that machinery to use in older standards, in this
very narrow scope of TTP matching.

This still doesn't solve the ambiguity when partial ordering NTTPs of
different integral types, this is blocked by a different bug which will
be fixed in a subsequent patch.
  • Loading branch information
mizvekov committed Jan 24, 2025
1 parent 28ad897 commit 1d96050
Show file tree
Hide file tree
Showing 25 changed files with 199 additions and 97 deletions.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,9 @@ C++17 Feature Support
^^^^^^^^^^^^^^^^^^^^^
- The implementation of the relaxed template template argument matching rules is
more complete and reliable, and should provide more accurate diagnostics.
This implements:
- `P3310R5: Solving issues introduced by relaxed template template parameter matching <https://wg21.link/p3310r5>`_.
- `P3579R0: Fix matching of non-type template parameters when matching template template parameters <https://wg21.link/p3579r0>`_.

Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ def err_typecheck_converted_constant_expression_indirect : Error<
"conversion from %0 to %1 in converted constant expression would "
"bind reference to a temporary">;
def err_expr_not_cce : Error<
"%select{case value|enumerator value|non-type template argument|"
"%select{case value|enumerator value|non-type template argument|non-type parameter of template template parameter|"
"array size|explicit specifier argument|noexcept specifier argument|"
"call to 'size()'|call to 'data()'}0 is not a constant expression">;
def ext_cce_narrowing : ExtWarn<
"%select{case value|enumerator value|non-type template argument|"
"%select{case value|enumerator value|non-type template argument|non-type parameter of template template parameter|"
"array size|explicit specifier argument|noexcept specifier argument|"
"call to 'size()'|call to 'data()'}0 %select{cannot be narrowed from "
"type %2 to %3|evaluates to %2, which cannot be narrowed to type %3}1">,
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -9999,6 +9999,7 @@ class Sema final : public SemaBase {
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.
Expand Down Expand Up @@ -11682,6 +11683,7 @@ class Sema final : public SemaBase {
SmallVectorImpl<TemplateArgument> &SugaredConverted,
SmallVectorImpl<TemplateArgument> &CanonicalConverted,
CheckTemplateArgumentKind CTAK, bool PartialOrdering,
bool PartialOrderingTTP,
bool *MatchedPackOnParmToNonPackOnArg);

/// Check that the given template arguments can be provided to
Expand Down Expand Up @@ -11755,6 +11757,7 @@ class Sema final : public SemaBase {
QualType InstantiatedParamType, Expr *Arg,
TemplateArgument &SugaredConverted,
TemplateArgument &CanonicalConverted,
bool PartialOrderingTTP,
CheckTemplateArgumentKind CTAK);

/// Check a template argument against its corresponding
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3676,7 +3676,7 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
if (CheckTemplateArgument(
Params->getParam(0), Arg, FD, R.getNameLoc(), R.getNameLoc(),
0, SugaredChecked, CanonicalChecked, CTAK_Specified,
/*PartialOrdering=*/false,
/*PartialOrdering=*/false, /*PartialOrderingTTP=*/false,
/*MatchedPackOnParmToNonPackOnArg=*/nullptr) ||
Trap.hasErrorOccurred())
IsTemplate = false;
Expand Down
23 changes: 16 additions & 7 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6151,8 +6151,8 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From,
Sema::CCEKind CCE,
NamedDecl *Dest,
APValue &PreNarrowingValue) {
assert(S.getLangOpts().CPlusPlus11 &&
"converted constant expression outside C++11");
assert((S.getLangOpts().CPlusPlus11 || CCE == Sema::CCEK_InjectedTTP) &&
"converted constant expression outside C++11 or TTP matching");

if (checkPlaceholderForOverload(S, From))
return ExprError();
Expand Down Expand Up @@ -6221,8 +6221,10 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From,
// earlier, but that's not guaranteed to work when initializing an object of
// class type.
ExprResult Result;
bool IsTemplateArgument =
CCE == Sema::CCEK_TemplateArg || CCE == Sema::CCEK_InjectedTTP;
if (T->isRecordType()) {
assert(CCE == Sema::CCEK_TemplateArg &&
assert(IsTemplateArgument &&
"unexpected class type converted constant expr");
Result = S.PerformCopyInitialization(
InitializedEntity::InitializeTemplateParameter(
Expand All @@ -6239,7 +6241,7 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From,
// A full-expression is [...] a constant-expression [...]
Result = S.ActOnFinishFullExpr(Result.get(), From->getExprLoc(),
/*DiscardedValue=*/false, /*IsConstexpr=*/true,
CCE == Sema::CCEKind::CCEK_TemplateArg);
IsTemplateArgument);
if (Result.isInvalid())
return Result;

Expand All @@ -6248,9 +6250,6 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From,
QualType PreNarrowingType;
switch (SCS->getNarrowingKind(S.Context, Result.get(), PreNarrowingValue,
PreNarrowingType)) {
case NK_Dependent_Narrowing:
// Implicit conversion to a narrower type, but the expression is
// value-dependent so we can't tell whether it's actually narrowing.
case NK_Variable_Narrowing:
// Implicit conversion to a narrower type, and the value is not a constant
// expression. We'll diagnose this in a moment.
Expand All @@ -6271,6 +6270,14 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From,
<< PreNarrowingValue.getAsString(S.Context, PreNarrowingType) << T;
break;

case NK_Dependent_Narrowing:
// Implicit conversion to a narrower type, but the expression is
// 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)
break;
[[fallthrough]];
case NK_Type_Narrowing:
// FIXME: It would be better to diagnose that the expression is not a
// constant expression.
Expand Down Expand Up @@ -6343,6 +6350,8 @@ Sema::EvaluateConvertedConstantExpression(Expr *E, QualType T, APValue &Value,
Expr::EvalResult Eval;
Eval.Diag = &Notes;

assert(CCE != Sema::CCEK_InjectedTTP && "unnexpected CCE Kind");

ConstantExprKind Kind;
if (CCE == Sema::CCEK_TemplateArg && T->isRecordType())
Kind = ConstantExprKind::ClassTemplateArgument;
Expand Down
44 changes: 26 additions & 18 deletions clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5205,7 +5205,7 @@ bool Sema::CheckTemplateArgument(
SmallVectorImpl<TemplateArgument> &SugaredConverted,
SmallVectorImpl<TemplateArgument> &CanonicalConverted,
CheckTemplateArgumentKind CTAK, bool PartialOrdering,
bool *MatchedPackOnParmToNonPackOnArg) {
bool PartialOrderingTTP, bool *MatchedPackOnParmToNonPackOnArg) {
// Check template type parameters.
if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(Param))
return CheckTemplateTypeArgument(TTP, Arg, SugaredConverted,
Expand Down Expand Up @@ -5260,8 +5260,9 @@ bool Sema::CheckTemplateArgument(
Expr *E = Arg.getArgument().getAsExpr();
TemplateArgument SugaredResult, CanonicalResult;
unsigned CurSFINAEErrors = NumSFINAEErrors;
ExprResult Res = CheckTemplateArgument(NTTP, NTTPType, E, SugaredResult,
CanonicalResult, CTAK);
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.
Expand Down Expand Up @@ -5326,7 +5327,8 @@ bool Sema::CheckTemplateArgument(

TemplateArgument SugaredResult, CanonicalResult;
E = CheckTemplateArgument(NTTP, NTTPType, E.get(), SugaredResult,
CanonicalResult, CTAK_Specified);
CanonicalResult, /*PartialOrderingTTP=*/false,
CTAK_Specified);
if (E.isInvalid())
return true;

Expand Down Expand Up @@ -5585,11 +5587,11 @@ bool Sema::CheckTemplateArgumentList(
getExpandedPackSize(*Param))
Arg = Arg.getPackExpansionPattern();
TemplateArgumentLoc NewArgLoc(Arg, ArgLoc.getLocInfo());
if (CheckTemplateArgument(*Param, NewArgLoc, Template, TemplateLoc,
RAngleLoc, SugaredArgumentPack.size(),
SugaredConverted, CanonicalConverted,
CTAK_Specified, /*PartialOrdering=*/false,
MatchedPackOnParmToNonPackOnArg))
if (CheckTemplateArgument(
*Param, NewArgLoc, Template, TemplateLoc, RAngleLoc,
SugaredArgumentPack.size(), SugaredConverted,
CanonicalConverted, CTAK_Specified, /*PartialOrdering=*/false,
/*PartialOrderingTTP=*/true, MatchedPackOnParmToNonPackOnArg))
return true;
Arg = NewArgLoc.getArgument();
CanonicalConverted.back().setIsDefaulted(
Expand All @@ -5601,11 +5603,11 @@ bool Sema::CheckTemplateArgumentList(
TemplateArgumentLoc(TemplateArgument::CreatePackCopy(Context, Args),
ArgLoc.getLocInfo());
} else {
if (CheckTemplateArgument(*Param, ArgLoc, Template, TemplateLoc,
RAngleLoc, SugaredArgumentPack.size(),
SugaredConverted, CanonicalConverted,
CTAK_Specified, /*PartialOrdering=*/false,
MatchedPackOnParmToNonPackOnArg))
if (CheckTemplateArgument(
*Param, ArgLoc, Template, TemplateLoc, RAngleLoc,
SugaredArgumentPack.size(), SugaredConverted,
CanonicalConverted, CTAK_Specified, /*PartialOrdering=*/false,
PartialOrderingTTP, MatchedPackOnParmToNonPackOnArg))
return true;
CanonicalConverted.back().setIsDefaulted(
clang::isSubstitutedDefaultArgument(Context, ArgLoc.getArgument(),
Expand Down Expand Up @@ -5753,6 +5755,7 @@ bool Sema::CheckTemplateArgumentList(
if (CheckTemplateArgument(*Param, Arg, Template, TemplateLoc, RAngleLoc, 0,
SugaredConverted, CanonicalConverted,
CTAK_Specified, /*PartialOrdering=*/false,
/*PartialOrderingTTP=*/false,
/*MatchedPackOnParmToNonPackOnArg=*/nullptr))
return true;

Expand Down Expand Up @@ -6740,6 +6743,7 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param,
QualType ParamType, Expr *Arg,
TemplateArgument &SugaredConverted,
TemplateArgument &CanonicalConverted,
bool PartialOrderingTTP,
CheckTemplateArgumentKind CTAK) {
SourceLocation StartLoc = Arg->getBeginLoc();

Expand Down Expand Up @@ -6930,17 +6934,21 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param,
IsConvertedConstantExpression = false;
}

if (getLangOpts().CPlusPlus17) {
if (getLangOpts().CPlusPlus17 || PartialOrderingTTP) {
// 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.
APValue Value;
ExprResult ArgResult;
if (IsConvertedConstantExpression) {
ArgResult = BuildConvertedConstantExpression(Arg, ParamType,
CCEK_TemplateArg, Param);
if (ArgResult.isInvalid())
ArgResult = BuildConvertedConstantExpression(
Arg, ParamType,
PartialOrderingTTP ? CCEK_InjectedTTP : CCEK_TemplateArg, Param);
assert(!ArgResult.isUnset());
if (ArgResult.isInvalid()) {
NoteTemplateParameterLocation(*Param);
return ExprError();
}
} else {
ArgResult = Arg;
}
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/Sema/SemaTemplateDeduction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2979,7 +2979,8 @@ static bool ConvertDeducedTemplateArgument(
? (Arg.wasDeducedFromArrayBound() ? Sema::CTAK_DeducedFromArrayBound
: Sema::CTAK_Deduced)
: Sema::CTAK_Specified,
PartialOrdering, &MatchedPackOnParmToNonPackOnArg);
PartialOrdering, /*PartialOrderingTTP=*/false,
&MatchedPackOnParmToNonPackOnArg);
if (MatchedPackOnParmToNonPackOnArg)
Info.setMatchedPackOnParmToNonPackOnArg();
return Res;
Expand Down Expand Up @@ -3179,6 +3180,7 @@ static TemplateDeductionResult ConvertDeducedTemplateArguments(
Param, DefArg, TD, TD->getLocation(), TD->getSourceRange().getEnd(),
/*ArgumentPackIndex=*/0, SugaredBuilder, CanonicalBuilder,
Sema::CTAK_Specified, /*PartialOrdering=*/false,
/*PartialOrderingTTP=*/false,
/*MatchedPackOnParmToNonPackOnArg=*/nullptr)) {
Info.Param = makeTemplateParameter(
const_cast<NamedDecl *>(TemplateParams->getParam(I)));
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2399,9 +2399,10 @@ TemplateInstantiator::TransformSubstNonTypeTemplateParmExpr(
// The call to CheckTemplateArgument here produces the ImpCast.
TemplateArgument SugaredConverted, CanonicalConverted;
if (SemaRef
.CheckTemplateArgument(E->getParameter(), SubstType,
SubstReplacement.get(), SugaredConverted,
CanonicalConverted, Sema::CTAK_Specified)
.CheckTemplateArgument(
E->getParameter(), SubstType, SubstReplacement.get(),
SugaredConverted, CanonicalConverted,
/*PartialOrderingTTP=*/false, Sema::CTAK_Specified)
.isInvalid())
return true;
return transformNonTypeTemplateParmRef(E->getAssociatedDecl(),
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CXX/drs/cwg0xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -521,12 +521,12 @@ namespace example1 {
namespace A {
int i;
}

namespace A1 {
using A::i;
using A::i;
}

void f()
{
using A::i;
Expand Down Expand Up @@ -1371,7 +1371,7 @@ namespace cwg92 { // cwg92: 4 c++17
// considered in this context. In C++17, we *do* perform an implicit
// conversion (which performs initialization), and the exception specification
// is part of the type of the parameter, so this is invalid.
template<void() throw()> struct X {};
template<void() throw()> struct X {}; // since-cxx17-note {{template parameter is declared here}}
X<&f> xp;
// since-cxx17-error@-1 {{value of type 'void (*)() throw(int, float)' is not implicitly convertible to 'void (*)() throw()'}}

Expand Down
2 changes: 2 additions & 0 deletions clang/test/CXX/drs/cwg12xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ namespace cwg1295 { // cwg1295: 4
// cxx98-14-error@-1 {{non-type template argument does not refer to any declaration}}
// cxx98-14-note@#cwg1295-Y {{template parameter is declared here}}
// since-cxx17-error@#cwg1295-y {{reference cannot bind to bit-field in converted constant expression}}
// since-cxx17-note@#cwg1295-Y {{template parameter is declared here}}


#if __cplusplus >= 201103L
const unsigned other = 0;
Expand Down
Loading

0 comments on commit 1d96050

Please sign in to comment.