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
