[Clang] Clarify diagnostic notes for implicitly generated deduction guides #96084
Conversation
zyn0217
added
clang
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesGiven the following invalid code, template <class T>
struct S {
T *a;
};
S s = {1};we produce such diagnostics currently: Which I think is confusing because there's no This patch alleviates the issue by adding extra notes demonstrating which implicit deduction guide we're deducing against. In other words, in addition to the note of Patch is 30.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/96084.diff 23 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 69aea6c21ad39..423f1e9198948 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -580,6 +580,9 @@ Improvements to Clang's diagnostics
- Clang no longer emits a "declared here" note for a builtin function that has no declaration in source.
Fixes #GH93369.
+- Clang now emits implicit deduction guides corresponding to non-user-defined constructors while at a failure
+ of overload resolution.
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ab223f2b806d5..a65491b0c4399 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2405,6 +2405,7 @@ def err_selected_explicit_constructor : Error<
"chosen constructor is explicit in copy-initialization">;
def note_explicit_ctor_deduction_guide_here : Note<
"explicit %select{constructor|deduction guide}0 declared here">;
+def note_implicit_deduction_guide : Note<"implicit deduction guide declared as '%0'">;
// C++11 auto
def warn_cxx98_compat_auto_type_specifier : Warning<
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index fb4ff72e42eb5..31b908eb3cc3c 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -40,6 +40,7 @@
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLForwardCompat.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
@@ -12114,6 +12115,35 @@ static void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand,
return;
}
+ // If this is an implicit deduction guide against a non-user-defined
+ // constructor, add a note for it. These deduction guides nor their
+ // corresponding constructors are not explicitly spelled in the source code,
+ // and simply producing a deduction failure note around the heading of the
+ // enclosing RecordDecl is confusing.
+ //
+ // We prefer adding such notes at the end of the last deduction failure
+ // reason because duplicate code snippets appearing in the diagnostic
+ // are likely becoming noisy.
+ auto _ = llvm::make_scope_exit([&] {
+ auto *DG = dyn_cast<CXXDeductionGuideDecl>(Fn);
+ if (!DG || !DG->isImplicit() || DG->getCorrespondingConstructor())
+ return;
+ std::string FunctionProto;
+ llvm::raw_string_ostream OS(FunctionProto);
+ FunctionTemplateDecl *Template = DG->getDescribedFunctionTemplate();
+ // This also could be an instantiation. Find out the primary template.
+ if (!Template) {
+ FunctionDecl *Pattern = DG->getTemplateInstantiationPattern();
+ assert(Pattern && Pattern->getDescribedFunctionTemplate() &&
+ "Cannot find the associated function template of "
+ "CXXDeductionGuideDecl?");
+ Template = Pattern->getDescribedFunctionTemplate();
+ }
+ Template->print(OS);
+ S.Diag(DG->getLocation(), diag::note_implicit_deduction_guide)
+ << FunctionProto;
+ });
+
switch (Cand->FailureKind) {
case ovl_fail_too_many_arguments:
case ovl_fail_too_few_arguments:
diff --git a/clang/test/CXX/drs/cwg26xx.cpp b/clang/test/CXX/drs/cwg26xx.cpp
index 2b17c8101438d..95dfa229530fb 100644
--- a/clang/test/CXX/drs/cwg26xx.cpp
+++ b/clang/test/CXX/drs/cwg26xx.cpp
@@ -194,8 +194,11 @@ static_assert(__is_same(decltype(i), I<char, 4>));
J j = { "ghi" };
// since-cxx20-error@-1 {{no viable constructor or deduction guide}}
// since-cxx20-note@#cwg2681-J {{candidate template ignored: could not match 'J<N>' against 'const char *'}}
+// since-cxx20-note@#cwg2681-J {{implicit deduction guide declared as 'template <size_t N> J(J<N>) -> J<N>'}}
// since-cxx20-note@#cwg2681-J {{candidate template ignored: could not match 'const unsigned char' against 'const char'}}
+// since-cxx20-note@#cwg2681-J {{implicit deduction guide declared as 'template <size_t N> J(const unsigned char (&)[N]) -> J<N>'}}
// since-cxx20-note@#cwg2681-J {{candidate function template not viable: requires 0 arguments, but 1 was provided}}
+// since-cxx20-note@#cwg2681-J {{implicit deduction guide declared as 'template <size_t N> J() -> J<N>'}}
#endif
}
diff --git a/clang/test/CXX/expr/expr.post/expr.type.conv/p1.cpp b/clang/test/CXX/expr/expr.post/expr.type.conv/p1.cpp
index f3608bc378bc7..aa3c022e9b134 100644
--- a/clang/test/CXX/expr/expr.post/expr.type.conv/p1.cpp
+++ b/clang/test/CXX/expr/expr.post/expr.type.conv/p1.cpp
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -std=c++1z -verify %s
-template<typename T> struct A { // expected-note 2{{candidate}}
+template<typename T> struct A { // expected-note 2{{candidate}} {{implicit deduction guide}}
T t, u;
};
template<typename T> A(T, T) -> A<T>; // expected-note {{deduced conflicting types for parameter 'T'}}
diff --git a/clang/test/CXX/over/over.match/over.match.funcs/over.match.class.deduct/p2.cpp b/clang/test/CXX/over/over.match/over.match.funcs/over.match.class.deduct/p2.cpp
index 49fde292f6a36..4bcf4ee2e3fe7 100644
--- a/clang/test/CXX/over/over.match/over.match.funcs/over.match.class.deduct/p2.cpp
+++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.class.deduct/p2.cpp
@@ -35,7 +35,7 @@ namespace std {
}
namespace p0702r1 {
- template<typename T> struct X { // expected-note {{candidate}}
+ template<typename T> struct X { // expected-note {{candidate}} expected-note {{implicit deduction guide}}
X(std::initializer_list<T>); // expected-note {{candidate template ignored: could not match 'std::initializer_list<T>' against 'Z'}}
};
diff --git a/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp b/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
index 8592626269eed..2e39697a1f07f 100644
--- a/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
+++ b/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
@@ -6,7 +6,7 @@
#if __cplusplus > 201402L
namespace ClassTemplateParamNotForwardingRef {
// This is not a forwarding reference.
- template<typename T> struct A { // expected-note {{candidate}}
+ template<typename T> struct A { // expected-note {{candidate}} expected-note {{implicit deduction guide}}
A(T&&); // expected-note {{expects an rvalue}}
};
int n;
@@ -75,7 +75,7 @@ namespace std_example {
int n3 = g(i); // expected-error{{no matching function for call to 'g'}}
#if __cplusplus > 201402L
- template<class T> struct A { // expected-note {{candidate}}
+ template<class T> struct A { // expected-note {{candidate}} expected-note {{implicit deduction guide}}
template<class U>
A(T &&, U &&, int *); // expected-note {{[with T = int, U = int] not viable: expects an rvalue}}
A(T &&, int *); // expected-note {{requires 2}}
diff --git a/clang/test/Modules/template_name_lookup.cpp b/clang/test/Modules/template_name_lookup.cpp
index 29375e514025f..82b06e83afce3 100644
--- a/clang/test/Modules/template_name_lookup.cpp
+++ b/clang/test/Modules/template_name_lookup.cpp
@@ -7,5 +7,7 @@ import foo;
void use() {
X x; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of 'X'}}
// expected-note@Inputs/template_name_lookup/foo.cppm:3 {{candidate template ignored: couldn't infer template argument 'T'}}
+ // expected-note@Inputs/template_name_lookup/foo.cppm:3 {{implicit deduction guide declared as 'template <typename T> X(X<T>) -> X<T>'}}
// expected-note@Inputs/template_name_lookup/foo.cppm:3 {{candidate function template not viable: requires 1 argument, but 0 were provided}}
+ // expected-note@Inputs/template_name_lookup/foo.cppm:3 {{implicit deduction guide declared as 'template <typename T> X() -> X<T>'}}
}
diff --git a/clang/test/PCH/cxx-explicit-specifier.cpp b/clang/test/PCH/cxx-explicit-specifier.cpp
index 94ca9f7b080db..68dfc911164c5 100644
--- a/clang/test/PCH/cxx-explicit-specifier.cpp
+++ b/clang/test/PCH/cxx-explicit-specifier.cpp
@@ -79,7 +79,7 @@ struct A {
B<true> b_true;
B<false> b_false;
#else
-//expected-note@-8 {{candidate template ignored}}
+//expected-note@-8 {{candidate template ignored}} expected-note@-8 {{implicit deduction guide declared as 'template <bool b> A(A<b>) -> A<b>'}}
//expected-note@-8 {{explicit constructor declared here}}
//expected-note@-15+ {{candidate constructor}}
//expected-note@-8+ {{explicit conversion function is not a candidate (explicit specifier}}
diff --git a/clang/test/Sema/tls_alignment.cpp b/clang/test/Sema/tls_alignment.cpp
index c5c79aafa9ead..65b6b035e7917 100644
--- a/clang/test/Sema/tls_alignment.cpp
+++ b/clang/test/Sema/tls_alignment.cpp
@@ -22,7 +22,9 @@ struct struct_with_aligned_field {
template <typename>
struct templated_struct {};
// expected-note@-1{{candidate template ignored: couldn't infer template argument ''}}
-// expected-note@-2{{candidate function template not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-2{{implicit deduction guide declared as 'template <typename> templated_struct() -> templated_struct<type-parameter-0-0>'}}
+// expected-note@-3{{candidate function template not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-4{{implicit deduction guide declared as 'template <typename> templated_struct(templated_struct<type-parameter-0-0>) -> templated_struct<type-parameter-0-0>'}}
// A typedef of the aligned struct.
typedef aligned_struct another_aligned_struct;
diff --git a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
index 90404f115c75f..0a296b32cfb6f 100644
--- a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -139,7 +139,8 @@ namespace look_into_current_instantiation {
// templates, and members of the current instantiation
A<float> &r = a;
- template<typename T> struct B { // expected-note {{could not match 'B<T>' against 'int'}}
+ template<typename T> struct B { // expected-note {{could not match 'B<T>' against 'int'}} \
+ expected-note {{implicit deduction guide declared as 'template <typename T> B(B<T>) -> B<T>'}}
struct X {
typedef T type;
};
@@ -564,7 +565,8 @@ namespace PR47175 {
// Ensure we don't crash when CTAD fails.
template <typename T1, typename T2>
-struct Foo { // expected-note{{candidate function template not viable}}
+struct Foo { // expected-note{{candidate function template not viable}} \
+ expected-note{{implicit deduction guide declared as 'template <typename T1, typename T2> Foo(Foo<T1, T2>) -> Foo<T1, T2>'}}
Foo(T1, T2); // expected-note{{candidate function template not viable}}
};
diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
index b71dfc6ccaf4f..901f64434bbee 100644
--- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
+++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
@@ -108,8 +108,11 @@ struct Foo {
Foo(T const (&)[N]);
};
+// FIXME: Clarify that __is_deducible constraints are unsatisfied. For example, GCC currently prints the code snippet around constructors
+// FIXME: Prefer non-canonical template arguments in the deduction guide?
template <typename X, int Y>
using Bar = Foo<X, sizeof(X)>; // expected-note {{candidate template ignored: couldn't infer template argument 'X'}} \
+ // expected-note {{implicit deduction guide declared as 'template <typename X> Bar(Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>) -> Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>'}} \
// expected-note {{candidate template ignored: constraints not satisfied [with X = int]}} \
// expected-note {{cannot deduce template arguments for 'Bar' from 'Foo<int, 4UL>'}}
@@ -137,9 +140,12 @@ struct A {};
template<class T> struct Foo { T c; };
template<class X, class Y=A>
using AFoo = Foo<Y>; // expected-note {{candidate template ignored: could not match 'Foo<type-parameter-0-0>' against 'int'}} \
+ // expected-note {{implicit deduction guide declared as 'template <class Y = A> AFoo(Foo<type-parameter-0-0>) -> Foo<type-parameter-0-0>'}} \
// expected-note {{candidate template ignored: constraints not satisfied [with Y = int]}} \
// expected-note {{cannot deduce template arguments for 'AFoo' from 'Foo<int>'}} \
- // expected-note {{candidate function template not viable: requires 0 arguments, but 1 was provided}}
+ // expected-note {{implicit deduction guide declared as 'template <class Y = A> AFoo(type-parameter-0-0) -> Foo<type-parameter-0-0>'}} \
+ // expected-note {{candidate function template not viable: requires 0 arguments, but 1 was provided}} \
+ // expected-note {{implicit deduction guide declared as 'template <class Y = A> AFoo() -> Foo<type-parameter-0-0>'}}
AFoo s = {1}; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of 'AFoo'}}
} // namespace test11
@@ -192,6 +198,7 @@ struct Foo {
template <int K>
using Bar = Foo<double, K>; // expected-note {{constraints not satisfied for class template 'Foo'}}
// expected-note@-1 {{candidate template ignored: could not match}}
+// expected-note@-2 {{implicit deduction guide declared as 'template <int K> Bar(Foo<double, K>) -> Foo<double, K>'}}
double abc[3];
Bar s2 = {abc}; // expected-error {{no viable constructor or deduction guide for deduction }}
} // namespace test14
@@ -204,6 +211,7 @@ template<typename> concept False = false;
template<False W>
using BFoo = AFoo<W>; // expected-note {{candidate template ignored: constraints not satisfied [with V = int]}} \
// expected-note {{cannot deduce template arguments for 'BFoo' from 'Foo<int *>'}} \
+ // expected-note {{implicit deduction guide declared as 'template <class V> BFoo(Foo<type-parameter-0-0 *>) -> Foo<type-parameter-0-0 *>'}} \
// expected-note {{candidate template ignored: could not match 'Foo<type-parameter-0-0 *>' against 'int *'}}
int i = 0;
AFoo a1(&i); // OK, deduce Foo<int *>
@@ -256,7 +264,9 @@ Foo(T) -> Foo<int>;
template <typename U>
using Bar = Foo<U>; // expected-note {{could not match 'Foo<type-parameter-0-0>' against 'int'}} \
// expected-note {{candidate template ignored: constraints not satisfied}} \
- // expected-note {{candidate function template not viable}}
+ // expected-note {{candidate function template not viable}} \
+ // expected-note {{implicit deduction guide declared as 'template <typename U> Bar() -> Foo<type-parameter-0-0>'}} \
+ // expected-note {{implicit deduction guide declared as 'template <typename U> Bar(Foo<type-parameter-0-0>) -> Foo<type-parameter-0-0>'}}
Bar s = {1}; // expected-error {{no viable constructor or deduction guide for deduction of template arguments}}
} // namespace test18
@@ -284,7 +294,8 @@ class Foo {};
// Verify that template template type parameter TTP is referenced/used in the
// template arguments of the RHS.
template <template<typename> typename TTP>
-using Bar = Foo<K<TTP>>; // expected-note {{candidate template ignored: could not match 'Foo<K<template-parameter-0-0>>' against 'int'}}
+using Bar = Foo<K<TTP>>; // expected-note {{candidate template ignored: could not match 'Foo<K<template-parameter-0-0>>' against 'int'}} \
+ // expected-note {{implicit deduction guide declared as 'template <template <typename> typename TTP> Bar(Foo<K<template-parameter-0-0>>) -> Foo<K<template-parameter-0-0>>'}}
template <class T>
class Container {};
diff --git a/clang/test/SemaCXX/cxx2a-explicit-bool.cpp b/clang/test/SemaCXX/cxx2a-explicit-bool.cpp
index 9fdc059493aac..13b5ff1d59b31 100644
--- a/clang/test/SemaCXX/cxx2a-explicit-bool.cpp
+++ b/clang/test/SemaCXX/cxx2a-explicit-bool.cpp
@@ -394,6 +394,7 @@ using type = T;
template<typename T1, typename T2, bool b>
struct A {
// expected-note@-1+ {{candidate function}}
+ // expected-note@-2+ {{implicit deduction guide}}
explicit(false)
A(typename nondeduced<T1>::type, typename nondeduced<T2>::type, typename nondeduced<B<b>>::type) {}
// expected-note@-1+ {{candidate template ignored}}
@@ -678,6 +679,7 @@ namespace deduction_guide2 {
template<typename T1 = int, typename T2 = int>
struct A {
// expected-note@-1+ {{candidate template ignored}}
+ // expected-note@-2+ {{implicit deduction guide}}
explicit(!is_same<T1, T2>::value)
A(T1 = 0, T2 = 0) {}
// expected-note@-1 {{explicit constructor declared here}}
diff --git a/clang/test/SemaCXX/gh65522.cpp b/clang/test/SemaCXX/gh65522.cpp
index 2d6331b0372a3..bfc3a1e12fe76 100644
--- a/clang/test/SemaCXX/gh65522.cpp
+++ b/clang/test/SemaCXX/gh65522.cpp
@@ -3,7 +3,8 @@
class X {};
template<typename T>
-class B3 { // expected-note {{candidate template ignored: could not match 'B3<T>' against 'int'}}
+class B3 { // expected-note {{candidate template ignored: could not match 'B3<T>' against 'int'}} \
+ // expected-note {{implicit deduction guide declared as 'template <typename T> B3(B3<T>) -> B3<T>'}}
template<X x> B3(T); // expected-warning 2{{non-type template parameter of type 'X' is incompatible with C++ standards before C++20}} \
// expected-note {{candidate template ignored: couldn't infer template argument 'x'}}
};
diff --git a/clang/test/SemaCXX/invalid-deduction-guide-as-template-candidates.cpp b/clang/test/SemaCXX/invalid-deduction-guide-as-template-candidates.cpp
index e9b362d67cd23..ad7441ff48477 100644
--- a/clang/test/SemaCXX/invalid-deduction-guide-as-template-candidates.cpp
+++ b/clang/test/SemaCXX/invalid-deduction-guide-as-template-candidates.cpp
@@ -1,6 +1,8 @@
// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
template <class T> class Foo {}; // expected-note {{candidate template ignored: couldn't infer template argument 'T'}} \
- // expected-note {{candidate function template not viable: requires 1 argument, but 0 were provided}}
+ // expected-note {{implicit deduction guide declared as 'template <class T> Foo(Foo<T>) -> Foo<T>'}} \
+ // expected-note {{candidate function template not viable: requires 1 argument, but 0 were provided}} \
+ // expected-note {{implicit deduction guide declared as 'template <class T> Foo() -> Foo<T>'}}
Foo(); // expected-error {{deduction guide declaration without trailing return type}}
Foo vs; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of 'Foo'}}
diff --git a/clang/test/SemaCXX/lookup-template-name-extern-CXX.cpp b/clang/test/SemaCXX/lookup-template-name-extern-CXX.cpp
index 93e7fb6199354..f851af7ecec26 100644
--- a/clang/test/SemaCXX/lookup-template-name-extern-CXX.cpp
+++ b/clang/test/SemaCXX/lookup-template-name-extern-CXX.cpp
@@ -3,8 +3,10 @@
// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
extern "C++" {
template <class T>
-class X {}; // expected-note {{candidate template ignored: couldn't infer template argument 'T'}}
- // expected-note@-1 {{candidate function template not viable: requires 1 argument, but 0 were provided}}
+class X {}; // expected-note {{candidate template ignored: couldn't infer template argument 'T'}} \
+ // expected-note {{implicit deduction guide declared as 'template <class T> X(X<T>) -> X<T>'}} \
+ // expected-note {{candidate function template not viable: requires 1 argument, but 0 were provided}} \
+ // expected-note {{implicit deduction guide declared as 'template <class T> X() -> X<T>'}}
}
void foo() {
diff --git a/clang/test/SemaTemplate/aggregate-deduction-candidate.cpp b/clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
index 5a5033...
[truncated]
|
|
(A screenshot that helps to understand the changes here) (compared to what we have now and what GCC gives: https://godbolt.org/z/948M7x7fE) |
zyn0217
changed the title
|
I will say, one worry that I do have is that this would end up issuing... a lot of notes for a single error:
C++ error messages already have a reputation of being rather long; I’m candidly doubtful as to whether this would help most users too much... Perhaps an approach more similar to what we display for ambiguous cast paths would be better (https://godbolt.org/z/zT1rW7zcv):
i.e. list each implicit deduction guide we tried in a single note; I personally at least think that using indentation and formatting a bit more rather than emitting a separate note for everything we might want to add to an error might go a long way in aiding legibility, but that’s just my opinion ;Þ |
I think this is more about making us more accurate when we want to point out there's something wrong with users' code. Admittedly, it's hard to tell whether most users would gain much from the diagnostic notes, and I could also bet that in most cases, people tend to evade a flood of errors and their accompanying notes. It is usually the case that a bunch of notes might bury the most helpful error message indicating where the first error happens; however, I don't think this would exacerbate it much because this 1) only notes implicitly generated CTAD guides; 2) won't be chatty if the guide is generated over a user-defined constructor: we have already pointed to a feasible source location that makes things clear.
Thanks for suggesting a better approach. However, doing so would require big surgery in our diagnostic building logic for overloads. We don't explain the reason for deduction failure in an error; instead, we do so in the following notes. To be clear, we now have ~16 deduction failure kinds, each with its own diagnostic message. Given that, I don't think it's necessary (and probably undoable) to special case the CTAD guides for each note... It's also sad that we're unable to pretty-print the deduction guides in a diagnostic. If I read it correctly, we currently only support printing either1) a verbatim code snippet or 2) a FIXIT suggestion. And neither does work for this case, though. |
Hmm, yeah, I guess it’s not that bad so long as we don’t end up w/ e.g. 20 extra notes because of this.
Yeah, I’ve been wanting to try and take a look at some of that, but other things have been getting in the way unfortunately... |
There was a problem hiding this comment.
Two nits, but the rest seems reasonable. I agree that make_scope_exit is probably the only sane way of doing this here.
While I still maintain that the formatting of this could be better, I also agree that this is a problem that we’ll probably have to solve separately.
|
Thanks @Sirraide for the review! I'd like to wait a couple of days before landing given that some folks are out for a wg21 meeting in these two weeks - in case they have other opinions. |
Sure, but afaik you’ll have to wait until July most likely if you want to give other people time to look at this (unless Aaron or someone else who is not at the meeting can find the time to take a look at this). |
Co-authored-by: Sirraide <[email protected]>
|
Thanks for the patch! +1 on the idea of printing deduction guides in diagnostics. This improves the experience for both users and compiler developers. This is #92393 :)
In my opinion, including the deduction guide in the existing note (e.g., However, I don't feel strongly about this, and am also fine with the current implementation, as it has already improved the experience. |
clang/lib/Sema/SemaOverload.cpp
Outdated
| @@ -12114,6 +12115,35 @@ static void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand, | |||
| return; | |||
| } | |||
|
|
|||
| // If this is an implicit deduction guide against an implicitly defined | |||
| // constructor, add a note for it. Neither these deduction guides nor their | |||
| // corresponding constructors are explicitly spelled in the source code, | |||
There was a problem hiding this comment.
Any reason to filter out the "explicit constructor" case? We still synthesize a deduction guide from a constructor, and they are not identical, e.g. its template parameters is a combination of template parameters of the class and template parameters of the corresponding constructor. I think it is useful to print them as well.
I think a simple model here would be that we always print synthesized deduction guides (this covers the using-alias case.)
There was a problem hiding this comment.
Yeah, we would have a deduction guide with combined template parameters; however, my concern is that we already have sufficient code contexts in the snippets of a diagnostic for users to understand the error. For example,
template <class T> struct Outer {
template <class U> struct Inner {
template <class V>
Inner(U, V, V) {}
};
};
Outer<int>::Inner i(42, "hello");https://clang.godbolt.org/z/1bEfqrrMv
we currently have a note followed by a code snippet:
<source>:4:2: note: candidate function template not viable: requires 3 arguments, but 2 were provided
4 | Inner(U, V, V) {}
| ^ ~~~~~~~
although we don't have the synthesized template parameters presented, I think (subjectively) this is probably already legible.
(this covers the using-alias case.)
With the filter currently in place, I have seen a bunch of additional notes following the using-alias cases. However, I'm not sure I have handled them all.
There was a problem hiding this comment.
But I'm not opposed to removing the filter and emitting all the candidates like what GCC does, anyway.
There was a problem hiding this comment.
I personally would probably prefer not printing them if there already is enough context, but I don’t feel too strongly either way.
There was a problem hiding this comment.
I have a different perspective: I'd prefer printing them as long as they are synthesized (not part of the written source code).
While the example of combined template parameters might not be the best one (and I agree that understanding them from existing contexts is often sufficient), the conjunction of associated constraints for a class and the corresponding constructor is probably less obvious. I think printing them would make the situation clearer and help users avoid guessing. Moreover, this approach would be consistent with what GCC does.
There was a problem hiding this comment.
Hmm, yeah, if there are situations where that might help, then it might make sense to do that.
| template <typename X, int Y> | ||
| using Bar = Foo<X, sizeof(X)>; // expected-note {{candidate template ignored: couldn't infer template argument 'X'}} \ | ||
| // expected-note {{implicit deduction guide declared as 'template <typename X> Bar(Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>) -> Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>'}} \ | ||
| // expected-note {{implicit deduction guide declared as 'template <typename X> Bar(const type-parameter-0-0 (&)[sizeof(type-parameter-0-0)]) -> Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>'}} \ | ||
| // expected-note {{candidate template ignored: constraints not satisfied [with X = int]}} \ |
There was a problem hiding this comment.
Right, it is not great for constraints not written in the source code.
If we print constraints (see my another comment), it will improve the situation.
| template <typename X, int Y> | ||
| using Bar = Foo<X, sizeof(X)>; // expected-note {{candidate template ignored: couldn't infer template argument 'X'}} \ | ||
| // expected-note {{implicit deduction guide declared as 'template <typename X> Bar(Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>) -> Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>'}} \ | ||
| // expected-note {{implicit deduction guide declared as 'template <typename X> Bar(const type-parameter-0-0 (&)[sizeof(type-parameter-0-0)]) -> Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>'}} \ | ||
| // expected-note {{candidate template ignored: constraints not satisfied [with X = int]}} \ |
There was a problem hiding this comment.
Right, it is not great for constraints not written in the source code.
If we print constraints (see my another comment), it will improve the situation.
| template <typename X, int Y> | ||
| using Bar = Foo<X, sizeof(X)>; // expected-note {{candidate template ignored: couldn't infer template argument 'X'}} \ | ||
| // expected-note {{implicit deduction guide declared as 'template <typename X> Bar(Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>) -> Foo<type-parameter-0-0, sizeof(type-parameter-0-0)>'}} \ |
There was a problem hiding this comment.
I think it would be useful to print the associated constraints.
I thought the attached constraints will be print automatically in the FunctionTemplateDecl::print(it is not implemented in the pretty printer yet?). No need to address here, but please add a FIXME.
There was a problem hiding this comment.
I think it's because we attach the __is_deducible constraint to the TemplateParameterList rather than the FunctionDecl corresponding to the synthesized deduction guide. And we don't have a handling in DeclPrinter that prints such trailing expressions for template parameters...
I could try adding such logic to DeclPrinter and see if it doesn't break anything seriously. Otherwise, we probably need to tweak our BuildDeductionGuideForTypeAlias so that we attach the constraints to the function decl. (I assume there aren't many differences between the two, i.e. attaching it to the function Decl or to the template parameter list?)
There was a problem hiding this comment.
Cool, check-clang runs successfully :-) Let me file a separate PR then.
…template parameters As discussed in llvm#96084 (comment), it would be nice to present these trailing constraints on template parameters when printing CTAD decls through a DeclPrinter.
…template parameters (#96864) As discussed in #96084 (comment), it would be nice to present these trailing constraints on template parameters when printing CTAD decls through a DeclPrinter.
|
Thank you folks for the review & suggestion. I plan to merge this PR next week in case @AaronBallman or @cor3ntin has objections. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/1796 Here is the relevant piece of the build log for the reference: |
…template parameters (llvm#96864) As discussed in llvm#96084 (comment), it would be nice to present these trailing constraints on template parameters when printing CTAD decls through a DeclPrinter.
…uides (llvm#96084) Given the following invalid code, ```cpp template <class T> struct S { T *a; }; S s = {1}; ``` we produce such diagnostics currently: ``` <source>:2:8: note: candidate template ignored: could not match 'S<T>' against 'int' 2 | struct S { | ^ <source>:2:8: note: candidate template ignored: could not match 'T *' against 'int' ``` Which I think is confusing because there's no `S<T>` nor `T *` at the location it points to. This is because we're deducing the initializer against implicitly generated deduction guides, and their source locations just point to the corresponding `RecordDecl`. Hence the misleading notes. This patch alleviates the issue by adding extra notes demonstrating which implicit deduction guide we're deducing against. In other words, in addition to the note of `could not match 'T *' against 'int'`, we would also say the implicit deduction guide we're trying to use: `template <class T> S(T *) -> S<T>`, which looks clearer IMO. --------- Co-authored-by: Sirraide <[email protected]>
…uides (llvm#96084) Given the following invalid code, ```cpp template <class T> struct S { T *a; }; S s = {1}; ``` we produce such diagnostics currently: ``` <source>:2:8: note: candidate template ignored: could not match 'S<T>' against 'int' 2 | struct S { | ^ <source>:2:8: note: candidate template ignored: could not match 'T *' against 'int' ``` Which I think is confusing because there's no `S<T>` nor `T *` at the location it points to. This is because we're deducing the initializer against implicitly generated deduction guides, and their source locations just point to the corresponding `RecordDecl`. Hence the misleading notes. This patch alleviates the issue by adding extra notes demonstrating which implicit deduction guide we're deducing against. In other words, in addition to the note of `could not match 'T *' against 'int'`, we would also say the implicit deduction guide we're trying to use: `template <class T> S(T *) -> S<T>`, which looks clearer IMO. --------- Co-authored-by: Sirraide <[email protected]>
…template parameters (llvm#96864) As discussed in llvm#96084 (comment), it would be nice to present these trailing constraints on template parameters when printing CTAD decls through a DeclPrinter.
Given the following invalid code,
we produce such diagnostics currently:
Which I think is confusing because there's no
S<T>norT *at the location it points to. This is because we're deducing the initializer against implicitly generated deduction guides, and their source locations just point to the correspondingRecordDecl. And hence the misleading notes.This patch alleviates the issue by adding extra notes demonstrating which implicit deduction guide we're deducing against. In other words, in addition to the note of
could not match 'T *' against 'int', we would also say the implicit deduction guide we're trying to use:template <class T> S(T *) -> S<T>, which looks clearer IMO.