* [PATCH] C++: improvements to diagnostics that use %P (more PR c++/85110)
@ 2018-11-11 22:59 David Malcolm
2018-12-03 22:06 ` [PATCH v2] C++: improvements to diagnostics using " David Malcolm
0 siblings, 1 reply; 5+ messages in thread
From: David Malcolm @ 2018-11-11 22:59 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch is based on grepping the C++ frontend for %P and %qP
i.e. diagnostics that refer to a parameter number. It fixes up
these diagnostics to highlight the pertinent param, where appropriate
(and possible), along with various other tweaks, as described in the
ChangeLog.
OK for trunk if it passes testing?
gcc/cp/ChangeLog:
PR c++/85110
* call.c (conversion_null_warnings): Try to use the location of
the expression for the warnings. Add notes showing the parameter
of the function decl, where available.
(get_fndecl_argument_location): Gracefully reject
non-FUNCTION_DECLs.
(maybe_inform_about_fndecl_for_bogus_argument_init): New function.
(convert_like_real): Use it in various places to avoid repetition.
* cp-tree.h (maybe_inform_about_fndecl_for_bogus_argument_init):
New declaration.
* decl2.c: Include "gcc-rich-location.h".
(check_default_args): Use the location of the parameter when
complaining about parameters with missing default arguments in
preference to that of the fndecl.
Attempt to record the location of first parameter with a default
argument and add it as a secondary range to such errors.
* typeck.c (convert_arguments): When complaining about parameters
with incomplete types, attempt to use the location of the
argument. Where available, add a note showing the pertinent
parameter in the fndecl.
(convert_for_assignment): When complaining about bad conversions
at function calls, use the location of the unstripped argument.
Update the note about the callee to use the location of the
pertinent parameter, and to use
maybe_inform_about_fndecl_for_bogus_argument_init.
(convert_for_initialization): When checking for bogus references,
add an auto_diagnostic_group, and update the note to use the
location of the pertinent parameter, rather than just the callee.
gcc/testsuite/ChangeLog:
PR c++/85110
* g++.dg/diagnostic/missing-default-args.C: New test.
* g++.dg/diagnostic/param-type-mismatch-3.C: New test.
* g++.dg/diagnostic/param-type-mismatch.C: Add tests for invalid
references and incomplete types.
* g++.dg/warn/Wconversion-null-4.C: New test.
---
gcc/cp/call.c | 83 ++++++++++++++--------
gcc/cp/cp-tree.h | 1 +
gcc/cp/decl2.c | 16 ++++-
gcc/cp/typeck.c | 19 +++--
.../g++.dg/diagnostic/missing-default-args.C | 53 ++++++++++++++
.../g++.dg/diagnostic/param-type-mismatch-3.C | 26 +++++++
.../g++.dg/diagnostic/param-type-mismatch.C | 41 +++++++++++
gcc/testsuite/g++.dg/warn/Wconversion-null-4.C | 37 ++++++++++
8 files changed, 238 insertions(+), 38 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 6f40156..ea5e73b 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6636,16 +6636,24 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
&& ARITHMETIC_TYPE_P (totype))
{
- source_location loc =
- expansion_point_location_if_in_system_header (input_location);
-
if (fn)
- warning_at (loc, OPT_Wconversion_null,
- "passing NULL to non-pointer argument %P of %qD",
- argnum, fn);
+ {
+ source_location loc = EXPR_LOC_OR_LOC (expr, input_location);
+ loc = expansion_point_location_if_in_system_header (loc);
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wconversion_null,
+ "passing NULL to non-pointer argument %P of %qD",
+ argnum, fn))
+ inform (get_fndecl_argument_location (fn, argnum),
+ " declared here");
+ }
else
- warning_at (loc, OPT_Wconversion_null,
- "converting to non-pointer type %qT from NULL", totype);
+ {
+ source_location loc
+ = expansion_point_location_if_in_system_header (input_location);
+ warning_at (loc, OPT_Wconversion_null,
+ "converting to non-pointer type %qT from NULL", totype);
+ }
}
/* Issue warnings if "false" is converted to a NULL pointer */
@@ -6653,9 +6661,15 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
&& TYPE_PTR_P (totype))
{
if (fn)
- warning_at (input_location, OPT_Wconversion_null,
- "converting %<false%> to pointer type for argument %P "
- "of %qD", argnum, fn);
+ {
+ source_location loc = EXPR_LOC_OR_LOC (expr, input_location);
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wconversion_null,
+ "converting %<false%> to pointer type for argument "
+ "%P of %qD", argnum, fn))
+ inform (get_fndecl_argument_location (fn, argnum),
+ " declared here");
+ }
else
warning_at (input_location, OPT_Wconversion_null,
"converting %<false%> to pointer type %qT", totype);
@@ -6695,6 +6709,10 @@ maybe_print_user_conv_context (conversion *convs)
location_t
get_fndecl_argument_location (tree fndecl, int argnum)
{
+ /* Gracefully fail for e.g. TEMPLATE_DECL. */
+ if (TREE_CODE (fndecl) != FUNCTION_DECL)
+ return DECL_SOURCE_LOCATION (fndecl);
+
int i;
tree param;
@@ -6712,6 +6730,18 @@ get_fndecl_argument_location (tree fndecl, int argnum)
return DECL_SOURCE_LOCATION (param);
}
+/* If FNDECL is non-NULL, issue a note highlighting ARGNUM
+ within its declaration (or the fndecl itself if something went
+ wrong). */
+
+void
+maybe_inform_about_fndecl_for_bogus_argument_init (tree fn, int argnum)
+{
+ if (fn)
+ inform (get_fndecl_argument_location (fn, argnum),
+ " initializing argument %P of %qD", argnum, fn);
+}
+
/* Perform the conversions in CONVS on the expression EXPR. FN and
ARGNUM are used for diagnostics. ARGNUM is zero based, -1
indicates the `this' argument of a method. INNER is nonzero when
@@ -6789,9 +6819,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
complain);
else
expr = cp_convert (totype, expr, complain);
- if (complained && fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ if (complained)
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
return expr;
}
else if (t->kind == ck_user || !t->bad_p)
@@ -6818,9 +6847,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
"invalid conversion from %qH to %qI",
TREE_TYPE (expr), totype);
}
- if (complained && fn)
- inform (get_fndecl_argument_location (fn, argnum),
- " initializing argument %P of %qD", argnum, fn);
+ if (complained)
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
return cp_convert (totype, expr, complain);
}
@@ -6942,9 +6970,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
? LOOKUP_IMPLICIT : LOOKUP_NORMAL);
build_user_type_conversion (totype, convs->u.expr, flags, complain);
gcc_assert (seen_error ());
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return error_mark_node;
@@ -7038,9 +7064,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
{
auto_diagnostic_group d;
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return error_mark_node;
}
@@ -7091,9 +7115,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
{
auto_diagnostic_group d;
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return build_cplus_new (totype, expr, complain);
@@ -7120,9 +7142,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
else
gcc_unreachable ();
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
+
return error_mark_node;
}
@@ -9254,8 +9275,8 @@ complain_about_bad_argument (location_t arg_loc,
error_at (&richloc,
"cannot convert %qH to %qI",
from_type, to_type);
- inform (get_fndecl_argument_location (fndecl, parmnum),
- " initializing argument %P of %qD", parmnum, fndecl);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fndecl,
+ parmnum);
}
/* Subroutine of build_new_method_call_1, for where there are no viable
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 4178829..42f996e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6128,6 +6128,7 @@ extern location_t get_fndecl_argument_location (tree, int);
extern void complain_about_bad_argument (location_t arg_loc,
tree from_type, tree to_type,
tree fndecl, int parmnum);
+extern void maybe_inform_about_fndecl_for_bogus_argument_init (tree, int);
/* A class for recording information about access failures (e.g. private
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index a163558..9fd564a 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not see
#include "intl.h"
#include "c-family/c-ada-spec.h"
#include "asan.h"
+#include "gcc-rich-location.h"
/* Id for dumping the raw trees. */
int raw_dump_id;
@@ -5121,14 +5122,25 @@ check_default_args (tree x)
{
tree arg = TYPE_ARG_TYPES (TREE_TYPE (x));
bool saw_def = false;
+ location_t loc_of_first_default_arg = UNKNOWN_LOCATION;
int i = 0 - (TREE_CODE (TREE_TYPE (x)) == METHOD_TYPE);
for (; arg && arg != void_list_node; arg = TREE_CHAIN (arg), ++i)
{
if (TREE_PURPOSE (arg))
- saw_def = true;
+ {
+ saw_def = true;
+ location_t loc = get_fndecl_argument_location (x, i);
+ if (loc != DECL_SOURCE_LOCATION (x))
+ loc_of_first_default_arg = loc;
+ }
else if (saw_def && !PACK_EXPANSION_P (TREE_VALUE (arg)))
{
- error ("default argument missing for parameter %P of %q+#D", i, x);
+ location_t loc = get_fndecl_argument_location (x, i);
+ gcc_rich_location richloc (loc);
+ if (loc_of_first_default_arg != UNKNOWN_LOCATION)
+ richloc.add_range (loc_of_first_default_arg);
+ error_at (&richloc,
+ "default argument missing for parameter %P of %q#D", i, x);
TREE_PURPOSE (arg) = error_mark_node;
}
}
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index a58050c..9801c11 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4014,11 +4014,19 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl,
{
if (complain & tf_error)
{
+ location_t loc = EXPR_LOC_OR_LOC (val, input_location);
if (fndecl)
- error ("parameter %P of %qD has incomplete type %qT",
- i, fndecl, type);
+ {
+ auto_diagnostic_group d;
+ error_at (loc,
+ "parameter %P of %qD has incomplete type %qT",
+ i, fndecl, type);
+ inform (get_fndecl_argument_location (fndecl, i),
+ " declared here");
+ }
else
- error ("parameter %P has incomplete type %qT", i, type);
+ error_at (loc, "parameter %P has incomplete type %qT", i,
+ type);
}
parmval = error_mark_node;
}
@@ -8801,6 +8809,7 @@ convert_for_assignment (tree type, tree rhs,
{
tree rhstype;
enum tree_code coder;
+ location_t rhs_loc = EXPR_LOC_OR_LOC (rhs, input_location);
/* Strip NON_LVALUE_EXPRs since we aren't using as an lvalue. */
if (TREE_CODE (rhs) == NON_LVALUE_EXPR)
@@ -9075,6 +9084,7 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
if (codel == REFERENCE_TYPE)
{
+ auto_diagnostic_group d;
/* This should eventually happen in convert_arguments. */
int savew = 0, savee = 0;
@@ -9084,9 +9094,8 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
if (fndecl
&& (warningcount + werrorcount > savew || errorcount > savee))
- inform (DECL_SOURCE_LOCATION (fndecl),
+ inform (get_fndecl_argument_location (fndecl, parmnum),
"in passing argument %P of %qD", parmnum, fndecl);
-
return rhs;
}
diff --git a/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C b/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
new file mode 100644
index 0000000..f681842
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
@@ -0,0 +1,53 @@
+// { dg-options "-fdiagnostics-show-caret" }
+
+/* Function. */
+
+void test_1 (int a, int b = 42, int c, int d); // { dg-line test_1 }
+
+// { dg-error "default argument missing for parameter 3 of " "" { target *-*-* } test_1 }
+/* { dg-begin-multiline-output "" }
+ void test_1 (int a, int b = 42, int c, int d);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
+
+// { dg-error "default argument missing for parameter 4 of " "" { target *-*-* } test_1 }
+/* { dg-begin-multiline-output "" }
+ void test_1 (int a, int b = 42, int c, int d);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
+
+
+/* Non-static member fn. */
+
+struct test_2
+{
+ void member_2 (int a, int b = 42, int c); // { dg-error "default argument missing for parameter 3 of " }
+};
+
+
+/* { dg-begin-multiline-output "" }
+ void member_2 (int a, int b = 42, int c);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
+
+
+/* Static member fn. */
+
+struct test_3
+{
+ static void member_3 (int a, int b = 42, int c); // { dg-error "default argument missing for parameter 3 of " }
+};
+/* { dg-begin-multiline-output "" }
+ static void member_3 (int a, int b = 42, int c);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
+
+
+/* Template. */
+
+template <typename Type>
+void test_4 (int a, int b = 42, int c); // { dg-error "default argument missing for parameter 3 of " }
+/* { dg-begin-multiline-output "" }
+ void test_4 (int a, int b = 42, int c);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
new file mode 100644
index 0000000..0534e6a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
@@ -0,0 +1,26 @@
+// { dg-do compile { target c++14 } }
+// { dg-options "-fdiagnostics-show-caret" }
+
+void f2(int, char (*)(int), int) { } // { dg-line f2 }
+
+void test_1 ()
+{
+ auto glambda = [](auto a) { return a; }; // { dg-line candidate }
+ int (*fp)(int) = glambda;
+ f2(1, glambda, 3); // { dg-error "invalid user-defined conversion" }
+ /* { dg-begin-multiline-output "" }
+ f2(1, glambda, 3);
+ ^~~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "candidate is: " "" { target *-*-* } candidate }
+ /* { dg-begin-multiline-output "" }
+ auto glambda = [](auto a) { return a; };
+ ^
+ { dg-end-multiline-output "" } */
+ // { dg-message "no known conversion from " "" { target *-*-* } candidate }
+ // { dg-message "initializing argument 2 of " "" { target *-*-* } f2 }
+ /* { dg-begin-multiline-output "" }
+ void f2(int, char (*)(int), int) { }
+ ^~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
index 50bbd4a..ac101b6 100644
--- a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
+++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
@@ -226,4 +226,45 @@ int test_11 (int first, int second, float third)
{ dg-end-multiline-output "" } */
}
+/* Bad reference. */
+
+struct s12;
+
+extern int callee_12 (int one, s12 &second, float three); // { dg-line callee_12 }
+
+int test_12 (int first, int second, float third)
+{
+ return callee_12 (first, second, third); // { dg-error "invalid initialization of reference of " }
+ /* { dg-begin-multiline-output "" }
+ return callee_12 (first, second, third);
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "in passing argument 2 of " "" { target *-*-* } callee_12 }
+ /* { dg-begin-multiline-output "" }
+ extern int callee_12 (int one, s12 &second, float three);
+ ~~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+/* Incomplete type. */
+
+struct s13;
+
+extern int callee_13 (int one, s13 second, float three); // { dg-line callee_13 }
+
+int test_13 (int first, int second, float third)
+{
+ return callee_13 (first, second, third); // { dg-error "has incomplete type" }
+ /* { dg-begin-multiline-output "" }
+ return callee_13 (first, second, third);
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "declared here" "" { target *-*-* } callee_13 }
+ /* { dg-begin-multiline-output "" }
+ extern int callee_13 (int one, s13 second, float three);
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+
// TODO: template callsite
diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C b/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
new file mode 100644
index 0000000..5de21e6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
@@ -0,0 +1,37 @@
+// { dg-do compile }
+// { dg-options "-Wconversion-null -fdiagnostics-show-caret" }
+
+#include <stddef.h>
+
+void callee_1 (int, int, int) {} // { dg-message "declared here" }
+
+void caller_1 (void)
+{
+ callee_1 (0, NULL, 2); // { dg-warning "passing NULL to non-pointer argument 2 of" }
+ /* { dg-begin-multiline-output "" }
+ callee_1 (0, NULL, 2);
+ ^~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ void callee_1 (int, int, int) {}
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+void callee_2 (int, void *, int) {} // { dg-message "declared here" "" { target { ! c++11 } } }
+// { dg-message "initializing argument 2 of " "" { target c++11 } .-1 }
+
+void caller_2 (void)
+{
+ callee_2 (0, false, 2); // { dg-warning "converting 'false' to pointer type for argument 2 of " "" { target { ! c++11 } } }
+ // { dg-error "cannot convert" "" { target c++11 } .-1 }
+
+ /* { dg-begin-multiline-output "" }
+ callee_2 (0, false, 2);
+ ^~~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ void callee_2 (int, void *, int) {}
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] C++: improvements to diagnostics using %P (more PR c++/85110)
2018-11-11 22:59 [PATCH] C++: improvements to diagnostics that use %P (more PR c++/85110) David Malcolm
@ 2018-12-03 22:06 ` David Malcolm
2018-12-06 0:49 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: David Malcolm @ 2018-12-03 22:06 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches, David Malcolm
I was going to ping this patch:
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00875.html
but it has bit-rotted somewhat, so here's a refreshed version of it.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
Thanks
Dave
Blurb from v1:
This patch is based on grepping the C++ frontend for %P
i.e. diagnostics that refer to a parameter number. It fixes up
these diagnostics to highlight the pertinent param where appropriate
(and possible), along with various other tweaks, as described in the
ChangeLog.
gcc/cp/ChangeLog:
PR c++/85110
* call.c (conversion_null_warnings): Try to use the location of
the expression for the warnings. Add notes showing the parameter
of the function decl, where available.
(get_fndecl_argument_location): Gracefully reject
non-FUNCTION_DECLs. For implicitly-declared functions, use the
fndecl location rather than that of the param.
(maybe_inform_about_fndecl_for_bogus_argument_init): New function.
(convert_like_real): Use it in various places to avoid repetition.
* cp-tree.h (maybe_inform_about_fndecl_for_bogus_argument_init):
New declaration.
* decl2.c: Include "gcc-rich-location.h".
(check_default_args): Use the location of the parameter when
complaining about parameters with missing default arguments in
preference to that of the fndecl.
Attempt to record the location of first parameter with a default
argument and add it as a secondary range to such errors.
* typeck.c (convert_arguments): When complaining about parameters
with incomplete types, attempt to use the location of the
argument. Where available, add a note showing the pertinent
parameter in the fndecl.
(convert_for_assignment): When complaining about bad conversions
at function calls, use the location of the unstripped argument.
(convert_for_initialization): When checking for bogus references,
add an auto_diagnostic_group, and update the note to use the
location of the pertinent parameter, rather than just the callee.
gcc/testsuite/ChangeLog:
PR c++/85110
* g++.dg/diagnostic/missing-default-args.C: New test.
* g++.dg/diagnostic/param-type-mismatch-3.C: New test.
* g++.dg/diagnostic/param-type-mismatch.C: Add tests for invalid
references and incomplete types.
* g++.dg/warn/Wconversion-null-4.C: New test.
---
gcc/cp/call.c | 88 ++++++++++++++--------
gcc/cp/cp-tree.h | 1 +
gcc/cp/decl2.c | 16 +++-
gcc/cp/typeck.c | 22 ++++--
.../g++.dg/diagnostic/missing-default-args.C | 53 +++++++++++++
.../g++.dg/diagnostic/param-type-mismatch-3.C | 26 +++++++
.../g++.dg/diagnostic/param-type-mismatch.C | 41 ++++++++++
gcc/testsuite/g++.dg/warn/Wconversion-null-4.C | 43 +++++++++++
8 files changed, 251 insertions(+), 39 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ee099cc..cfc5641 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6681,16 +6681,24 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
&& ARITHMETIC_TYPE_P (totype))
{
- location_t loc =
- expansion_point_location_if_in_system_header (input_location);
-
if (fn)
- warning_at (loc, OPT_Wconversion_null,
- "passing NULL to non-pointer argument %P of %qD",
- argnum, fn);
+ {
+ location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
+ loc = expansion_point_location_if_in_system_header (loc);
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wconversion_null,
+ "passing NULL to non-pointer argument %P of %qD",
+ argnum, fn))
+ inform (get_fndecl_argument_location (fn, argnum),
+ " declared here");
+ }
else
- warning_at (loc, OPT_Wconversion_null,
- "converting to non-pointer type %qT from NULL", totype);
+ {
+ location_t loc
+ = expansion_point_location_if_in_system_header (input_location);
+ warning_at (loc, OPT_Wconversion_null,
+ "converting to non-pointer type %qT from NULL", totype);
+ }
}
/* Issue warnings if "false" is converted to a NULL pointer */
@@ -6698,9 +6706,15 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
&& TYPE_PTR_P (totype))
{
if (fn)
- warning_at (input_location, OPT_Wconversion_null,
- "converting %<false%> to pointer type for argument %P "
- "of %qD", argnum, fn);
+ {
+ location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wconversion_null,
+ "converting %<false%> to pointer type for argument "
+ "%P of %qD", argnum, fn))
+ inform (get_fndecl_argument_location (fn, argnum),
+ " declared here");
+ }
else
warning_at (input_location, OPT_Wconversion_null,
"converting %<false%> to pointer type %qT", totype);
@@ -6740,6 +6754,15 @@ maybe_print_user_conv_context (conversion *convs)
location_t
get_fndecl_argument_location (tree fndecl, int argnum)
{
+ /* Gracefully fail for e.g. TEMPLATE_DECL. */
+ if (TREE_CODE (fndecl) != FUNCTION_DECL)
+ return DECL_SOURCE_LOCATION (fndecl);
+
+ /* The locations of implicitly-declared functions are likely to be
+ more meaningful than those of their parameters. */
+ if (DECL_ARTIFICIAL (fndecl))
+ return DECL_SOURCE_LOCATION (fndecl);
+
int i;
tree param;
@@ -6757,6 +6780,18 @@ get_fndecl_argument_location (tree fndecl, int argnum)
return DECL_SOURCE_LOCATION (param);
}
+/* If FNDECL is non-NULL, issue a note highlighting ARGNUM
+ within its declaration (or the fndecl itself if something went
+ wrong). */
+
+void
+maybe_inform_about_fndecl_for_bogus_argument_init (tree fn, int argnum)
+{
+ if (fn)
+ inform (get_fndecl_argument_location (fn, argnum),
+ " initializing argument %P of %qD", argnum, fn);
+}
+
/* Perform the conversions in CONVS on the expression EXPR. FN and
ARGNUM are used for diagnostics. ARGNUM is zero based, -1
indicates the `this' argument of a method. INNER is nonzero when
@@ -6834,9 +6869,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
complain);
else
expr = cp_convert (totype, expr, complain);
- if (complained && fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ if (complained)
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
return expr;
}
else if (t->kind == ck_user || !t->bad_p)
@@ -6863,9 +6897,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
"invalid conversion from %qH to %qI",
TREE_TYPE (expr), totype);
}
- if (complained && fn)
- inform (get_fndecl_argument_location (fn, argnum),
- " initializing argument %P of %qD", argnum, fn);
+ if (complained)
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
return cp_convert (totype, expr, complain);
}
@@ -6987,9 +7020,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
? LOOKUP_IMPLICIT : LOOKUP_NORMAL);
build_user_type_conversion (totype, convs->u.expr, flags, complain);
gcc_assert (seen_error ());
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return error_mark_node;
@@ -7083,9 +7114,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
{
auto_diagnostic_group d;
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return error_mark_node;
}
@@ -7136,9 +7165,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
{
auto_diagnostic_group d;
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return build_cplus_new (totype, expr, complain);
@@ -7165,9 +7192,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
else
gcc_unreachable ();
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
+
return error_mark_node;
}
@@ -9299,8 +9325,8 @@ complain_about_bad_argument (location_t arg_loc,
error_at (&richloc,
"cannot convert %qH to %qI",
from_type, to_type);
- inform (get_fndecl_argument_location (fndecl, parmnum),
- " initializing argument %P of %qD", parmnum, fndecl);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fndecl,
+ parmnum);
}
/* Subroutine of build_new_method_call_1, for where there are no viable
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 111a123..c1f9931 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6145,6 +6145,7 @@ extern location_t get_fndecl_argument_location (tree, int);
extern void complain_about_bad_argument (location_t arg_loc,
tree from_type, tree to_type,
tree fndecl, int parmnum);
+extern void maybe_inform_about_fndecl_for_bogus_argument_init (tree, int);
/* A class for recording information about access failures (e.g. private
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index ffc0d0d..265826a 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not see
#include "intl.h"
#include "c-family/c-ada-spec.h"
#include "asan.h"
+#include "gcc-rich-location.h"
/* Id for dumping the raw trees. */
int raw_dump_id;
@@ -5179,14 +5180,25 @@ check_default_args (tree x)
{
tree arg = TYPE_ARG_TYPES (TREE_TYPE (x));
bool saw_def = false;
+ location_t loc_of_first_default_arg = UNKNOWN_LOCATION;
int i = 0 - (TREE_CODE (TREE_TYPE (x)) == METHOD_TYPE);
for (; arg && arg != void_list_node; arg = TREE_CHAIN (arg), ++i)
{
if (TREE_PURPOSE (arg))
- saw_def = true;
+ {
+ saw_def = true;
+ location_t loc = get_fndecl_argument_location (x, i);
+ if (loc != DECL_SOURCE_LOCATION (x))
+ loc_of_first_default_arg = loc;
+ }
else if (saw_def && !PACK_EXPANSION_P (TREE_VALUE (arg)))
{
- error ("default argument missing for parameter %P of %q+#D", i, x);
+ location_t loc = get_fndecl_argument_location (x, i);
+ gcc_rich_location richloc (loc);
+ if (loc_of_first_default_arg != UNKNOWN_LOCATION)
+ richloc.add_range (loc_of_first_default_arg);
+ error_at (&richloc,
+ "default argument missing for parameter %P of %q#D", i, x);
TREE_PURPOSE (arg) = error_mark_node;
}
}
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index f45c06e..c3807de 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4007,11 +4007,19 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl,
{
if (complain & tf_error)
{
+ location_t loc = EXPR_LOC_OR_LOC (val, input_location);
if (fndecl)
- error ("parameter %P of %qD has incomplete type %qT",
- i, fndecl, type);
+ {
+ auto_diagnostic_group d;
+ error_at (loc,
+ "parameter %P of %qD has incomplete type %qT",
+ i, fndecl, type);
+ inform (get_fndecl_argument_location (fndecl, i),
+ " declared here");
+ }
else
- error ("parameter %P has incomplete type %qT", i, type);
+ error_at (loc, "parameter %P has incomplete type %qT", i,
+ type);
}
parmval = error_mark_node;
}
@@ -8795,6 +8803,8 @@ convert_for_assignment (tree type, tree rhs,
tree rhstype;
enum tree_code coder;
+ location_t rhs_loc = EXPR_LOC_OR_LOC (rhs, input_location);
+
/* Strip NON_LVALUE_EXPRs since we aren't using as an lvalue. */
if (TREE_CODE (rhs) == NON_LVALUE_EXPR)
rhs = TREE_OPERAND (rhs, 0);
@@ -8901,7 +8911,7 @@ convert_for_assignment (tree type, tree rhs,
parmnum, complain, flags);
}
else if (fndecl)
- complain_about_bad_argument (cp_expr_location (rhs),
+ complain_about_bad_argument (rhs_loc,
rhstype, type,
fndecl, parmnum);
else
@@ -9068,6 +9078,7 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
if (codel == REFERENCE_TYPE)
{
+ auto_diagnostic_group d;
/* This should eventually happen in convert_arguments. */
int savew = 0, savee = 0;
@@ -9077,9 +9088,8 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
if (fndecl
&& (warningcount + werrorcount > savew || errorcount > savee))
- inform (DECL_SOURCE_LOCATION (fndecl),
+ inform (get_fndecl_argument_location (fndecl, parmnum),
"in passing argument %P of %qD", parmnum, fndecl);
-
return rhs;
}
diff --git a/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C b/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
new file mode 100644
index 0000000..f681842
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
@@ -0,0 +1,53 @@
+// { dg-options "-fdiagnostics-show-caret" }
+
+/* Function. */
+
+void test_1 (int a, int b = 42, int c, int d); // { dg-line test_1 }
+
+// { dg-error "default argument missing for parameter 3 of " "" { target *-*-* } test_1 }
+/* { dg-begin-multiline-output "" }
+ void test_1 (int a, int b = 42, int c, int d);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
+
+// { dg-error "default argument missing for parameter 4 of " "" { target *-*-* } test_1 }
+/* { dg-begin-multiline-output "" }
+ void test_1 (int a, int b = 42, int c, int d);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
+
+
+/* Non-static member fn. */
+
+struct test_2
+{
+ void member_2 (int a, int b = 42, int c); // { dg-error "default argument missing for parameter 3 of " }
+};
+
+
+/* { dg-begin-multiline-output "" }
+ void member_2 (int a, int b = 42, int c);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
+
+
+/* Static member fn. */
+
+struct test_3
+{
+ static void member_3 (int a, int b = 42, int c); // { dg-error "default argument missing for parameter 3 of " }
+};
+/* { dg-begin-multiline-output "" }
+ static void member_3 (int a, int b = 42, int c);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
+
+
+/* Template. */
+
+template <typename Type>
+void test_4 (int a, int b = 42, int c); // { dg-error "default argument missing for parameter 3 of " }
+/* { dg-begin-multiline-output "" }
+ void test_4 (int a, int b = 42, int c);
+ ~~~~~~~~~~ ~~~~^
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
new file mode 100644
index 0000000..3ffbbd8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
@@ -0,0 +1,26 @@
+// { dg-do compile { target c++14 } }
+// { dg-options "-fdiagnostics-show-caret" }
+
+void f2(int, char (*)(int), int) { } // { dg-line f2 }
+
+void test_1 ()
+{
+ auto glambda = [](auto a) { return a; }; // { dg-line candidate }
+ int (*fp)(int) = glambda;
+ f2(1, glambda, 3); // { dg-error "invalid user-defined conversion" }
+ /* { dg-begin-multiline-output "" }
+ f2(1, glambda, 3);
+ ^~~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "candidate is: " "" { target *-*-* } candidate }
+ /* { dg-begin-multiline-output "" }
+ auto glambda = [](auto a) { return a; };
+ ^
+ { dg-end-multiline-output "" } */
+ // { dg-message "no known conversion from " "" { target *-*-* } candidate }
+ // { dg-message "initializing argument 2 of " "" { target *-*-* } f2 }
+ /* { dg-begin-multiline-output "" }
+ void f2(int, char (*)(int), int) { }
+ ^~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
index cb5c360..54ffb06 100644
--- a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
+++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
@@ -224,4 +224,45 @@ int test_11 (int first, int second, float third)
{ dg-end-multiline-output "" } */
}
+/* Bad reference. */
+
+struct s12;
+
+extern int callee_12 (int one, s12 &second, float three); // { dg-line callee_12 }
+
+int test_12 (int first, int second, float third)
+{
+ return callee_12 (first, second, third); // { dg-error "invalid initialization of reference of " }
+ /* { dg-begin-multiline-output "" }
+ return callee_12 (first, second, third);
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "in passing argument 2 of " "" { target *-*-* } callee_12 }
+ /* { dg-begin-multiline-output "" }
+ extern int callee_12 (int one, s12 &second, float three);
+ ~~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+/* Incomplete type. */
+
+struct s13;
+
+extern int callee_13 (int one, s13 second, float three); // { dg-line callee_13 }
+
+int test_13 (int first, int second, float third)
+{
+ return callee_13 (first, second, third); // { dg-error "has incomplete type" }
+ /* { dg-begin-multiline-output "" }
+ return callee_13 (first, second, third);
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "declared here" "" { target *-*-* } callee_13 }
+ /* { dg-begin-multiline-output "" }
+ extern int callee_13 (int one, s13 second, float three);
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+
// TODO: template callsite
diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C b/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
new file mode 100644
index 0000000..465dc5a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
@@ -0,0 +1,43 @@
+// { dg-do compile }
+// { dg-options "-Wconversion-null -fdiagnostics-show-caret" }
+
+#include <stddef.h>
+
+void callee_1 (int, int, int) {} // { dg-message "declared here" }
+
+void caller_1 (void)
+{
+ callee_1 (0, NULL, 2); // { dg-warning "passing NULL to non-pointer argument 2 of" }
+ /* { dg-begin-multiline-output "" }
+ callee_1 (0, NULL, 2);
+ ^~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ void callee_1 (int, int, int) {}
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+void callee_2 (int, void *, int) {} // { dg-message "declared here" "" { target { ! c++11 } } }
+// { dg-message "initializing argument 2 of " "" { target c++11 } .-1 }
+
+void caller_2 (void)
+{
+ callee_2 (0, false, 2); // { dg-warning "converting 'false' to pointer type for argument 2 of " "" { target { ! c++11 } } }
+ // { dg-error "cannot convert" "" { target c++11 } .-1 }
+
+ /* { dg-begin-multiline-output "" }
+ callee_2 (0, false, 2);
+ ^~~~~
+ { dg-end-multiline-output "" { target { ! c++11 } } } */
+ /* { dg-begin-multiline-output "" }
+ callee_2 (0, false, 2);
+ ^~~~~
+ |
+ bool
+ { dg-end-multiline-output "" { target c++11 } } */
+ /* { dg-begin-multiline-output "" }
+ void callee_2 (int, void *, int) {}
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] C++: improvements to diagnostics using %P (more PR c++/85110)
2018-12-03 22:06 ` [PATCH v2] C++: improvements to diagnostics using " David Malcolm
@ 2018-12-06 0:49 ` Jason Merrill
2018-12-06 14:51 ` [PATCH] v3: " David Malcolm
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2018-12-06 0:49 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
On 12/3/18 5:54 PM, David Malcolm wrote:
> I was going to ping this patch:
> https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00875.html
> but it has bit-rotted somewhat, so here's a refreshed version of it.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> Thanks
> Dave
>
>
> Blurb from v1:
>
> This patch is based on grepping the C++ frontend for %P
> i.e. diagnostics that refer to a parameter number. It fixes up
> these diagnostics to highlight the pertinent param where appropriate
> (and possible), along with various other tweaks, as described in the
> ChangeLog.
>
> gcc/cp/ChangeLog:
> PR c++/85110
> * call.c (conversion_null_warnings): Try to use the location of
> the expression for the warnings. Add notes showing the parameter
> of the function decl, where available.
> (get_fndecl_argument_location): Gracefully reject
> non-FUNCTION_DECLs. For implicitly-declared functions, use the
> fndecl location rather than that of the param.
> (maybe_inform_about_fndecl_for_bogus_argument_init): New function.
> (convert_like_real): Use it in various places to avoid repetition.
> * cp-tree.h (maybe_inform_about_fndecl_for_bogus_argument_init):
> New declaration.
> * decl2.c: Include "gcc-rich-location.h".
> (check_default_args): Use the location of the parameter when
> complaining about parameters with missing default arguments in
> preference to that of the fndecl.
> Attempt to record the location of first parameter with a default
> argument and add it as a secondary range to such errors.
> * typeck.c (convert_arguments): When complaining about parameters
> with incomplete types, attempt to use the location of the
> argument. Where available, add a note showing the pertinent
> parameter in the fndecl.
> (convert_for_assignment): When complaining about bad conversions
> at function calls, use the location of the unstripped argument.
> (convert_for_initialization): When checking for bogus references,
> add an auto_diagnostic_group, and update the note to use the
> location of the pertinent parameter, rather than just the callee.
>
> gcc/testsuite/ChangeLog:
> PR c++/85110
> * g++.dg/diagnostic/missing-default-args.C: New test.
> * g++.dg/diagnostic/param-type-mismatch-3.C: New test.
> * g++.dg/diagnostic/param-type-mismatch.C: Add tests for invalid
> references and incomplete types.
> * g++.dg/warn/Wconversion-null-4.C: New test.
> ---
> gcc/cp/call.c | 88 ++++++++++++++--------
> gcc/cp/cp-tree.h | 1 +
> gcc/cp/decl2.c | 16 +++-
> gcc/cp/typeck.c | 22 ++++--
> .../g++.dg/diagnostic/missing-default-args.C | 53 +++++++++++++
> .../g++.dg/diagnostic/param-type-mismatch-3.C | 26 +++++++
> .../g++.dg/diagnostic/param-type-mismatch.C | 41 ++++++++++
> gcc/testsuite/g++.dg/warn/Wconversion-null-4.C | 43 +++++++++++
> 8 files changed, 251 insertions(+), 39 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
> create mode 100644 gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
> create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
>
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index ee099cc..cfc5641 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -6681,16 +6681,24 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
> if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
> && ARITHMETIC_TYPE_P (totype))
> {
> - location_t loc =
> - expansion_point_location_if_in_system_header (input_location);
> -
> if (fn)
> - warning_at (loc, OPT_Wconversion_null,
> - "passing NULL to non-pointer argument %P of %qD",
> - argnum, fn);
> + {
> + location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
> + loc = expansion_point_location_if_in_system_header (loc);
> + auto_diagnostic_group d;
> + if (warning_at (loc, OPT_Wconversion_null,
> + "passing NULL to non-pointer argument %P of %qD",
> + argnum, fn))
> + inform (get_fndecl_argument_location (fn, argnum),
> + " declared here");
> + }
> else
> - warning_at (loc, OPT_Wconversion_null,
> - "converting to non-pointer type %qT from NULL", totype);
> + {
> + location_t loc
> + = expansion_point_location_if_in_system_header (input_location);
> + warning_at (loc, OPT_Wconversion_null,
> + "converting to non-pointer type %qT from NULL", totype);
> + }
Why is 'loc' different between the branches?
> @@ -6698,9 +6706,15 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
> && TYPE_PTR_P (totype))
> {
> if (fn)
> - warning_at (input_location, OPT_Wconversion_null,
> - "converting %<false%> to pointer type for argument %P "
> - "of %qD", argnum, fn);
> + {
> + location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
> + auto_diagnostic_group d;
> + if (warning_at (loc, OPT_Wconversion_null,
> + "converting %<false%> to pointer type for argument "
> + "%P of %qD", argnum, fn))
> + inform (get_fndecl_argument_location (fn, argnum),
> + " declared here");
> + }
> else
> warning_at (input_location, OPT_Wconversion_null,
> "converting %<false%> to pointer type %qT", totype);
Same question.
> @@ -6740,6 +6754,15 @@ maybe_print_user_conv_context (conversion *convs)
> location_t
> get_fndecl_argument_location (tree fndecl, int argnum)
> {
> + /* Gracefully fail for e.g. TEMPLATE_DECL. */
> + if (TREE_CODE (fndecl) != FUNCTION_DECL)
> + return DECL_SOURCE_LOCATION (fndecl);
For a TEMPLATE_DECL we can use DECL_TEMPLATE_RESULT or STRIP_TEMPLATE to
get the FUNCTION_DECL. But I'm somewhat surprised we would get here
with a TEMPLATE_DECL rather than an instantiation.
> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
> index ffc0d0d..265826a 100644
> --- a/gcc/cp/decl2.c
> +++ b/gcc/cp/decl2.c
> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not see
> #include "intl.h"
> #include "c-family/c-ada-spec.h"
> #include "asan.h"
> +#include "gcc-rich-location.h"
>
> /* Id for dumping the raw trees. */
> int raw_dump_id;
> @@ -5179,14 +5180,25 @@ check_default_args (tree x)
> {
> tree arg = TYPE_ARG_TYPES (TREE_TYPE (x));
> bool saw_def = false;
> + location_t loc_of_first_default_arg = UNKNOWN_LOCATION;
> int i = 0 - (TREE_CODE (TREE_TYPE (x)) == METHOD_TYPE);
> for (; arg && arg != void_list_node; arg = TREE_CHAIN (arg), ++i)
> {
> if (TREE_PURPOSE (arg))
> - saw_def = true;
> + {
> + saw_def = true;
> + location_t loc = get_fndecl_argument_location (x, i);
> + if (loc != DECL_SOURCE_LOCATION (x))
> + loc_of_first_default_arg = loc;
> + }
> else if (saw_def && !PACK_EXPANSION_P (TREE_VALUE (arg)))
> {
> - error ("default argument missing for parameter %P of %q+#D", i, x);
> + location_t loc = get_fndecl_argument_location (x, i);
> + gcc_rich_location richloc (loc);
> + if (loc_of_first_default_arg != UNKNOWN_LOCATION)
> + richloc.add_range (loc_of_first_default_arg);
> + error_at (&richloc,
> + "default argument missing for parameter %P of %q#D", i, x);
If we're going to highlight the earlier parameter that has a default
argument, we should mention in the diagnostic why it's relevant.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] v3: C++: improvements to diagnostics using %P (more PR c++/85110)
2018-12-06 0:49 ` Jason Merrill
@ 2018-12-06 14:51 ` David Malcolm
2018-12-06 15:04 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: David Malcolm @ 2018-12-06 14:51 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches, David Malcolm
On Wed, 2018-12-05 at 19:49 -0500, Jason Merrill wrote:
> On 12/3/18 5:54 PM, David Malcolm wrote:
[...]
Thanks for the review. Here's a v3 of the patch; comments inline
below.
> > diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> > index ee099cc..cfc5641 100644
> > --- a/gcc/cp/call.c
> > +++ b/gcc/cp/call.c
> > @@ -6681,16 +6681,24 @@ conversion_null_warnings (tree totype, tree
> > expr, tree fn, int argnum)
> > if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
> > && ARITHMETIC_TYPE_P (totype))
> > {
> > - location_t loc =
> > - expansion_point_location_if_in_system_header
> > (input_location);
> > -
> > if (fn)
> > - warning_at (loc, OPT_Wconversion_null,
> > - "passing NULL to non-pointer argument %P of
> > %qD",
> > - argnum, fn);
> > + {
> > + location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
> > + loc = expansion_point_location_if_in_system_header
> > (loc);
> > + auto_diagnostic_group d;
> > + if (warning_at (loc, OPT_Wconversion_null,
> > + "passing NULL to non-pointer argument %P
> > of %qD",
> > + argnum, fn))
> > + inform (get_fndecl_argument_location (fn, argnum),
> > + " declared here");
> > + }
> > else
> > - warning_at (loc, OPT_Wconversion_null,
> > - "converting to non-pointer type %qT from
> > NULL", totype);
> > + {
> > + location_t loc
> > + = expansion_point_location_if_in_system_header
> > (input_location);
> > + warning_at (loc, OPT_Wconversion_null,
> > + "converting to non-pointer type %qT from
> > NULL", totype);
> > + }
>
> Why is 'loc' different between the branches?
Good catch; I've consolidated them in the v3 patch.
> > @@ -6698,9 +6706,15 @@ conversion_null_warnings (tree totype, tree
> > expr, tree fn, int argnum)
> > && TYPE_PTR_P (totype))
> > {
> > if (fn)
> > - warning_at (input_location, OPT_Wconversion_null,
> > - "converting %<false%> to pointer type for
> > argument %P "
> > - "of %qD", argnum, fn);
> > + {
> > + location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
> > + auto_diagnostic_group d;
> > + if (warning_at (loc, OPT_Wconversion_null,
> > + "converting %<false%> to pointer type
> > for argument "
> > + "%P of %qD", argnum, fn))
> > + inform (get_fndecl_argument_location (fn, argnum),
> > + " declared here");
> > + }
> > else
> > warning_at (input_location, OPT_Wconversion_null,
> > "converting %<false%> to pointer type %qT",
> > totype);
>
> Same question.
Likewise.
> > @@ -6740,6 +6754,15 @@ maybe_print_user_conv_context (conversion
> > *convs)
> > location_t
> > get_fndecl_argument_location (tree fndecl, int argnum)
> > {
> > + /* Gracefully fail for e.g. TEMPLATE_DECL. */
> > + if (TREE_CODE (fndecl) != FUNCTION_DECL)
> > + return DECL_SOURCE_LOCATION (fndecl);
>
> For a TEMPLATE_DECL we can use DECL_TEMPLATE_RESULT or STRIP_TEMPLATE
> to
> get the FUNCTION_DECL. But I'm somewhat surprised we would get here
> with a TEMPLATE_DECL rather than an instantiation.
FWIW I hit this for e.g. g++.dg/diagnostic/missing-default-args.C within
the new code in check_default_args, when reporting on an template
with a param with no-default-args following a param with default args.
In the v3 patch I've removed the check for FUNCTION_DECL, in favor of
a STRIP_TEMPLATE within check_default_args.
> > diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
> > index ffc0d0d..265826a 100644
> > --- a/gcc/cp/decl2.c
> > +++ b/gcc/cp/decl2.c
> > @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not
> > see
> > #include "intl.h"
> > #include "c-family/c-ada-spec.h"
> > #include "asan.h"
> > +#include "gcc-rich-location.h"
> >
> > /* Id for dumping the raw trees. */
> > int raw_dump_id;
> > @@ -5179,14 +5180,25 @@ check_default_args (tree x)
> > {
> > tree arg = TYPE_ARG_TYPES (TREE_TYPE (x));
> > bool saw_def = false;
> > + location_t loc_of_first_default_arg = UNKNOWN_LOCATION;
> > int i = 0 - (TREE_CODE (TREE_TYPE (x)) == METHOD_TYPE);
> > for (; arg && arg != void_list_node; arg = TREE_CHAIN (arg),
> > ++i)
> > {
> > if (TREE_PURPOSE (arg))
> > - saw_def = true;
> > + {
> > + saw_def = true;
> > + location_t loc = get_fndecl_argument_location (x, i);
> > + if (loc != DECL_SOURCE_LOCATION (x))
> > + loc_of_first_default_arg = loc;
> > + }
> > else if (saw_def && !PACK_EXPANSION_P (TREE_VALUE (arg)))
> > {
> > - error ("default argument missing for parameter %P of
> > %q+#D", i, x);
> > + location_t loc = get_fndecl_argument_location (x, i);
> > + gcc_rich_location richloc (loc);
> > + if (loc_of_first_default_arg != UNKNOWN_LOCATION)
> > + richloc.add_range (loc_of_first_default_arg);
> > + error_at (&richloc,
> > + "default argument missing for parameter %P of
> > %q#D", i, x);
>
> If we're going to highlight the earlier parameter that has a default
> argument, we should mention in the diagnostic why it's relevant.
Yeah, the secondary location wasn't very self-explanatory.
In the v3 patch I've changed it to emit a note (once per fndecl)
highlighting the first param that has a default argument.
Given e.g.:
void test (int a, int b = 42, int c=1776, int d, int e);
With trunk we have:
demo.cc:5:6: error: default argument missing for parameter 4 of
'void test(int, int, int, int, int)'
5 | void test (int a, int b = 42, int c=1776, int d, int e);
| ^~~~
demo.cc:5:6: error: default argument missing for parameter 5 of
'void test(int, int, int, int, int)'
With the v3 patch it emits:
demo.cc:5:47: error: default argument missing for parameter 4 of
'void test(int, int, int, int, int)'
5 | void test (int a, int b = 42, int c=1776, int d, int e);
| ~~~~^
demo.cc:5:23: note: ...following parameter 2 which has a default
argument
5 | void test (int a, int b = 42, int c=1776, int d, int e);
| ~~~~^~~~~~
demo.cc:5:54: error: default argument missing for parameter 5 of
'void test(int, int, int, int, int)'
5 | void test (int a, int b = 42, int c=1776, int d, int e);
| ~~~~^
thus highlighting which specific params we mean, and giving a
hint to the user about why we're complaining. Maybe the
wording could use some improvement (the above is about as terse
as I felt I could make it without it becoming hard to read).
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
Changes in v3 relative to v2:
gcc/cp/ChangeLog:
* call.c (conversion_null_warnings): Consolidate location
handling.
(get_fndecl_argument_location): Remove rejection of
non-FUNCTION_DECL in favor of STRIP_TEMPLATE within
check_default_args.
* decl2.c: Remove added include "gcc-rich-location.h".
(check_default_args): Put all diagnostics for the fndecl into a
diagnostic group. Introduce "fndecl" via STRIP_TEMPLATE. Add a
note about the first parameter with a default argument, rather
than a secondary range, only emitting for the first diagnostic
within the group.
gcc/testsuite/ChangeLog:
* g++.dg/diagnostic/missing-default-args.C: Update for changes to
check_default_args.
Blurb from v1:
This patch is based on grepping the C++ frontend for %P
i.e. diagnostics that refer to a parameter number. It fixes up
these diagnostics to highlight the pertinent param where appropriate
(and possible), along with various other tweaks, as described in the
ChangeLog.
Consolidated ChangeLog for v3:
gcc/cp/ChangeLog:
PR c++/85110
* call.c (conversion_null_warnings): Try to use the location of
the expression for the warnings. Add notes showing the parameter
of the function decl, where available.
(get_fndecl_argument_location): For implicitly-declared functions,
use the fndecl location rather than that of the param.
(maybe_inform_about_fndecl_for_bogus_argument_init): New function.
(convert_like_real): Use it in various places to avoid repetition.
(complain_about_bad_argument): Likewise.
* cp-tree.h (maybe_inform_about_fndecl_for_bogus_argument_init):
New declaration.
* decl2.c (check_default_args): Put all diagnostics for the fndecl
into a diagnostic group. Use the location of the parameter when
complaining about parameters with missing default arguments in
preference to that of the fndecl. Attempt to record the location
of the first parameter with a default argument and emit a note
for the first parameter that's missing one.
* typeck.c (convert_arguments): When complaining about parameters
with incomplete types, attempt to use the location of the
argument. Where available, add a note showing the pertinent
parameter in the fndecl.
(convert_for_assignment): When complaining about bad conversions
at function calls, use the location of the unstripped argument.
(convert_for_initialization): When checking for bogus references,
add an auto_diagnostic_group, and update the note to use the
location of the pertinent parameter, rather than just the callee.
gcc/testsuite/ChangeLog:
PR c++/85110
* g++.dg/diagnostic/missing-default-args.C: New test.
* g++.dg/diagnostic/param-type-mismatch-3.C: New test.
* g++.dg/diagnostic/param-type-mismatch.C: Add tests for invalid
references and incomplete types.
* g++.dg/warn/Wconversion-null-4.C: New test.
---
gcc/cp/call.c | 78 +++++++++++++---------
gcc/cp/cp-tree.h | 1 +
gcc/cp/decl2.c | 27 +++++++-
gcc/cp/typeck.c | 22 ++++--
.../g++.dg/diagnostic/missing-default-args.C | 73 ++++++++++++++++++++
.../g++.dg/diagnostic/param-type-mismatch-3.C | 26 ++++++++
.../g++.dg/diagnostic/param-type-mismatch.C | 41 ++++++++++++
gcc/testsuite/g++.dg/warn/Wconversion-null-4.C | 43 ++++++++++++
8 files changed, 273 insertions(+), 38 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ee099cc..148e2d5 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6681,13 +6681,17 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
&& ARITHMETIC_TYPE_P (totype))
{
- location_t loc =
- expansion_point_location_if_in_system_header (input_location);
-
+ location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
+ loc = expansion_point_location_if_in_system_header (loc);
if (fn)
- warning_at (loc, OPT_Wconversion_null,
- "passing NULL to non-pointer argument %P of %qD",
- argnum, fn);
+ {
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wconversion_null,
+ "passing NULL to non-pointer argument %P of %qD",
+ argnum, fn))
+ inform (get_fndecl_argument_location (fn, argnum),
+ " declared here");
+ }
else
warning_at (loc, OPT_Wconversion_null,
"converting to non-pointer type %qT from NULL", totype);
@@ -6697,12 +6701,18 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
else if (TREE_CODE (TREE_TYPE (expr)) == BOOLEAN_TYPE
&& TYPE_PTR_P (totype))
{
+ location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
if (fn)
- warning_at (input_location, OPT_Wconversion_null,
- "converting %<false%> to pointer type for argument %P "
- "of %qD", argnum, fn);
+ {
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wconversion_null,
+ "converting %<false%> to pointer type for argument "
+ "%P of %qD", argnum, fn))
+ inform (get_fndecl_argument_location (fn, argnum),
+ " declared here");
+ }
else
- warning_at (input_location, OPT_Wconversion_null,
+ warning_at (loc, OPT_Wconversion_null,
"converting %<false%> to pointer type %qT", totype);
}
/* Handle zero as null pointer warnings for cases other
@@ -6740,6 +6750,11 @@ maybe_print_user_conv_context (conversion *convs)
location_t
get_fndecl_argument_location (tree fndecl, int argnum)
{
+ /* The locations of implicitly-declared functions are likely to be
+ more meaningful than those of their parameters. */
+ if (DECL_ARTIFICIAL (fndecl))
+ return DECL_SOURCE_LOCATION (fndecl);
+
int i;
tree param;
@@ -6757,6 +6772,18 @@ get_fndecl_argument_location (tree fndecl, int argnum)
return DECL_SOURCE_LOCATION (param);
}
+/* If FNDECL is non-NULL, issue a note highlighting ARGNUM
+ within its declaration (or the fndecl itself if something went
+ wrong). */
+
+void
+maybe_inform_about_fndecl_for_bogus_argument_init (tree fn, int argnum)
+{
+ if (fn)
+ inform (get_fndecl_argument_location (fn, argnum),
+ " initializing argument %P of %qD", argnum, fn);
+}
+
/* Perform the conversions in CONVS on the expression EXPR. FN and
ARGNUM are used for diagnostics. ARGNUM is zero based, -1
indicates the `this' argument of a method. INNER is nonzero when
@@ -6834,9 +6861,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
complain);
else
expr = cp_convert (totype, expr, complain);
- if (complained && fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ if (complained)
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
return expr;
}
else if (t->kind == ck_user || !t->bad_p)
@@ -6863,9 +6889,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
"invalid conversion from %qH to %qI",
TREE_TYPE (expr), totype);
}
- if (complained && fn)
- inform (get_fndecl_argument_location (fn, argnum),
- " initializing argument %P of %qD", argnum, fn);
+ if (complained)
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
return cp_convert (totype, expr, complain);
}
@@ -6987,9 +7012,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
? LOOKUP_IMPLICIT : LOOKUP_NORMAL);
build_user_type_conversion (totype, convs->u.expr, flags, complain);
gcc_assert (seen_error ());
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return error_mark_node;
@@ -7083,9 +7106,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
{
auto_diagnostic_group d;
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return error_mark_node;
}
@@ -7136,9 +7157,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
{
auto_diagnostic_group d;
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
return build_cplus_new (totype, expr, complain);
@@ -7165,9 +7184,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
else
gcc_unreachable ();
maybe_print_user_conv_context (convs);
- if (fn)
- inform (DECL_SOURCE_LOCATION (fn),
- " initializing argument %P of %qD", argnum, fn);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
+
return error_mark_node;
}
@@ -9299,8 +9317,8 @@ complain_about_bad_argument (location_t arg_loc,
error_at (&richloc,
"cannot convert %qH to %qI",
from_type, to_type);
- inform (get_fndecl_argument_location (fndecl, parmnum),
- " initializing argument %P of %qD", parmnum, fndecl);
+ maybe_inform_about_fndecl_for_bogus_argument_init (fndecl,
+ parmnum);
}
/* Subroutine of build_new_method_call_1, for where there are no viable
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 111a123..c1f9931 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6145,6 +6145,7 @@ extern location_t get_fndecl_argument_location (tree, int);
extern void complain_about_bad_argument (location_t arg_loc,
tree from_type, tree to_type,
tree fndecl, int parmnum);
+extern void maybe_inform_about_fndecl_for_bogus_argument_init (tree, int);
/* A class for recording information about access failures (e.g. private
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index ffc0d0d..a717290 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -5179,14 +5179,37 @@ check_default_args (tree x)
{
tree arg = TYPE_ARG_TYPES (TREE_TYPE (x));
bool saw_def = false;
+ bool noted_first_def = false;
+ int idx_of_first_default_arg = 0;
+ location_t loc_of_first_default_arg = UNKNOWN_LOCATION;
int i = 0 - (TREE_CODE (TREE_TYPE (x)) == METHOD_TYPE);
+ tree fndecl = STRIP_TEMPLATE (x);
+ auto_diagnostic_group d;
for (; arg && arg != void_list_node; arg = TREE_CHAIN (arg), ++i)
{
if (TREE_PURPOSE (arg))
- saw_def = true;
+ {
+ if (!saw_def)
+ {
+ saw_def = true;
+ idx_of_first_default_arg = i;
+ location_t loc = get_fndecl_argument_location (fndecl, i);
+ if (loc != DECL_SOURCE_LOCATION (x))
+ loc_of_first_default_arg = loc;
+ }
+ }
else if (saw_def && !PACK_EXPANSION_P (TREE_VALUE (arg)))
{
- error ("default argument missing for parameter %P of %q+#D", i, x);
+ error_at (get_fndecl_argument_location (fndecl, i),
+ "default argument missing for parameter %P of %q#D", i, x);
+ if (loc_of_first_default_arg != UNKNOWN_LOCATION
+ && !noted_first_def)
+ {
+ inform (loc_of_first_default_arg,
+ "...following parameter %P which has a default argument",
+ idx_of_first_default_arg);
+ noted_first_def = true;
+ }
TREE_PURPOSE (arg) = error_mark_node;
}
}
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index f45c06e..c3807de 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4007,11 +4007,19 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl,
{
if (complain & tf_error)
{
+ location_t loc = EXPR_LOC_OR_LOC (val, input_location);
if (fndecl)
- error ("parameter %P of %qD has incomplete type %qT",
- i, fndecl, type);
+ {
+ auto_diagnostic_group d;
+ error_at (loc,
+ "parameter %P of %qD has incomplete type %qT",
+ i, fndecl, type);
+ inform (get_fndecl_argument_location (fndecl, i),
+ " declared here");
+ }
else
- error ("parameter %P has incomplete type %qT", i, type);
+ error_at (loc, "parameter %P has incomplete type %qT", i,
+ type);
}
parmval = error_mark_node;
}
@@ -8795,6 +8803,8 @@ convert_for_assignment (tree type, tree rhs,
tree rhstype;
enum tree_code coder;
+ location_t rhs_loc = EXPR_LOC_OR_LOC (rhs, input_location);
+
/* Strip NON_LVALUE_EXPRs since we aren't using as an lvalue. */
if (TREE_CODE (rhs) == NON_LVALUE_EXPR)
rhs = TREE_OPERAND (rhs, 0);
@@ -8901,7 +8911,7 @@ convert_for_assignment (tree type, tree rhs,
parmnum, complain, flags);
}
else if (fndecl)
- complain_about_bad_argument (cp_expr_location (rhs),
+ complain_about_bad_argument (rhs_loc,
rhstype, type,
fndecl, parmnum);
else
@@ -9068,6 +9078,7 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
if (codel == REFERENCE_TYPE)
{
+ auto_diagnostic_group d;
/* This should eventually happen in convert_arguments. */
int savew = 0, savee = 0;
@@ -9077,9 +9088,8 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
if (fndecl
&& (warningcount + werrorcount > savew || errorcount > savee))
- inform (DECL_SOURCE_LOCATION (fndecl),
+ inform (get_fndecl_argument_location (fndecl, parmnum),
"in passing argument %P of %qD", parmnum, fndecl);
-
return rhs;
}
diff --git a/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C b/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
new file mode 100644
index 0000000..2e9401d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
@@ -0,0 +1,73 @@
+// { dg-options "-fdiagnostics-show-caret" }
+
+/* Function. */
+
+void test_1 (int a, int b = 42, int c, int d); // { dg-line test_1 }
+
+// { dg-error "default argument missing for parameter 3 of " "" { target *-*-* } test_1 }
+/* { dg-begin-multiline-output "" }
+ void test_1 (int a, int b = 42, int c, int d);
+ ~~~~^
+ { dg-end-multiline-output "" } */
+// { dg-message "...following parameter 2 which has a default argument" "" { target *-*-* } test_1 }
+/* { dg-begin-multiline-output "" }
+ void test_1 (int a, int b = 42, int c, int d);
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+// { dg-error "default argument missing for parameter 4 of " "" { target *-*-* } test_1 }
+/* { dg-begin-multiline-output "" }
+ void test_1 (int a, int b = 42, int c, int d);
+ ~~~~^
+ { dg-end-multiline-output "" } */
+
+
+/* Non-static member fn. */
+
+struct test_2
+{
+ void member_2 (int a, int b = 42, int c); // { dg-line test_2 }
+};
+// { dg-error "default argument missing for parameter 3 of " "" { target *-*-* } test_2 }
+/* { dg-begin-multiline-output "" }
+ void member_2 (int a, int b = 42, int c);
+ ~~~~^
+ { dg-end-multiline-output "" } */
+// { dg-message "...following parameter 2 which has a default argument" "" { target *-*-* } test_2 }
+/* { dg-begin-multiline-output "" }
+ void member_2 (int a, int b = 42, int c);
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+
+
+/* Static member fn. */
+
+struct test_3
+{
+ static void member_3 (int a, int b = 42, int c); // { dg-line test_3 }
+};
+// { dg-error "default argument missing for parameter 3 of " "" { target *-*-* } test_3 }
+/* { dg-begin-multiline-output "" }
+ static void member_3 (int a, int b = 42, int c);
+ ~~~~^
+ { dg-end-multiline-output "" } */
+// { dg-message "...following parameter 2 which has a default argument" "" { target *-*-* } test_3 }
+/* { dg-begin-multiline-output "" }
+ static void member_3 (int a, int b = 42, int c);
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+
+
+/* Template. */
+
+template <typename Type>
+void test_4 (int a, int b = 42, int c); // { dg-line test_4 }
+// { dg-error "default argument missing for parameter 3 of " "" { target *-*-* } test_4 }
+/* { dg-begin-multiline-output "" }
+ void test_4 (int a, int b = 42, int c);
+ ~~~~^
+ { dg-end-multiline-output "" } */
+// { dg-message "...following parameter 2 which has a default argument" "" { target *-*-* } test_4 }
+/* { dg-begin-multiline-output "" }
+ void test_4 (int a, int b = 42, int c);
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
new file mode 100644
index 0000000..3ffbbd8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
@@ -0,0 +1,26 @@
+// { dg-do compile { target c++14 } }
+// { dg-options "-fdiagnostics-show-caret" }
+
+void f2(int, char (*)(int), int) { } // { dg-line f2 }
+
+void test_1 ()
+{
+ auto glambda = [](auto a) { return a; }; // { dg-line candidate }
+ int (*fp)(int) = glambda;
+ f2(1, glambda, 3); // { dg-error "invalid user-defined conversion" }
+ /* { dg-begin-multiline-output "" }
+ f2(1, glambda, 3);
+ ^~~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "candidate is: " "" { target *-*-* } candidate }
+ /* { dg-begin-multiline-output "" }
+ auto glambda = [](auto a) { return a; };
+ ^
+ { dg-end-multiline-output "" } */
+ // { dg-message "no known conversion from " "" { target *-*-* } candidate }
+ // { dg-message "initializing argument 2 of " "" { target *-*-* } f2 }
+ /* { dg-begin-multiline-output "" }
+ void f2(int, char (*)(int), int) { }
+ ^~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
index cb5c360..54ffb06 100644
--- a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
+++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
@@ -224,4 +224,45 @@ int test_11 (int first, int second, float third)
{ dg-end-multiline-output "" } */
}
+/* Bad reference. */
+
+struct s12;
+
+extern int callee_12 (int one, s12 &second, float three); // { dg-line callee_12 }
+
+int test_12 (int first, int second, float third)
+{
+ return callee_12 (first, second, third); // { dg-error "invalid initialization of reference of " }
+ /* { dg-begin-multiline-output "" }
+ return callee_12 (first, second, third);
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "in passing argument 2 of " "" { target *-*-* } callee_12 }
+ /* { dg-begin-multiline-output "" }
+ extern int callee_12 (int one, s12 &second, float three);
+ ~~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+/* Incomplete type. */
+
+struct s13;
+
+extern int callee_13 (int one, s13 second, float three); // { dg-line callee_13 }
+
+int test_13 (int first, int second, float third)
+{
+ return callee_13 (first, second, third); // { dg-error "has incomplete type" }
+ /* { dg-begin-multiline-output "" }
+ return callee_13 (first, second, third);
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+ // { dg-message "declared here" "" { target *-*-* } callee_13 }
+ /* { dg-begin-multiline-output "" }
+ extern int callee_13 (int one, s13 second, float three);
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+
// TODO: template callsite
diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C b/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
new file mode 100644
index 0000000..465dc5a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wconversion-null-4.C
@@ -0,0 +1,43 @@
+// { dg-do compile }
+// { dg-options "-Wconversion-null -fdiagnostics-show-caret" }
+
+#include <stddef.h>
+
+void callee_1 (int, int, int) {} // { dg-message "declared here" }
+
+void caller_1 (void)
+{
+ callee_1 (0, NULL, 2); // { dg-warning "passing NULL to non-pointer argument 2 of" }
+ /* { dg-begin-multiline-output "" }
+ callee_1 (0, NULL, 2);
+ ^~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ void callee_1 (int, int, int) {}
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+void callee_2 (int, void *, int) {} // { dg-message "declared here" "" { target { ! c++11 } } }
+// { dg-message "initializing argument 2 of " "" { target c++11 } .-1 }
+
+void caller_2 (void)
+{
+ callee_2 (0, false, 2); // { dg-warning "converting 'false' to pointer type for argument 2 of " "" { target { ! c++11 } } }
+ // { dg-error "cannot convert" "" { target c++11 } .-1 }
+
+ /* { dg-begin-multiline-output "" }
+ callee_2 (0, false, 2);
+ ^~~~~
+ { dg-end-multiline-output "" { target { ! c++11 } } } */
+ /* { dg-begin-multiline-output "" }
+ callee_2 (0, false, 2);
+ ^~~~~
+ |
+ bool
+ { dg-end-multiline-output "" { target c++11 } } */
+ /* { dg-begin-multiline-output "" }
+ void callee_2 (int, void *, int) {}
+ ^~~~~~
+ { dg-end-multiline-output "" } */
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] v3: C++: improvements to diagnostics using %P (more PR c++/85110)
2018-12-06 14:51 ` [PATCH] v3: " David Malcolm
@ 2018-12-06 15:04 ` Jason Merrill
0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2018-12-06 15:04 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
On 12/6/18 10:38 AM, David Malcolm wrote:
> On Wed, 2018-12-05 at 19:49 -0500, Jason Merrill wrote:
>> On 12/3/18 5:54 PM, David Malcolm wrote:
>
> [...]
>
> Thanks for the review. Here's a v3 of the patch; comments inline
> below.
>
>>> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
>>> index ee099cc..cfc5641 100644
>>> --- a/gcc/cp/call.c
>>> +++ b/gcc/cp/call.c
>>> @@ -6681,16 +6681,24 @@ conversion_null_warnings (tree totype, tree
>>> expr, tree fn, int argnum)
>>> if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
>>> && ARITHMETIC_TYPE_P (totype))
>>> {
>>> - location_t loc =
>>> - expansion_point_location_if_in_system_header
>>> (input_location);
>>> -
>>> if (fn)
>>> - warning_at (loc, OPT_Wconversion_null,
>>> - "passing NULL to non-pointer argument %P of
>>> %qD",
>>> - argnum, fn);
>>> + {
>>> + location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
>>> + loc = expansion_point_location_if_in_system_header
>>> (loc);
>>> + auto_diagnostic_group d;
>>> + if (warning_at (loc, OPT_Wconversion_null,
>>> + "passing NULL to non-pointer argument %P
>>> of %qD",
>>> + argnum, fn))
>>> + inform (get_fndecl_argument_location (fn, argnum),
>>> + " declared here");
>>> + }
>>> else
>>> - warning_at (loc, OPT_Wconversion_null,
>>> - "converting to non-pointer type %qT from
>>> NULL", totype);
>>> + {
>>> + location_t loc
>>> + = expansion_point_location_if_in_system_header
>>> (input_location);
>>> + warning_at (loc, OPT_Wconversion_null,
>>> + "converting to non-pointer type %qT from
>>> NULL", totype);
>>> + }
>>
>> Why is 'loc' different between the branches?
>
> Good catch; I've consolidated them in the v3 patch.
>
>>> @@ -6698,9 +6706,15 @@ conversion_null_warnings (tree totype, tree
>>> expr, tree fn, int argnum)
>>> && TYPE_PTR_P (totype))
>>> {
>>> if (fn)
>>> - warning_at (input_location, OPT_Wconversion_null,
>>> - "converting %<false%> to pointer type for
>>> argument %P "
>>> - "of %qD", argnum, fn);
>>> + {
>>> + location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
>>> + auto_diagnostic_group d;
>>> + if (warning_at (loc, OPT_Wconversion_null,
>>> + "converting %<false%> to pointer type
>>> for argument "
>>> + "%P of %qD", argnum, fn))
>>> + inform (get_fndecl_argument_location (fn, argnum),
>>> + " declared here");
>>> + }
>>> else
>>> warning_at (input_location, OPT_Wconversion_null,
>>> "converting %<false%> to pointer type %qT",
>>> totype);
>>
>> Same question.
>
> Likewise.
>
>>> @@ -6740,6 +6754,15 @@ maybe_print_user_conv_context (conversion
>>> *convs)
>>> location_t
>>> get_fndecl_argument_location (tree fndecl, int argnum)
>>> {
>>> + /* Gracefully fail for e.g. TEMPLATE_DECL. */
>>> + if (TREE_CODE (fndecl) != FUNCTION_DECL)
>>> + return DECL_SOURCE_LOCATION (fndecl);
>>
>> For a TEMPLATE_DECL we can use DECL_TEMPLATE_RESULT or STRIP_TEMPLATE
>> to
>> get the FUNCTION_DECL. But I'm somewhat surprised we would get here
>> with a TEMPLATE_DECL rather than an instantiation.
>
> FWIW I hit this for e.g. g++.dg/diagnostic/missing-default-args.C within
> the new code in check_default_args, when reporting on an template
> with a param with no-default-args following a param with default args.
>
> In the v3 patch I've removed the check for FUNCTION_DECL, in favor of
> a STRIP_TEMPLATE within check_default_args.
>
>>> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
>>> index ffc0d0d..265826a 100644
>>> --- a/gcc/cp/decl2.c
>>> +++ b/gcc/cp/decl2.c
>>> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not
>>> see
>>> #include "intl.h"
>>> #include "c-family/c-ada-spec.h"
>>> #include "asan.h"
>>> +#include "gcc-rich-location.h"
>>>
>>> /* Id for dumping the raw trees. */
>>> int raw_dump_id;
>>> @@ -5179,14 +5180,25 @@ check_default_args (tree x)
>>> {
>>> tree arg = TYPE_ARG_TYPES (TREE_TYPE (x));
>>> bool saw_def = false;
>>> + location_t loc_of_first_default_arg = UNKNOWN_LOCATION;
>>> int i = 0 - (TREE_CODE (TREE_TYPE (x)) == METHOD_TYPE);
>>> for (; arg && arg != void_list_node; arg = TREE_CHAIN (arg),
>>> ++i)
>>> {
>>> if (TREE_PURPOSE (arg))
>>> - saw_def = true;
>>> + {
>>> + saw_def = true;
>>> + location_t loc = get_fndecl_argument_location (x, i);
>>> + if (loc != DECL_SOURCE_LOCATION (x))
>>> + loc_of_first_default_arg = loc;
>>> + }
>>> else if (saw_def && !PACK_EXPANSION_P (TREE_VALUE (arg)))
>>> {
>>> - error ("default argument missing for parameter %P of
>>> %q+#D", i, x);
>>> + location_t loc = get_fndecl_argument_location (x, i);
>>> + gcc_rich_location richloc (loc);
>>> + if (loc_of_first_default_arg != UNKNOWN_LOCATION)
>>> + richloc.add_range (loc_of_first_default_arg);
>>> + error_at (&richloc,
>>> + "default argument missing for parameter %P of
>>> %q#D", i, x);
>>
>> If we're going to highlight the earlier parameter that has a default
>> argument, we should mention in the diagnostic why it's relevant.
>
> Yeah, the secondary location wasn't very self-explanatory.
> In the v3 patch I've changed it to emit a note (once per fndecl)
> highlighting the first param that has a default argument.
>
> Given e.g.:
> void test (int a, int b = 42, int c=1776, int d, int e);
>
> With trunk we have:
>
> demo.cc:5:6: error: default argument missing for parameter 4 of
> 'void test(int, int, int, int, int)'
> 5 | void test (int a, int b = 42, int c=1776, int d, int e);
> | ^~~~
> demo.cc:5:6: error: default argument missing for parameter 5 of
> 'void test(int, int, int, int, int)'
>
> With the v3 patch it emits:
>
> demo.cc:5:47: error: default argument missing for parameter 4 of
> 'void test(int, int, int, int, int)'
> 5 | void test (int a, int b = 42, int c=1776, int d, int e);
> | ~~~~^
> demo.cc:5:23: note: ...following parameter 2 which has a default
> argument
> 5 | void test (int a, int b = 42, int c=1776, int d, int e);
> | ~~~~^~~~~~
> demo.cc:5:54: error: default argument missing for parameter 5 of
> 'void test(int, int, int, int, int)'
> 5 | void test (int a, int b = 42, int c=1776, int d, int e);
> | ~~~~^
>
> thus highlighting which specific params we mean, and giving a
> hint to the user about why we're complaining. Maybe the
> wording could use some improvement (the above is about as terse
> as I felt I could make it without it becoming hard to read).
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
OK.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-06 15:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-11 22:59 [PATCH] C++: improvements to diagnostics that use %P (more PR c++/85110) David Malcolm
2018-12-03 22:06 ` [PATCH v2] C++: improvements to diagnostics using " David Malcolm
2018-12-06 0:49 ` Jason Merrill
2018-12-06 14:51 ` [PATCH] v3: " David Malcolm
2018-12-06 15:04 ` Jason Merrill
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).