From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115569 invoked by alias); 6 Dec 2018 00:49:40 -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 115540 invoked by uid 89); 6 Dec 2018 00:49:39 -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,RCVD_IN_DNSWL_NONE,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=highlight, UD:call.c, blurb, call.c X-HELO: mail-qt1-f177.google.com Received: from mail-qt1-f177.google.com (HELO mail-qt1-f177.google.com) (209.85.160.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 06 Dec 2018 00:49:29 +0000 Received: by mail-qt1-f177.google.com with SMTP id d19so24534985qtq.9 for ; Wed, 05 Dec 2018 16:49:29 -0800 (PST) Return-Path: Received: from [192.168.1.132] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id a79sm12490818qkc.9.2018.12.05.16.49.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Dec 2018 16:49:26 -0800 (PST) Subject: Re: [PATCH v2] C++: improvements to diagnostics using %P (more PR c++/85110) To: David Malcolm Cc: gcc-patches@gcc.gnu.org References: <1541979984-39424-1-git-send-email-dmalcolm@redhat.com> <1543877659-41220-1-git-send-email-dmalcolm@redhat.com> From: Jason Merrill Message-ID: <9b627d17-0203-3b5a-7257-790b8243156c@redhat.com> Date: Thu, 06 Dec 2018 00:49:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <1543877659-41220-1-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00329.txt.bz2 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 % 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); 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