* [PATCH] c++: tweaks for explicit conversion fns diagnostic
@ 2023-08-25 23:37 Marek Polacek
2023-08-26 0:34 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Marek Polacek @ 2023-08-25 23:37 UTC (permalink / raw)
To: GCC Patches, Jason Merrill
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
1) When saying that a conversion is erroneous because it would use
an explicit constructor, it might be nice to show where exactly
the explicit constructor is located. For example, with this patch:
[...]
explicit.C:4:12: note: 'S::S(int)' declared here
4 | explicit S(int) { }
| ^
2) When a conversion doesn't work out merely because the conversion
function necessary to do the conversion couldn't be used because
it was marked explicit, it would be useful to the user to say so,
rather than just saying "cannot convert". For example, with this patch:
explicit.C:13:12: error: cannot convert 'S' to 'bool' in initialization
13 | bool b = S{1};
| ^~~~
| |
| S
explicit.C:5:12: note: explicit conversion function was not considered
5 | explicit operator bool() const { return true; }
| ^~~~~~~~
gcc/cp/ChangeLog:
* call.cc (convert_like_internal): Show where the conversion function
was declared.
(maybe_show_nonconverting_candidate): New.
* cp-tree.h (maybe_show_nonconverting_candidate): Declare.
* typeck.cc (convert_for_assignment): Call it.
gcc/testsuite/ChangeLog:
* g++.dg/diagnostic/explicit.C: New test.
---
gcc/cp/call.cc | 41 +++++++++++++++++++---
gcc/cp/cp-tree.h | 1 +
gcc/cp/typeck.cc | 5 +++
gcc/testsuite/g++.dg/diagnostic/explicit.C | 16 +++++++++
4 files changed, 59 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/explicit.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 23e458d3252..09ebcf6a115 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -8459,12 +8459,21 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
if (pedwarn (loc, 0, "converting to %qT from initializer list "
"would use explicit constructor %qD",
totype, convfn))
- inform (loc, "in C++11 and above a default constructor "
- "can be explicit");
+ {
+ inform (loc, "in C++11 and above a default constructor "
+ "can be explicit");
+ inform (DECL_SOURCE_LOCATION (convfn), "%qD declared here",
+ convfn);
+ }
}
else
- error ("converting to %qT from initializer list would use "
- "explicit constructor %qD", totype, convfn);
+ {
+ auto_diagnostic_group d;
+ error ("converting to %qT from initializer list would use "
+ "explicit constructor %qD", totype, convfn);
+ inform (DECL_SOURCE_LOCATION (convfn), "%qD declared here",
+ convfn);
+ }
}
/* If we're initializing from {}, it's value-initialization. */
@@ -14323,4 +14332,28 @@ is_list_ctor (tree decl)
return true;
}
+/* We know that can_convert_arg_bad already said "no" when trying to convert
+ FROM to TO with ARG and FLAGS. Try to figure out if it was because
+ an explicit conversion function was skipped when looking for a way to
+ perform the conversion. At this point we've already printed an error. */
+
+void
+maybe_show_nonconverting_candidate (tree to, tree from, tree arg, int flags)
+{
+ if (!(flags & LOOKUP_ONLYCONVERTING))
+ return;
+
+ conversion_obstack_sentinel cos;
+ conversion *c = implicit_conversion (to, from, arg, /*c_cast_p=*/false,
+ flags & ~LOOKUP_ONLYCONVERTING, tf_none);
+ if (c && !c->bad_p && c->user_conv_p)
+ /* Ay, the conversion would have worked in copy-init context. */
+ for (; c; c = next_conversion (c))
+ if (c->kind == ck_user
+ && DECL_P (c->cand->fn)
+ && DECL_NONCONVERTING_P (c->cand->fn))
+ inform (DECL_SOURCE_LOCATION (c->cand->fn), "explicit conversion "
+ "function was not considered");
+}
+
#include "gt-cp-call.h"
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 608d6310e53..6b225ca182f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6727,6 +6727,7 @@ extern bool cp_handle_deprecated_or_unavailable (tree, tsubst_flags_t = tf_warni
extern void cp_warn_deprecated_use_scopes (tree);
extern tree get_function_version_dispatcher (tree);
extern bool any_template_arguments_need_structural_equality_p (tree);
+extern void maybe_show_nonconverting_candidate (tree, tree, tree, int);
/* in class.cc */
extern tree build_vfield_ref (tree, tree);
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index d5c0c85ed51..bf9963db3ba 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10262,6 +10262,8 @@ convert_for_assignment (tree type, tree rhs,
{
range_label_for_type_mismatch label (rhstype, type);
gcc_rich_location richloc (rhs_loc, has_loc ? &label : NULL);
+ auto_diagnostic_group d;
+
switch (errtype)
{
case ICR_DEFAULT_ARGUMENT:
@@ -10295,6 +10297,9 @@ convert_for_assignment (tree type, tree rhs,
default:
gcc_unreachable();
}
+
+ /* See if we can be more helpful. */
+ maybe_show_nonconverting_candidate (type, rhstype, rhs, flags);
}
if (TYPE_PTR_P (rhstype)
&& TYPE_PTR_P (type)
diff --git a/gcc/testsuite/g++.dg/diagnostic/explicit.C b/gcc/testsuite/g++.dg/diagnostic/explicit.C
new file mode 100644
index 00000000000..fde582b8e1c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/explicit.C
@@ -0,0 +1,16 @@
+// { dg-do compile { target c++11 } }
+
+struct S {
+ explicit S(int) { }
+ explicit operator bool() const { return true; } // { dg-message "explicit conversion function was not considered" }
+ explicit operator int() const { return 42; } // { dg-message "explicit conversion function was not considered" }
+};
+
+void
+g ()
+{
+ S s = {1}; // { dg-error "would use explicit constructor" }
+ bool b = S{1}; // { dg-error "cannot convert .S. to .bool. in initialization" }
+ int i;
+ i = S{2}; // { dg-error "cannot convert .S. to .int. in assignment" }
+}
base-commit: e1f096a3cc96c71907cfbc7b8baf67a3d863cb6d
--
2.41.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: tweaks for explicit conversion fns diagnostic
2023-08-25 23:37 [PATCH] c++: tweaks for explicit conversion fns diagnostic Marek Polacek
@ 2023-08-26 0:34 ` Jason Merrill
2023-08-28 23:24 ` [PATCH v2] " Marek Polacek
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2023-08-26 0:34 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 8/25/23 19:37, Marek Polacek wrote:
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >8 --
>
> 1) When saying that a conversion is erroneous because it would use
> an explicit constructor, it might be nice to show where exactly
> the explicit constructor is located. For example, with this patch:
>
> [...]
> explicit.C:4:12: note: 'S::S(int)' declared here
> 4 | explicit S(int) { }
> | ^
>
> 2) When a conversion doesn't work out merely because the conversion
> function necessary to do the conversion couldn't be used because
> it was marked explicit, it would be useful to the user to say so,
> rather than just saying "cannot convert". For example, with this patch:
>
> explicit.C:13:12: error: cannot convert 'S' to 'bool' in initialization
> 13 | bool b = S{1};
> | ^~~~
> | |
> | S
> explicit.C:5:12: note: explicit conversion function was not considered
> 5 | explicit operator bool() const { return true; }
> | ^~~~~~~~
>
> gcc/cp/ChangeLog:
>
> * call.cc (convert_like_internal): Show where the conversion function
> was declared.
> (maybe_show_nonconverting_candidate): New.
> * cp-tree.h (maybe_show_nonconverting_candidate): Declare.
> * typeck.cc (convert_for_assignment): Call it.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/diagnostic/explicit.C: New test.
> ---
> gcc/cp/call.cc | 41 +++++++++++++++++++---
> gcc/cp/cp-tree.h | 1 +
> gcc/cp/typeck.cc | 5 +++
> gcc/testsuite/g++.dg/diagnostic/explicit.C | 16 +++++++++
> 4 files changed, 59 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/diagnostic/explicit.C
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 23e458d3252..09ebcf6a115 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -8459,12 +8459,21 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
> if (pedwarn (loc, 0, "converting to %qT from initializer list "
> "would use explicit constructor %qD",
> totype, convfn))
> - inform (loc, "in C++11 and above a default constructor "
> - "can be explicit");
> + {
> + inform (loc, "in C++11 and above a default constructor "
> + "can be explicit");
> + inform (DECL_SOURCE_LOCATION (convfn), "%qD declared here",
> + convfn);
I'd swap these two informs.
> +++ b/gcc/testsuite/g++.dg/diagnostic/explicit.C
> @@ -0,0 +1,16 @@
> +// { dg-do compile { target c++11 } }
> +
> +struct S {
> + explicit S(int) { }
> + explicit operator bool() const { return true; } // { dg-message "explicit conversion function was not considered" }
> + explicit operator int() const { return 42; } // { dg-message "explicit conversion function was not considered" }
> +};
> +
> +void
> +g ()
> +{
> + S s = {1}; // { dg-error "would use explicit constructor" }
> + bool b = S{1}; // { dg-error "cannot convert .S. to .bool. in initialization" }
> + int i;
> + i = S{2}; // { dg-error "cannot convert .S. to .int. in assignment" }
> +}
Let's also test other copy-initialization contexts: parameter passing,
return, throw, aggregate member initialization.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] c++: tweaks for explicit conversion fns diagnostic
2023-08-26 0:34 ` Jason Merrill
@ 2023-08-28 23:24 ` Marek Polacek
2023-08-29 20:42 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Marek Polacek @ 2023-08-28 23:24 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Fri, Aug 25, 2023 at 08:34:37PM -0400, Jason Merrill wrote:
> On 8/25/23 19:37, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > -- >8 --
> >
> > 1) When saying that a conversion is erroneous because it would use
> > an explicit constructor, it might be nice to show where exactly
> > the explicit constructor is located. For example, with this patch:
> >
> > [...]
> > explicit.C:4:12: note: 'S::S(int)' declared here
> > 4 | explicit S(int) { }
> > | ^
> >
> > 2) When a conversion doesn't work out merely because the conversion
> > function necessary to do the conversion couldn't be used because
> > it was marked explicit, it would be useful to the user to say so,
> > rather than just saying "cannot convert". For example, with this patch:
> >
> > explicit.C:13:12: error: cannot convert 'S' to 'bool' in initialization
> > 13 | bool b = S{1};
> > | ^~~~
> > | |
> > | S
> > explicit.C:5:12: note: explicit conversion function was not considered
> > 5 | explicit operator bool() const { return true; }
> > | ^~~~~~~~
> >
> > gcc/cp/ChangeLog:
> >
> > * call.cc (convert_like_internal): Show where the conversion function
> > was declared.
> > (maybe_show_nonconverting_candidate): New.
> > * cp-tree.h (maybe_show_nonconverting_candidate): Declare.
> > * typeck.cc (convert_for_assignment): Call it.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/diagnostic/explicit.C: New test.
> > ---
> > gcc/cp/call.cc | 41 +++++++++++++++++++---
> > gcc/cp/cp-tree.h | 1 +
> > gcc/cp/typeck.cc | 5 +++
> > gcc/testsuite/g++.dg/diagnostic/explicit.C | 16 +++++++++
> > 4 files changed, 59 insertions(+), 4 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/diagnostic/explicit.C
> >
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index 23e458d3252..09ebcf6a115 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -8459,12 +8459,21 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
> > if (pedwarn (loc, 0, "converting to %qT from initializer list "
> > "would use explicit constructor %qD",
> > totype, convfn))
> > - inform (loc, "in C++11 and above a default constructor "
> > - "can be explicit");
> > + {
> > + inform (loc, "in C++11 and above a default constructor "
> > + "can be explicit");
> > + inform (DECL_SOURCE_LOCATION (convfn), "%qD declared here",
> > + convfn);
>
> I'd swap these two informs.
Done.
> > +++ b/gcc/testsuite/g++.dg/diagnostic/explicit.C
> > @@ -0,0 +1,16 @@
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct S {
> > + explicit S(int) { }
> > + explicit operator bool() const { return true; } // { dg-message "explicit conversion function was not considered" }
> > + explicit operator int() const { return 42; } // { dg-message "explicit conversion function was not considered" }
> > +};
> > +
> > +void
> > +g ()
> > +{
> > + S s = {1}; // { dg-error "would use explicit constructor" }
> > + bool b = S{1}; // { dg-error "cannot convert .S. to .bool. in initialization" }
> > + int i;
> > + i = S{2}; // { dg-error "cannot convert .S. to .int. in assignment" }
> > +}
>
> Let's also test other copy-initialization contexts: parameter passing,
> return, throw, aggregate member initialization.
Done except for throw. To handle arg passing I moved the call to
maybe_show_nonconverting_candidate one line down. I guess a testcase
for throw would be
struct T {
T() { } // #1
explicit T(const T&) { } // #2
};
void
g ()
{
T t{};
throw t;
}
but #2 isn't a viable candidate so this would take more effort to handle.
We just say about #1 that "candidate expects 0 arguments, 1 provided".
clang++ says
e.C:3:12: note: explicit constructor is not a candidate
3 | explicit T(const T&) { }
| ^
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
1) When saying that a conversion is erroneous because it would use
an explicit constructor, it might be nice to show where exactly
the explicit constructor is located. For example, with this patch:
[...]
explicit.C:4:12: note: 'S::S(int)' declared here
4 | explicit S(int) { }
| ^
2) When a conversion doesn't work out merely because the conversion
function necessary to do the conversion couldn't be used because
it was marked explicit, it would be useful to the user to say so,
rather than just saying "cannot convert". For example, with this patch:
explicit.C:13:12: error: cannot convert 'S' to 'bool' in initialization
13 | bool b = S{1};
| ^~~~
| |
| S
explicit.C:5:12: note: explicit conversion function was not considered
5 | explicit operator bool() const { return true; }
| ^~~~~~~~
gcc/cp/ChangeLog:
* call.cc (convert_like_internal): Show where the conversion function
was declared.
(maybe_show_nonconverting_candidate): New.
* cp-tree.h (maybe_show_nonconverting_candidate): Declare.
* typeck.cc (convert_for_assignment): Call it.
gcc/testsuite/ChangeLog:
* g++.dg/diagnostic/explicit.C: New test.
---
gcc/cp/call.cc | 41 +++++++++++++++++++---
gcc/cp/cp-tree.h | 1 +
gcc/cp/typeck.cc | 6 ++++
gcc/testsuite/g++.dg/diagnostic/explicit.C | 33 +++++++++++++++++
4 files changed, 77 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/explicit.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 23e458d3252..52c9f4265a4 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -8459,12 +8459,21 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
if (pedwarn (loc, 0, "converting to %qT from initializer list "
"would use explicit constructor %qD",
totype, convfn))
- inform (loc, "in C++11 and above a default constructor "
- "can be explicit");
+ {
+ inform (DECL_SOURCE_LOCATION (convfn), "%qD declared here",
+ convfn);
+ inform (loc, "in C++11 and above a default constructor "
+ "can be explicit");
+ }
}
else
- error ("converting to %qT from initializer list would use "
- "explicit constructor %qD", totype, convfn);
+ {
+ auto_diagnostic_group d;
+ error ("converting to %qT from initializer list would use "
+ "explicit constructor %qD", totype, convfn);
+ inform (DECL_SOURCE_LOCATION (convfn), "%qD declared here",
+ convfn);
+ }
}
/* If we're initializing from {}, it's value-initialization. */
@@ -14323,4 +14332,28 @@ is_list_ctor (tree decl)
return true;
}
+/* We know that can_convert_arg_bad already said "no" when trying to convert
+ FROM to TO with ARG and FLAGS. Try to figure out if it was because
+ an explicit conversion function was skipped when looking for a way to
+ perform the conversion. At this point we've already printed an error. */
+
+void
+maybe_show_nonconverting_candidate (tree to, tree from, tree arg, int flags)
+{
+ if (!(flags & LOOKUP_ONLYCONVERTING))
+ return;
+
+ conversion_obstack_sentinel cos;
+ conversion *c = implicit_conversion (to, from, arg, /*c_cast_p=*/false,
+ flags & ~LOOKUP_ONLYCONVERTING, tf_none);
+ if (c && !c->bad_p && c->user_conv_p)
+ /* Ay, the conversion would have worked in copy-init context. */
+ for (; c; c = next_conversion (c))
+ if (c->kind == ck_user
+ && DECL_P (c->cand->fn)
+ && DECL_NONCONVERTING_P (c->cand->fn))
+ inform (DECL_SOURCE_LOCATION (c->cand->fn), "explicit conversion "
+ "function was not considered");
+}
+
#include "gt-cp-call.h"
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 608d6310e53..6b225ca182f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6727,6 +6727,7 @@ extern bool cp_handle_deprecated_or_unavailable (tree, tsubst_flags_t = tf_warni
extern void cp_warn_deprecated_use_scopes (tree);
extern tree get_function_version_dispatcher (tree);
extern bool any_template_arguments_need_structural_equality_p (tree);
+extern void maybe_show_nonconverting_candidate (tree, tree, tree, int);
/* in class.cc */
extern tree build_vfield_ref (tree, tree);
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index d5c0c85ed51..459739d5866 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10262,6 +10262,8 @@ convert_for_assignment (tree type, tree rhs,
{
range_label_for_type_mismatch label (rhstype, type);
gcc_rich_location richloc (rhs_loc, has_loc ? &label : NULL);
+ auto_diagnostic_group d;
+
switch (errtype)
{
case ICR_DEFAULT_ARGUMENT:
@@ -10296,6 +10298,10 @@ convert_for_assignment (tree type, tree rhs,
gcc_unreachable();
}
}
+
+ /* See if we can be more helpful. */
+ maybe_show_nonconverting_candidate (type, rhstype, rhs, flags);
+
if (TYPE_PTR_P (rhstype)
&& TYPE_PTR_P (type)
&& CLASS_TYPE_P (TREE_TYPE (rhstype))
diff --git a/gcc/testsuite/g++.dg/diagnostic/explicit.C b/gcc/testsuite/g++.dg/diagnostic/explicit.C
new file mode 100644
index 00000000000..122accb86c1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/explicit.C
@@ -0,0 +1,33 @@
+// { dg-do compile { target c++11 } }
+
+struct A {
+ long int l;
+};
+
+struct S {
+ explicit S(int) { }
+ explicit operator bool() const { return true; } // { dg-message "explicit conversion function was not considered" }
+ explicit operator int() const { return 42; } // { dg-message "explicit conversion function was not considered" }
+ explicit operator char() const { return 42; } // { dg-message "explicit conversion function was not considered" }
+ explicit operator double() const { return 42.; } // { dg-message "explicit conversion function was not considered" }
+ explicit operator float() const { return 42.; } // { dg-message "explicit conversion function was not considered" }
+ explicit operator long() const { return 42.; } // { dg-message "explicit conversion function was not considered" }
+};
+
+double
+f (char)
+{
+ return S{2}; // { dg-error "cannot convert .S. to .double. in return" }
+}
+
+void
+g ()
+{
+ S s = {1}; // { dg-error "would use explicit constructor" }
+ bool b = S{1}; // { dg-error "cannot convert .S. to .bool. in initialization" }
+ int i;
+ i = S{2}; // { dg-error "cannot convert .S. to .int. in assignment" }
+ f (S{3}); // { dg-error "cannot convert .S. to .char." }
+ A a{ S{4} }; // { dg-error "cannot convert .S. to .long int. in initialization" }
+ float arr[1] = { S{5} }; // { dg-error "cannot convert .S. to .float. in initialization" }
+}
base-commit: cf64ab18e3f820376ff20c663c7c7bf1af290f02
--
2.41.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] c++: tweaks for explicit conversion fns diagnostic
2023-08-28 23:24 ` [PATCH v2] " Marek Polacek
@ 2023-08-29 20:42 ` Jason Merrill
2023-08-29 21:45 ` Marek Polacek
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2023-08-29 20:42 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On 8/28/23 19:24, Marek Polacek wrote:
> On Fri, Aug 25, 2023 at 08:34:37PM -0400, Jason Merrill wrote:
>> On 8/25/23 19:37, Marek Polacek wrote:
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> -- >8 --
>>>
>>> 1) When saying that a conversion is erroneous because it would use
>>> an explicit constructor, it might be nice to show where exactly
>>> the explicit constructor is located. For example, with this patch:
>>>
>>> [...]
>>> explicit.C:4:12: note: 'S::S(int)' declared here
>>> 4 | explicit S(int) { }
>>> | ^
>>>
>>> 2) When a conversion doesn't work out merely because the conversion
>>> function necessary to do the conversion couldn't be used because
>>> it was marked explicit, it would be useful to the user to say so,
>>> rather than just saying "cannot convert". For example, with this patch:
>>>
>>> explicit.C:13:12: error: cannot convert 'S' to 'bool' in initialization
>>> 13 | bool b = S{1};
>>> | ^~~~
>>> | |
>>> | S
>>> explicit.C:5:12: note: explicit conversion function was not considered
>>> 5 | explicit operator bool() const { return true; }
>>> | ^~~~~~~~
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> * call.cc (convert_like_internal): Show where the conversion function
>>> was declared.
>>> (maybe_show_nonconverting_candidate): New.
>>> * cp-tree.h (maybe_show_nonconverting_candidate): Declare.
>>> * typeck.cc (convert_for_assignment): Call it.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * g++.dg/diagnostic/explicit.C: New test.
>>> ---
>>> gcc/cp/call.cc | 41 +++++++++++++++++++---
>>> gcc/cp/cp-tree.h | 1 +
>>> gcc/cp/typeck.cc | 5 +++
>>> gcc/testsuite/g++.dg/diagnostic/explicit.C | 16 +++++++++
>>> 4 files changed, 59 insertions(+), 4 deletions(-)
>>> create mode 100644 gcc/testsuite/g++.dg/diagnostic/explicit.C
>>>
>>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>>> index 23e458d3252..09ebcf6a115 100644
>>> --- a/gcc/cp/call.cc
>>> +++ b/gcc/cp/call.cc
>>> @@ -8459,12 +8459,21 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
>>> if (pedwarn (loc, 0, "converting to %qT from initializer list "
>>> "would use explicit constructor %qD",
>>> totype, convfn))
>>> - inform (loc, "in C++11 and above a default constructor "
>>> - "can be explicit");
>>> + {
>>> + inform (loc, "in C++11 and above a default constructor "
>>> + "can be explicit");
>>> + inform (DECL_SOURCE_LOCATION (convfn), "%qD declared here",
>>> + convfn);
>>
>> I'd swap these two informs.
>
> Done.
>
>>> +++ b/gcc/testsuite/g++.dg/diagnostic/explicit.C
>>> @@ -0,0 +1,16 @@
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +struct S {
>>> + explicit S(int) { }
>>> + explicit operator bool() const { return true; } // { dg-message "explicit conversion function was not considered" }
>>> + explicit operator int() const { return 42; } // { dg-message "explicit conversion function was not considered" }
>>> +};
>>> +
>>> +void
>>> +g ()
>>> +{
>>> + S s = {1}; // { dg-error "would use explicit constructor" }
>>> + bool b = S{1}; // { dg-error "cannot convert .S. to .bool. in initialization" }
>>> + int i;
>>> + i = S{2}; // { dg-error "cannot convert .S. to .int. in assignment" }
>>> +}
>>
>> Let's also test other copy-initialization contexts: parameter passing,
>> return, throw, aggregate member initialization.
>
> Done except for throw. To handle arg passing I moved the call to
> maybe_show_nonconverting_candidate one line down. I guess a testcase
> for throw would be
>
> struct T {
> T() { } // #1
> explicit T(const T&) { } // #2
> };
>
> void
> g ()
> {
> T t{};
> throw t;
> }
>
> but #2 isn't a viable candidate so this would take more effort to handle.
True, copy-initialization is different when the types are the same.
> We just say about #1 that "candidate expects 0 arguments, 1 provided".
>
> clang++ says
>
> e.C:3:12: note: explicit constructor is not a candidate
> 3 | explicit T(const T&) { }
> | ^
That would be better; in add_candidates when we see an explicit
constructor we could add it to bad_fns instead of ignoring it. But that
doesn't need to be part of this patch.
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >8 --
> 1) When saying that a conversion is erroneous because it would use
> an explicit constructor, it might be nice to show where exactly
> the explicit constructor is located. For example, with this patch:
>
> [...]
> explicit.C:4:12: note: 'S::S(int)' declared here
> 4 | explicit S(int) { }
> | ^
>
> 2) When a conversion doesn't work out merely because the conversion
> function necessary to do the conversion couldn't be used because
> it was marked explicit, it would be useful to the user to say so,
> rather than just saying "cannot convert". For example, with this patch:
>
> explicit.C:13:12: error: cannot convert 'S' to 'bool' in initialization
> 13 | bool b = S{1};
> | ^~~~
> | |
> | S
> explicit.C:5:12: note: explicit conversion function was not considered
> 5 | explicit operator bool() const { return true; }
> | ^~~~~~~~
>
> gcc/cp/ChangeLog:
>
> * call.cc (convert_like_internal): Show where the conversion function
> was declared.
> (maybe_show_nonconverting_candidate): New.
> * cp-tree.h (maybe_show_nonconverting_candidate): Declare.
> * typeck.cc (convert_for_assignment): Call it.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/diagnostic/explicit.C: New test.
> ---
> gcc/cp/call.cc | 41 +++++++++++++++++++---
> gcc/cp/cp-tree.h | 1 +
> gcc/cp/typeck.cc | 6 ++++
> gcc/testsuite/g++.dg/diagnostic/explicit.C | 33 +++++++++++++++++
> 4 files changed, 77 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/diagnostic/explicit.C
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 23e458d3252..52c9f4265a4 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -14323,4 +14332,28 @@ is_list_ctor (tree decl)
> return true;
> }
>
> +/* We know that can_convert_arg_bad already said "no" when trying to convert
> + FROM to TO with ARG and FLAGS. Try to figure out if it was because
> + an explicit conversion function was skipped when looking for a way to
> + perform the conversion. At this point we've already printed an error. */
> +
> +void
> +maybe_show_nonconverting_candidate (tree to, tree from, tree arg, int flags)
> +{
> + if (!(flags & LOOKUP_ONLYCONVERTING))
> + return;
> +
> + conversion_obstack_sentinel cos;
> + conversion *c = implicit_conversion (to, from, arg, /*c_cast_p=*/false,
> + flags & ~LOOKUP_ONLYCONVERTING, tf_none);
> + if (c && !c->bad_p && c->user_conv_p)
> + /* Ay, the conversion would have worked in copy-init context. */
s/copy/direct/
OK with that change.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] c++: tweaks for explicit conversion fns diagnostic
2023-08-29 20:42 ` Jason Merrill
@ 2023-08-29 21:45 ` Marek Polacek
0 siblings, 0 replies; 5+ messages in thread
From: Marek Polacek @ 2023-08-29 21:45 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Tue, Aug 29, 2023 at 04:42:33PM -0400, Jason Merrill wrote:
> On 8/28/23 19:24, Marek Polacek wrote:
> > On Fri, Aug 25, 2023 at 08:34:37PM -0400, Jason Merrill wrote:
> > > On 8/25/23 19:37, Marek Polacek wrote:
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > >
> > > > -- >8 --
> > > >
> > > > 1) When saying that a conversion is erroneous because it would use
> > > > an explicit constructor, it might be nice to show where exactly
> > > > the explicit constructor is located. For example, with this patch:
> > > >
> > > > [...]
> > > > explicit.C:4:12: note: 'S::S(int)' declared here
> > > > 4 | explicit S(int) { }
> > > > | ^
> > > >
> > > > 2) When a conversion doesn't work out merely because the conversion
> > > > function necessary to do the conversion couldn't be used because
> > > > it was marked explicit, it would be useful to the user to say so,
> > > > rather than just saying "cannot convert". For example, with this patch:
> > > >
> > > > explicit.C:13:12: error: cannot convert 'S' to 'bool' in initialization
> > > > 13 | bool b = S{1};
> > > > | ^~~~
> > > > | |
> > > > | S
> > > > explicit.C:5:12: note: explicit conversion function was not considered
> > > > 5 | explicit operator bool() const { return true; }
> > > > | ^~~~~~~~
> > > >
> > > > gcc/cp/ChangeLog:
> > > >
> > > > * call.cc (convert_like_internal): Show where the conversion function
> > > > was declared.
> > > > (maybe_show_nonconverting_candidate): New.
> > > > * cp-tree.h (maybe_show_nonconverting_candidate): Declare.
> > > > * typeck.cc (convert_for_assignment): Call it.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > * g++.dg/diagnostic/explicit.C: New test.
> > > > ---
> > > > gcc/cp/call.cc | 41 +++++++++++++++++++---
> > > > gcc/cp/cp-tree.h | 1 +
> > > > gcc/cp/typeck.cc | 5 +++
> > > > gcc/testsuite/g++.dg/diagnostic/explicit.C | 16 +++++++++
> > > > 4 files changed, 59 insertions(+), 4 deletions(-)
> > > > create mode 100644 gcc/testsuite/g++.dg/diagnostic/explicit.C
> > > >
> > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > > > index 23e458d3252..09ebcf6a115 100644
> > > > --- a/gcc/cp/call.cc
> > > > +++ b/gcc/cp/call.cc
> > > > @@ -8459,12 +8459,21 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
> > > > if (pedwarn (loc, 0, "converting to %qT from initializer list "
> > > > "would use explicit constructor %qD",
> > > > totype, convfn))
> > > > - inform (loc, "in C++11 and above a default constructor "
> > > > - "can be explicit");
> > > > + {
> > > > + inform (loc, "in C++11 and above a default constructor "
> > > > + "can be explicit");
> > > > + inform (DECL_SOURCE_LOCATION (convfn), "%qD declared here",
> > > > + convfn);
> > >
> > > I'd swap these two informs.
> >
> > Done.
> > > > +++ b/gcc/testsuite/g++.dg/diagnostic/explicit.C
> > > > @@ -0,0 +1,16 @@
> > > > +// { dg-do compile { target c++11 } }
> > > > +
> > > > +struct S {
> > > > + explicit S(int) { }
> > > > + explicit operator bool() const { return true; } // { dg-message "explicit conversion function was not considered" }
> > > > + explicit operator int() const { return 42; } // { dg-message "explicit conversion function was not considered" }
> > > > +};
> > > > +
> > > > +void
> > > > +g ()
> > > > +{
> > > > + S s = {1}; // { dg-error "would use explicit constructor" }
> > > > + bool b = S{1}; // { dg-error "cannot convert .S. to .bool. in initialization" }
> > > > + int i;
> > > > + i = S{2}; // { dg-error "cannot convert .S. to .int. in assignment" }
> > > > +}
> > >
> > > Let's also test other copy-initialization contexts: parameter passing,
> > > return, throw, aggregate member initialization.
> >
> > Done except for throw. To handle arg passing I moved the call to
> > maybe_show_nonconverting_candidate one line down. I guess a testcase
> > for throw would be
> >
> > struct T {
> > T() { } // #1
> > explicit T(const T&) { } // #2
> > };
> >
> > void
> > g ()
> > {
> > T t{};
> > throw t;
> > }
> >
> > but #2 isn't a viable candidate so this would take more effort to handle.
>
> True, copy-initialization is different when the types are the same.
>
> > We just say about #1 that "candidate expects 0 arguments, 1 provided".
> >
> > clang++ says
> >
> > e.C:3:12: note: explicit constructor is not a candidate
> > 3 | explicit T(const T&) { }
> > | ^
>
> That would be better; in add_candidates when we see an explicit constructor
> we could add it to bad_fns instead of ignoring it. But that doesn't need to
> be part of this patch.
I created https://gcc.gnu.org/PR111230 for that.
> > +void
> > +maybe_show_nonconverting_candidate (tree to, tree from, tree arg, int flags)
> > +{
> > + if (!(flags & LOOKUP_ONLYCONVERTING))
> > + return;
> > +
> > + conversion_obstack_sentinel cos;
> > + conversion *c = implicit_conversion (to, from, arg, /*c_cast_p=*/false,
> > + flags & ~LOOKUP_ONLYCONVERTING, tf_none);
> > + if (c && !c->bad_p && c->user_conv_p)
> > + /* Ay, the conversion would have worked in copy-init context. */
>
> s/copy/direct/
>
> OK with that change.
Duh, of course. I'm pushing the patch with that fixed.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-29 21:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-25 23:37 [PATCH] c++: tweaks for explicit conversion fns diagnostic Marek Polacek
2023-08-26 0:34 ` Jason Merrill
2023-08-28 23:24 ` [PATCH v2] " Marek Polacek
2023-08-29 20:42 ` Jason Merrill
2023-08-29 21:45 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).