[clang] disallow narrowing when matching template template parameters #124313
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mizvekov
requested review from
zygoloid,
cor3ntin,
erichkeane,
sdkrystian and
zyn0217
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Matheus Izvekov (mizvekov) ChangesThis 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 are disallowed for TTP matching. This reuses the existing machinery for diagnosing narrowing in a converted constant expression. 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 (but the test cases are added). Patch is 45.61 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124313.diff 25 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f110b8cf765075..a2745d682616a4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -329,6 +329,7 @@ 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>`_ and `P3579R0: Fix matching of non-type template parameters when matching template template parameters <https://wg21.link/p3579r0>`_.
Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 774e5484cfa0e7..6be67a36b9fe7d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -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">,
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 4d6e02fe2956e0..1ea7c62cb36f05 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -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.
@@ -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
@@ -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
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 2ed8d3608d49ec..26f53532a1491f 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -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;
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 6ae9c51c06b315..20eaf494e90bb8 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -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();
@@ -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(
@@ -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;
@@ -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.
@@ -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.
@@ -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;
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 38196c5c2bc125..f3d7749c32f918 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -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,
@@ -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.
@@ -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;
@@ -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(
@@ -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(),
@@ -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;
@@ -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();
@@ -6930,17 +6934,20 @@ 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);
+ if (ArgResult.isInvalid()) {
+ NoteTemplateParameterLocation(*Param);
return ExprError();
+ }
} else {
ArgResult = Arg;
}
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 7882d7a755d345..2b96692727a7c8 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -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;
@@ -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)));
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 3dc5696bd38216..862086a0c53408 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -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(),
diff --git a/clang/test/CXX/drs/cwg0xx.cpp b/clang/test/CXX/drs/cwg0xx.cpp
index 15f469440c66f2..44a0eb520af221 100644
--- a/clang/test/CXX/drs/cwg0xx.cpp
+++ b/clang/test/CXX/drs/cwg0xx.cpp
@@ -521,12 +521,12 @@ namespace example1 {
namespace A {
int i;
}
-
+
namespace A1 {
using A::i;
using A::i;
}
-
+
void f()
{
using A::i;
@@ -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()'}}
diff --git a/clang/test/CXX/drs/cwg12xx.cpp b/clang/test/CXX/drs/cwg12xx.cpp
index 344adb6d720231..e02a7e11b80b2a 100644
--- a/clang/test/CXX/drs/cwg12xx.cpp
+++ b/clang/test/CXX/drs/cwg12xx.cpp
@@ -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;
diff --git a/clang/test/CXX/drs/cwg3xx.cpp b/clang/test/CXX/drs/cwg3xx.cpp
index b5e07a66bb4eda..6c420ecd4c91db 100644
--- a/clang/test/CXX/drs/cwg3xx.cpp
+++ b/clang/test/CXX/drs/cwg3xx.cpp
@@ -444,7 +444,7 @@ namespace cwg329 { // cwg329: 3.5
// expected-note@#cwg329-b {{in instantiation of template class 'cwg329::A<char>' requested here}}
// expected-note@#cwg329-i {{previous definition is here}}
};
- A<int> a;
+ A<int> a;
A<char> b; // #cwg329-b
void test() {
@@ -688,9 +688,9 @@ namespace cwg341 { // cwg341: sup 1708
namespace B {
extern "C" int &cwg341_a = cwg341_a;
// expected-error@-1 {{redefinition of 'cwg341_a'}}
- // expected-note@#cwg341_a {{previous definition is here}}
+ // expected-note@#cwg341_a {{previous definition is here}}
}
- extern "C" void cwg341_b(); // #cwg341_b
+ extern "C" void cwg341_b(); // #cwg341_b
}
int cwg341_a;
// expected-error@-1 {{declaration of 'cwg341_a' in global scope conflicts with declaration with C language linkage}}
@@ -708,7 +708,7 @@ namespace cwg341 {
// expected-error@-1 {{declaration of 'cwg341_d' with C language linkage conflicts with declaration in global scope}}
// expected-note@#cwg341_d {{declared in global scope here}}
- namespace A { extern "C" int cwg341_e; } // #cwg341_e
+ namespace A { extern "C" int cwg341_e; } // #cwg341_e
namespace B { extern "C" void cwg341_e(); }
// expected-error@-1 {{redefinition of 'cwg341_e' as different kind of symbol}}
// expected-note@#cwg341_e {{previous definition is here}}
@@ -960,6 +960,7 @@ namespace cwg354 { // cwg354: 3.1 c++11
// cxx11-14-error@#cwg354-p0 {{null non-type template argument must be cast to template parameter type 'int *'}}
// cxx11-14-note@#cwg354-ptr {{template parameter is declared here}}
// since-cxx17-error@#cwg354-p0 {{conversion from 'int' to 'int *' is not allowed in a converted constant expression}}
+ // since-cxx17-note@#cwg354-ptr {{template parameter is declared here}}
ptr<(int*)0> p1;
// cxx98-error@-1 {{non-type template argument does not refer to any declaration}}
// cxx98-note@#cwg354-ptr {{template parameter is declared here}}
@@ -969,12 +970,14 @@ namespace cwg354 { // cwg354: 3.1 c++11
// cxx11-14-error@#cwg354-p2 {{null non-type template argument of type 'float *' does not match template parameter of type 'int *'}}
// cxx11-14-note@#cwg354-ptr {{template parameter is declared here}}
// since-cxx17-error@#cwg354-p2 {{value of type 'float *' is not implicitly convertible to 'int *'}}
+ // since-cxx17-note@#cwg354-ptr {{template parameter is declared here}}
ptr<(int S::*)0> p3; // #...
[truncated]
|
erichkeane
reviewed
Jan 24, 2025
There was a problem hiding this comment.
I can't claim to completely understand what is going on here, but most of this seems to be pretty mechanical. I THINK this seems about right? 2 nits/style concerns, else this looks about right to me.
If someone else could spend some additional time on the tests making sure I didn't miss anything, I would appreciate it.
| @@ -11682,6 +11683,7 @@ class Sema final : public SemaBase { | |||
| SmallVectorImpl<TemplateArgument> &SugaredConverted, | |||
| SmallVectorImpl<TemplateArgument> &CanonicalConverted, | |||
| CheckTemplateArgumentKind CTAK, bool PartialOrdering, | |||
| bool PartialOrderingTTP, | |||
There was a problem hiding this comment.
Find myself wishing we could combine this with at least PartialOrdering in an enum of some sort.
There was a problem hiding this comment.
In this case, they are orthogonal and all four combinations are valid. Maybe renaming s/PartialOrderingTTP/MatchingTTP would make that more clear, but there are tons of pre-existing users.
There was a problem hiding this comment.
Thats always the balance unfortunately. I hate having 2 bools in a row like this, particularly when they are both SO similar in name/meaning. I would still love an enum with 4 values, but I can wait to see if others are as concerned as I am, and disagree and commit if necessary.
There was a problem hiding this comment.
We can turn the two bools into enums, and we can group parameters which often go together into an 'Info' like thing. Grouping ortogonal bools into one enum like that will create friction in that we will often want to combine and split them apart anyway.
There was a problem hiding this comment.
Ping, how may we proceed?
I don't think a good solution to this would come in this patch, as we have the same problem in multiple places, and this would add a lot of noise here.
We are due a refactor of these partial ordering functions, and I'd prefer to tackle that on a followup.
There was a problem hiding this comment.
This is a case where I would really like to stop the bleeding, so I'd like to find a better solution than just a pair of bools here. If you have a patch to do the refactor, it would be nice to land that first to see its effect in action.
There was a problem hiding this comment.
Done on top of the patch stack: #124668
mizvekov
force-pushed
the
users/mizvekov/clang-cwg2398-nttp-narrowing
branch
2 times, most recently
from
7a7950d to
1d96050
Compare
cor3ntin
reviewed
Jan 25, 2025
Comment on lines
+69
to
+71
| using err2 = tT0<short, i>; | ||
| using err2a = tT0<long long, i>; // expected-error@#tT0 {{cannot be narrowed from type 'long long' to 'int'}} | ||
| // expected-note@-1 {{different template parameters}} |
There was a problem hiding this comment.
Do we have any tests for floating points and enumeration types?
There was a problem hiding this comment.
We never allowed non-exact match for floats, so this would be a different problem. It would certainly be the design intention of P0522 to accept non-narrowing non-exact matches there.
Apparently only MSVC accepts non-exact matches here, so this would need some research and maybe a paper.
For enums sure, that works the same as integrals, I'll add a test case.
mizvekov
force-pushed
the
users/mizvekov/clang-cwg2398-nttp-narrowing
branch
from
1d96050 to
f2df46d
Compare
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.
mizvekov
force-pushed
the
users/mizvekov/clang-cwg2398-nttp-narrowing
branch
from
f2df46d to
8210ac8
Compare
erichkeane
approved these changes
Jan 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 are 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 (but the test cases are added).