Skip to content

[Clang][RFC] Do not eat SFINAE diagnostics for explicit template arguments #139066

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+68 −24
Open

[Clang][RFC] Do not eat SFINAE diagnostics for explicit template arguments #139066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6ee1af
[Clang] Do not eat SFINAE diagnostics for explicit template arguments
zyn0217 May 8, 2025
9fa53c1
Fix tests
zyn0217 May 9, 2025
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
19 changes: 18 additions & 1 deletion clang/lib/Sema/SemaOverload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,8 +749,16 @@ clang::MakeDeductionFailureInfo(ASTContext &Context,
break;

case TemplateDeductionResult::Incomplete:
Result.Data = Info.Param.getOpaqueValue();
break;
case TemplateDeductionResult::InvalidExplicitArguments:
Result.Data = Info.Param.getOpaqueValue();
if (Info.hasSFINAEDiagnostic()) {
PartialDiagnosticAt *Diag = new (Result.Diagnostic) PartialDiagnosticAt(
SourceLocation(), PartialDiagnostic::NullDiagnostic());
Info.takeSFINAEDiagnostic(*Diag);
Result.HasDiagnostic = true;
}
break;

case TemplateDeductionResult::DeducedMismatch:
Expand Down Expand Up @@ -822,7 +830,6 @@ void DeductionFailureInfo::Destroy() {
case TemplateDeductionResult::Incomplete:
case TemplateDeductionResult::TooManyArguments:
case TemplateDeductionResult::TooFewArguments:
case TemplateDeductionResult::InvalidExplicitArguments:
case TemplateDeductionResult::CUDATargetMismatch:
case TemplateDeductionResult::NonDependentConversionFailure:
break;
Expand All @@ -837,6 +844,7 @@ void DeductionFailureInfo::Destroy() {
Data = nullptr;
break;

case TemplateDeductionResult::InvalidExplicitArguments:
case TemplateDeductionResult::SubstitutionFailure:
// FIXME: Destroy the template argument list?
Data = nullptr;
Expand Down Expand Up @@ -12166,6 +12174,15 @@ static void DiagnoseBadDeduction(Sema &S, NamedDecl *Found, Decl *Templated,
diag::note_ovl_candidate_explicit_arg_mismatch_unnamed)
<< (index + 1);
}

if (PartialDiagnosticAt *PDiag = DeductionFailure.getSFINAEDiagnostic()) {
unsigned DiagID =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you going through all this work to emit the string that is already in the partial diagnostic? ALSO, why not use the location in the Partial Diagnostic?

You should be able to do:

S.Diag(PDiag.first, PDiag.second).

Copy link
Contributor Author

@zyn0217 zyn0217 May 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Because SFINAE diagnostics basically take the form of errors, I think it's more reasonable to emit them as notes when enumerating failed candidates, to help reduce noise, like how SubstitutionFailure below works.

  2. That behavior comes from how we draw squiggle lines: if the next note's location differs from the current one, a new code snippet is introduced. But in this context, we've already printed a source snippet in the "candidate is ..." line, so from my perspective, it's clearer if there's just a single line attached to that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2 makes sense to me.

For #1: It seems to me that a way to 'downgrade' a diagnostic to a note is a better solution here. I dont' really know what that looks like and might require a bit of a trip through the DiagnosticsEngine (@AaronBallman for visibility), but I would vastly prefer that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont' really know what that looks like and might require a bit of a trip through the DiagnosticsEngine

SGTM. I'll look into it and come back with an answer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obviously a step in the right direction, but bear in mind that this is still limited to a single error, and in some cases it can be hard to understand that error without the notes which follow it.

An alternative here, instead of degrading the level of diagnostic, would be to come up with a way to establish a nesting of diagnostics, beyond what we do implicitly, with notes always attached to the previous non-note diagnostic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative here, instead of degrading the level of diagnostic, would be to come up with a way to establish a nesting of diagnostics, beyond what we do implicitly, with notes always attached to the previous non-note diagnostic.

Can you please say more? Our current deduction failure diagnostics are already implemented in the way that note diagnostics (like SFINAE errors) are attached to the error, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @mizvekov Is saying that if we could "attach" a diagnostics to another, all nested diagnostics would automatically could become notes.

But this is a fairly involved project...

S.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Note, "%0");
SmallString<128> SFINAEArgString;
PDiag->second.EmitToString(S.getDiagnostics(), SFINAEArgString);
S.Diag(Templated->getLocation(), DiagID) << SFINAEArgString;
}

MaybeEmitInheritedConstructorNote(S, Found);
return;

Expand Down
3 changes: 2 additions & 1 deletion clang/test/AST/ByteCode/builtin-align-cxx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

// Check that we don't crash when using dependent types in __builtin_align:
template <typename a, a b>
void *c(void *d) { // both-note{{candidate template ignored}}
void *c(void *d) { // both-note{{candidate template ignored}} \
// both-note {{a non-type template parameter cannot have type 'struct x' before C++20}}
return __builtin_align_down(d, b);
}

Expand Down
3 changes: 2 additions & 1 deletion clang/test/AST/ByteCode/cxx20.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,8 @@ namespace FailingDestructor {
}
};
template<D d>
void f() {} // both-note {{invalid explicitly-specified argument}}
void f() {} // both-note {{invalid explicitly-specified argument}} \
// both-note {{non-type template argument is not a constant expression}}

void g() {
f<D{0, false}>(); // both-error {{no matching function}}
Expand Down
12 changes: 8 additions & 4 deletions clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,21 +113,25 @@ namespace test3 {

struct Derived1 : Base {
using Base::foo;
template <int n> Opaque<2> foo() { return Opaque<2>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'n'}}
template <int n> Opaque<2> foo() { return Opaque<2>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'n'}} \
// expected-note {{template argument for non-type template parameter must be an expression}}
};

struct Derived2 : Base {
template <int n> Opaque<2> foo() { return Opaque<2>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'n'}}
template <int n> Opaque<2> foo() { return Opaque<2>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'n'}} \
// expected-note {{template argument for non-type template parameter must be an expression}}
using Base::foo;
};

struct Derived3 : Base {
using Base::foo;
template <class T> Opaque<3> foo() { return Opaque<3>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'T'}}
template <class T> Opaque<3> foo() { return Opaque<3>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'T'}} \
// expected-note {{template argument for template type parameter must be a type}}
};

struct Derived4 : Base {
template <class T> Opaque<3> foo() { return Opaque<3>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'T'}}
template <class T> Opaque<3> foo() { return Opaque<3>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'T'}} \
// expected-note {{template argument for template type parameter must be a type}}
using Base::foo;
};

Expand Down
4 changes: 4 additions & 0 deletions clang/test/CXX/drs/cwg2xx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,14 +650,17 @@ namespace cwg241 { // cwg241: 9
C::f<3>(b);
// expected-error@-1 {{no matching function for call to 'f'}}
// expected-note@#cwg241-C-f {{candidate template ignored: invalid explicitly-specified argument for template parameter 'T'}}
// expected-note@#cwg241-C-f {{template argument for template type parameter must be a type}}
C::g<3>(b);
// expected-error@-1 {{no matching function for call to 'g'}}
// expected-note@#cwg241-C-g {{candidate template ignored: invalid explicitly-specified argument for template parameter 'T'}}
// expected-note@#cwg241-C-g {{template argument for template type parameter must be a type}}
using C::f;
using C::g;
f<3>(b);
// expected-error@-1 {{no matching function for call to 'f'}}
// expected-note@#cwg241-C-f {{candidate template ignored: invalid explicitly-specified argument for template parameter 'T'}}
// expected-note@#cwg241-C-f {{template argument for template type parameter must be a type}}
// expected-note@#cwg241-A-f {{candidate function template not viable: requires 0 arguments, but 1 was provided}}
g<3>(b);
}
Expand Down Expand Up @@ -952,6 +955,7 @@ namespace cwg258 { // cwg258: 2.8
int &x = b.g<int>(0);
// expected-error@-1 {{no matching member function for call to 'g'}}
// expected-note@#cwg258-B-g {{candidate template ignored: invalid explicitly-specified argument for 1st template parameter}}
// expected-note@#cwg258-B-g {{template argument for non-type template parameter must be an expression}}
int &y = b.h();
float &z = const_cast<const B&>(b).h();

Expand Down
2 changes: 2 additions & 0 deletions clang/test/CXX/drs/cwg3xx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,9 @@ namespace cwg354 { // cwg354: 3.1 c++11
int b1 = both<(int*)0>();
// cxx98-error@-1 {{no matching function for call to 'both'}}
// cxx98-note@#cwg354-both-int-ptr {{candidate template ignored: invalid explicitly-specified argument for 1st template parameter}}
// cxx98-note@#cwg354-both-int-ptr {{non-type template argument does not refer to any declaration}}
// cxx98-note@#cwg354-both-int {{candidate template ignored: invalid explicitly-specified argument for 1st template parameter}}
// cxx98-note@#cwg354-both-int {{non-type template argument of type 'int *' must have an integral or enumeration type}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the enumeration part, because you need two conversions to get from (unscoped) enum to pointer (via integer), and https://eel.is/c++draft/conv#general-1 doesn't seem to allow both in the same standard conversion sequence


template<int S::*> struct ptr_mem {}; // #cwg354-ptr_mem
ptr_mem<0> m0; // #cwg354-m0
Expand Down
3 changes: 2 additions & 1 deletion clang/test/CXX/expr/expr.const/p3-0x.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ void c() {
break;
}
}
template <bool B> int f() { return B; } // expected-note {{candidate template ignored: invalid explicitly-specified argument for template parameter 'B'}}
template <bool B> int f() { return B; } // expected-note {{candidate template ignored: invalid explicitly-specified argument for template parameter 'B'}} \
// expected-note {{conversion from 'int (S::*)() const' to 'bool' is not allowed in a converted constant expression}}
template int f<&S::operator int>(); // expected-error {{does not refer to a function template}}
template int f<(bool)&S::operator int>();

Expand Down
3 changes: 2 additions & 1 deletion clang/test/CXX/temp/temp.param/p8-cxx20.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ namespace ConstDestruction {
};

template<D d>
void f() {} // expected-note 2{{invalid explicitly-specified argument}}
void f() {} // expected-note 2{{invalid explicitly-specified argument}} \
// expected-note 2{{non-type template argument is not a constant expression}}

void g() {
f<D{0, true}>();
Expand Down
6 changes: 4 additions & 2 deletions clang/test/CXX/temp/temp.res/temp.local/p1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ template<typename> char id;
template<typename> struct TempType {};
template<template<typename> class> struct TempTemp {};

template<typename> void use(int&); // expected-note {{invalid explicitly-specified argument}} expected-note {{no known conversion}}
template<typename> void use(int&); // expected-note {{invalid explicitly-specified argument}} expected-note {{no known conversion}} \
// expected-note {{use of class template 'B::template C' requires template arguments}}
template<template<typename> class> void use(float&); // expected-note 2{{no known conversion}}
template<int> void use(char&); // expected-note 2{{invalid explicitly-specified argument}}
template<int> void use(char&); // expected-note 2{{invalid explicitly-specified argument}} \
// expected-note 2{{template argument for non-type template parameter must be an expression}}

template<typename T> struct A {
template<typename> struct C {};
Expand Down
2 changes: 2 additions & 0 deletions clang/test/Modules/cxx-templates.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@ void g() {

template_param_kinds_2<Tmpl_T_C>(); // expected-error {{no matching function for call}}
// expected-note@Inputs/cxx-templates-a.h:11 {{candidate}}
// expected-note@Inputs/cxx-templates-a.h:11 {{non-type parameter of template template parameter cannot be narrowed from type 'int' to 'char'}}
// expected-note@Inputs/cxx-templates-b.h:11 {{candidate}}

template_param_kinds_2<Tmpl_T_I_I>(); // expected-error {{ambiguous}}
// expected-note@Inputs/cxx-templates-a.h:11 {{candidate}}
// expected-note@Inputs/cxx-templates-b.h:11 {{candidate}}
// expected-note@Inputs/cxx-templates-b.h:11 {{non-type parameter of template template parameter cannot be narrowed from type 'int' to 'char'}}

template_param_kinds_3<Tmpl_T_T_A>();
template_param_kinds_3<Tmpl_T_T_B>();
Expand Down
3 changes: 2 additions & 1 deletion clang/test/SemaCXX/builtin-align-cxx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

// Check that we don't crash when using dependent types in __builtin_align:
template <typename a, a b>
void *c(void *d) { // expected-note{{candidate template ignored}}
void *c(void *d) { // expected-note{{candidate template ignored}} \
// expected-note {{a non-type template parameter cannot have type 'struct x' before C++20}}
return __builtin_align_down(d, b);
}

Expand Down
3 changes: 2 additions & 1 deletion clang/test/SemaCXX/calling-conv-compat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ namespace D50526 {
void h() { g<void, h>(); }
#if !_M_X64
// expected-error@-2 {{no matching function for call to}}
// expected-note@-4 {{invalid explicitly-specified argument}}
// expected-note@-4 {{invalid explicitly-specified argument}} \
// expected-note@-4 {{non-type template argument of type 'void ()' cannot be converted to a value of type 'void (*)() __attribute__((stdcall))'}}
#endif
}
3 changes: 2 additions & 1 deletion clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ constexpr void test8() {
throw "bad";
}

template<int x> constexpr int f(int y) { // expected-note {{candidate template ignored}}
template<int x> constexpr int f(int y) { // expected-note {{candidate template ignored}} \
// expected-note {{non-type template argument is not a constant expression}}
return x * y;
}
constexpr int test9(int x) {
Expand Down
6 changes: 4 additions & 2 deletions clang/test/SemaCXX/cxx2a-template-lambdas.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// RUN: %clang_cc1 -std=c++03 -verify -Dstatic_assert=_Static_assert -Wno-c++11-extensions -Wno-c++14-extensions -Wno-c++17-extensions -Wno-c++20-extensions %s
// RUN: %clang_cc1 -std=c++11 -verify=expected,cxx11,cxx11-cxx14 -Wno-c++20-extensions -Wno-c++17-extensions -Wno-c++14-extensions %s
// RUN: %clang_cc1 -std=c++14 -verify=expected,cxx11-cxx14,cxx14 -Wno-c++20-extensions -Wno-c++17-extensions %s
// RUN: %clang_cc1 -std=c++17 -verify -Wno-c++20-extensions %s
// RUN: %clang_cc1 -std=c++20 -verify %s
// RUN: %clang_cc1 -std=c++17 -verify=expected,cxx17,cxx17-cxx20 -Wno-c++20-extensions %s
// RUN: %clang_cc1 -std=c++20 -verify=expected,cxx20,cxx17-cxx20 %s

template<typename, typename>
inline const bool is_same = false;
Expand Down Expand Up @@ -47,6 +47,8 @@ constexpr T outer() {
return []<T x>() { return x; }.template operator()<123>(); // expected-error {{no matching member function}} \
expected-note {{candidate template ignored}} \
cxx11-note {{non-literal type '<dependent type>' cannot be used in a constant expression}} \
cxx11-cxx14-note {{non-type template argument does not refer to any declaration}} \
cxx17-cxx20-note {{value of type 'int' is not implicitly convertible to 'int *'}} \
cxx14-note {{non-literal type}}
}
static_assert(outer<int>() == 123); // cxx11-cxx14-error {{not an integral constant expression}} cxx11-cxx14-note {{in call}}
Expand Down
3 changes: 2 additions & 1 deletion clang/test/SemaCXX/typo-correction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,8 @@ int bar() {

namespace testIncludeTypeInTemplateArgument {
template <typename T, typename U>
void foo(T t = {}, U = {}); // expected-note {{candidate template ignored}}
void foo(T t = {}, U = {}); // expected-note {{candidate template ignored}} \
// expected-note {{template argument for template type parameter must be a type}}

class AddObservation {}; // expected-note {{declared here}}
int bar1() {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaTemplate/concepts-using-decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ struct base {

struct bar : public base {
using base::foo;
template <int N>
template <int N>
int foo() { return 2; }; // expected-note {{candidate template ignored: substitution failure: too many template arguments for function template 'foo'}}
};

Expand Down
10 changes: 6 additions & 4 deletions clang/test/SemaTemplate/overload-candidates.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ void test_dyn_cast(int* ptr) {
(void)dyn_cast(ptr); // expected-error{{no matching function for call to 'dyn_cast'}}
}

template<int I, typename T>
void get(const T&); // expected-note{{candidate template ignored: invalid explicitly-specified argument for template parameter 'I'}}
template<template<class T> class, typename T>
void get(const T&); // expected-note{{candidate template ignored: invalid explicitly-specified argument for 1st template parameter}}
template<int I, typename T>
void get(const T&); // expected-note{{candidate template ignored: invalid explicitly-specified argument for template parameter 'I'}} \
// expected-note {{template argument for non-type template parameter must be an expression}}
template<template<class T> class, typename T>
void get(const T&); // expected-note{{candidate template ignored: invalid explicitly-specified argument for 1st template parameter}} \
// expected-note {{template argument for template template parameter must be a class template}}

void test_get(void *ptr) {
get<int>(ptr); // expected-error{{no matching function for call to 'get'}}
Expand Down
5 changes: 3 additions & 2 deletions clang/test/SemaTemplate/temp_arg_nontype_cxx11.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ template <int a, unsigned b, int c>
void TempFunc() {}

void Useage() {
//expected-error@+2 {{no matching function}}
//expected-note@-4 {{candidate template ignored: invalid explicitly-specified argument for template parameter 'b'}}
//expected-error@+3 {{no matching function}}
//expected-note@-4 {{candidate template ignored: invalid explicitly-specified argument for template parameter 'b'}} \
//expected-note@-4 {{non-type template argument evaluates to -1, which cannot be narrowed to type 'unsigned int'}}
TempFunc<1, -1, 1>();
}
}
Expand Down
Loading