From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 102693 invoked by alias); 3 Dec 2018 22:06:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 101570 invoked by uid 89); 3 Dec 2018 22:06:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=sk:build_n, asanh, showing, UD:asan.h X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Dec 2018 22:06:39 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5DA3C2D7E5 for ; Mon, 3 Dec 2018 22:06:38 +0000 (UTC) Received: from c64.redhat.com (ovpn-112-11.phx2.redhat.com [10.3.112.11]) by smtp.corp.redhat.com (Postfix) with ESMTP id 007F619745; Mon, 3 Dec 2018 22:06:36 +0000 (UTC) From: David Malcolm To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, David Malcolm Subject: [PATCH v2] C++: improvements to diagnostics using %P (more PR c++/85110) Date: Mon, 03 Dec 2018 22:06:00 -0000 Message-Id: <1543877659-41220-1-git-send-email-dmalcolm@redhat.com> In-Reply-To: <1541979984-39424-1-git-send-email-dmalcolm@redhat.com> References: <1541979984-39424-1-git-send-email-dmalcolm@redhat.com> X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00135.txt.bz2 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 % 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 % 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 % 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 **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 +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 + +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