[PATCH] C++: underline parameters in mismatching function calls
Martin Sebor
msebor@gmail.com
Thu Sep 21 16:52:00 GMT 2017
On 09/20/2017 01:52 PM, David Malcolm wrote:
> When we have a type mismatch in a C++ function call, e.g.
>
> extern int callee (int one, const char *two, float three);
>
> int caller (int first, int second, float third)
> {
> return callee (first, second, third);
> }
>
> we currently emit something like this:
>
> test.c: In function 'int caller(int, int, float)':
> test.c:5:38: error: invalid conversion from 'int' to 'const char*'
> [-fpermissive]
> return callee (first, second, third);
> ^
> test.c:1:12: note: initializing argument 2 of 'int callee(int,
> const char*, float)'
> extern int callee (int one, const char *two, float three);
> ^~~~~~
>
> whereas underlining the mismatching things would make the messages
> easier to comprehend:
>
> test.c: In function 'int caller(int, int, float)':
> test.c:5:38: error: invalid conversion from 'int' to 'const char*'
> [-fpermissive]
> return callee (first, second, third);
> ^~~~~~
> test.c:1:12: note: initializing argument 2 of 'int callee(int,
> const char*, float)'
> extern int callee (int one, const char *two, float three);
> ^~~~~~~~~~~~~~~
>
> The two parts of this require two separate fixes; this patch is for
> the second part: the "note" describing the parameter of the callee.
The underlining is definite improvement! When there are multiple
mismatches, GCC emits multiple errors and notes, one for each
mismatch. The errors are different but the notes are the same.
AFAICS, with your patch, the notes will also be different because
of the underlining, but I wonder if it would make sense to emit
just one note that underlines all the mismatched arguments. (Clang
emits just a single error for the first mismatch.)
Martin
>
> In my Cauldron talk I proposed the "BLT" patch for this:
> https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01458.html
> but both Jason and Nathan seemed unhappy with the overall BLT
> approach: they didn't want a separate representation for the locations,
> but to instead reuse and extend the existing represenatation
> (I hope I'm correctly characterizing your thoughts here; sorry if this
> is wrong).
>
> Jason pointed out that a FUNCTION_DECL has a DECL_ARGUMENTS chain
> containing a PARAM_DECL for each parameter, and that these have
> locations, and recommended using this instead, fixing the locations
> as necessary (currently trunk just uses the location of the last token
> consumed before grokdeclarator is called for each such PARAM_DECL).
>
> This patch implements this approach for the C++ FE:
> (a) it updates the locations of the params to cover the range of all
> of their tokens, putting the caret on the first character of the
> param name (if present), otherwise at the start of the first token.
> (b) using the param locations, rather than the fndecl location for
> the "note" at mismatches.
>
> This takes the note in the example above from:
>
> test.c:1:12: note: initializing argument 2 of 'int callee(int,
> const char*, float)'
> extern int callee (int one, const char *two, float three);
> ^~~~~~
>
> to:
>
> test.c:1:41: note: initializing argument 2 of 'int callee(int,
> const char*, float)'
> extern int callee (int one, const char *two, float three);
> ~~~~~~~~~~~~^~~
>
> making it more obvious to the user where the problem is and how to
> fix it.
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> I'm working on something similar for the C frontend.
>
> gcc/cp/ChangeLog:
> * call.c (get_fndecl_argument_location): New function.
> (convert_like_real): Use it when complaining about argument type
> mismatches.
> * cp-tree.h (struct cp_parameter_declarator): Add "loc" field.
> * parser.c (make_parameter_declarator): Add "loc" param and use
> it to initialize the new field.
> (cp_parser_translation_unit): Add UNKNOWN_LOCATION for "loc" of
> the "no_parameters" parameter.
> (cp_parser_parameter_declaration_list): Set the location of the
> result of grokdeclarator to be the parameter's loc, assuming no
> errors.
> (cp_parser_parameter_declaration): Generate a location for the
> parameter and pass to make_parameter_declarator.
>
> gcc/testsuite/ChangeLog:
> * g++.dg/diagnostic/param-type-mismatch.C: Update expected results
> to reflect highlighting of parameters; add test coverage for
> callback parameters.
> ---
> gcc/cp/call.c | 31 +++++++++++-
> gcc/cp/cp-tree.h | 2 +
> gcc/cp/parser.c | 35 ++++++++++++-
> .../g++.dg/diagnostic/param-type-mismatch.C | 57 +++++++++++++++++-----
> 4 files changed, 111 insertions(+), 14 deletions(-)
>
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 88af0d3..66d8e3f 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -6579,6 +6579,35 @@ maybe_print_user_conv_context (conversion *convs)
> }
> }
>
> +/* Locate the parameter with the given index within FNDECL.
> + ARGNUM is zero based, -1 indicates the `this' argument of a method.
> + Return the location of the FNDECL itself if there are problems. */
> +
> +static location_t
> +get_fndecl_argument_location (tree fndecl, int argnum)
> +{
> + int i;
> + tree param;
> +
> + /* If we have a method, then DECL_ARGUMENTS begins with "this";
> + increment ARGNUM to skip it. */
> + if (TREE_CODE (TREE_TYPE (fndecl)) == METHOD_TYPE)
> + argnum++;
> +
> + /* Locate param by index within DECL_ARGUMENTS (fndecl). */
> + for (i = 0, param = DECL_ARGUMENTS (fndecl);
> + i < argnum && param;
> + i++, param = TREE_CHAIN (param))
> + ;
> +
> + /* If something went wrong (e.g. if we have a builtin and thus no arguments),
> + return the location of FNDECL. */
> + if (param == NULL)
> + return DECL_SOURCE_LOCATION (fndecl);
> +
> + return DECL_SOURCE_LOCATION (param);
> +}
> +
> /* 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
> @@ -6680,7 +6709,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
> complained = permerror (loc, "invalid conversion from %qH to %qI",
> TREE_TYPE (expr), totype);
> if (complained && fn)
> - inform (DECL_SOURCE_LOCATION (fn),
> + inform (get_fndecl_argument_location (fn, argnum),
> " initializing argument %P of %qD", argnum, fn);
>
> return cp_convert (totype, expr, complain);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index f4d6f80..7f498b8 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -5659,6 +5659,8 @@ struct cp_parameter_declarator {
> tree default_argument;
> /* True iff this is a template parameter pack. */
> bool template_parameter_pack_p;
> + /* Location within source. */
> + location_t loc;
> };
>
> /* A declarator. */
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 4bfae36..9c1562c 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -1691,6 +1691,7 @@ cp_parameter_declarator *
> make_parameter_declarator (cp_decl_specifier_seq *decl_specifiers,
> cp_declarator *declarator,
> tree default_argument,
> + location_t loc,
> bool template_parameter_pack_p = false)
> {
> cp_parameter_declarator *parameter;
> @@ -1705,6 +1706,7 @@ make_parameter_declarator (cp_decl_specifier_seq *decl_specifiers,
> parameter->declarator = declarator;
> parameter->default_argument = default_argument;
> parameter->template_parameter_pack_p = template_parameter_pack_p;
> + parameter->loc = loc;
>
> return parameter;
> }
> @@ -4379,7 +4381,8 @@ cp_parser_translation_unit (cp_parser* parser)
> /* Create the error declarator. */
> cp_error_declarator = make_declarator (cdk_error);
> /* Create the empty parameter list. */
> - no_parameters = make_parameter_declarator (NULL, NULL, NULL_TREE);
> + no_parameters = make_parameter_declarator (NULL, NULL, NULL_TREE,
> + UNKNOWN_LOCATION);
> /* Remember where the base of the declarator obstack lies. */
> declarator_obstack_base = obstack_next_free (&declarator_obstack);
> }
> @@ -21217,6 +21220,8 @@ cp_parser_parameter_declaration_list (cp_parser* parser, bool *is_error)
> PARM,
> parameter->default_argument != NULL_TREE,
> ¶meter->decl_specifiers.attributes);
> + if (decl != error_mark_node && parameter->loc != UNKNOWN_LOCATION)
> + DECL_SOURCE_LOCATION (decl) = parameter->loc;
> }
>
> deprecated_state = DEPRECATED_NORMAL;
> @@ -21370,6 +21375,7 @@ cp_parser_parameter_declaration (cp_parser *parser,
> = G_("types may not be defined in parameter types");
>
> /* Parse the declaration-specifiers. */
> + cp_token *decl_spec_token_start = cp_lexer_peek_token (parser->lexer);
> cp_parser_decl_specifier_seq (parser,
> CP_PARSER_FLAGS_NONE,
> &decl_specifiers,
> @@ -21554,9 +21560,36 @@ cp_parser_parameter_declaration (cp_parser *parser,
> else
> default_argument = NULL_TREE;
>
> + /* Generate a location for the parameter, ranging from the start of the
> + initial token to the end of the final token (using input_location for
> + the latter, set up by cp_lexer_set_source_position_from_token when
> + consuming tokens).
> +
> + If we have a identifier, then use it for the caret location, e.g.
> +
> + extern int callee (int one, int (*two)(int, int), float three);
> + ~~~~~~^~~~~~~~~~~~~~
> +
> + otherwise, reuse the start location for the caret location e.g.:
> +
> + extern int callee (int one, int (*)(int, int), float three);
> + ^~~~~~~~~~~~~~~~~
> +
> + */
> + location_t param_loc;
> + if (declarator && declarator->id_loc)
> + param_loc = make_location (declarator->id_loc,
> + decl_spec_token_start->location,
> + input_location);
> + else
> + param_loc = make_location (decl_spec_token_start->location,
> + decl_spec_token_start->location,
> + input_location);
> +
> return make_parameter_declarator (&decl_specifiers,
> declarator,
> default_argument,
> + param_loc,
> template_parameter_pack_p);
> }
>
> diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
> index b8833ef..bc3a938 100644
> --- a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
> +++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
> @@ -3,10 +3,7 @@
> /* A collection of calls where argument 2 is of the wrong type.
>
> TODO: we should put the caret and underline for the diagnostic
> - at the second argument, rather than the close paren.
> -
> - TODO: we should highlight the second parameter of the callee, rather
> - than its name. */
> + at the second argument, rather than the close paren. */
>
> /* decl, with argname. */
>
> @@ -22,7 +19,7 @@ int test_1 (int first, int second, float third)
> // { dg-message "initializing argument 2 of 'int callee_1\\(int, const char\\*, float\\)'" "" { target *-*-* } callee_1 }
> /* { dg-begin-multiline-output "" }
> extern int callee_1 (int one, const char *two, float three);
> - ^~~~~~~~
> + ~~~~~~~~~~~~^~~
> { dg-end-multiline-output "" } */
> }
>
> @@ -40,7 +37,7 @@ int test_2 (int first, int second, float third)
> // { dg-message "initializing argument 2 of 'int callee_2\\(int, const char\\*, float\\)'" "" { target *-*-* } callee_2 }
> /* { dg-begin-multiline-output "" }
> extern int callee_2 (int, const char *, float);
> - ^~~~~~~~
> + ^~~~~~~~~~~~
> { dg-end-multiline-output "" } */
> }
>
> @@ -61,7 +58,7 @@ int test_3 (int first, int second, float third)
> // { dg-message "initializing argument 2 of 'int callee_3\\(int, const char\\*, float\\)'" "" { target *-*-* } callee_3 }
> /* { dg-begin-multiline-output "" }
> static int callee_3 (int one, const char *two, float three)
> - ^~~~~~~~
> + ~~~~~~~~~~~~^~~
> { dg-end-multiline-output "" } */
> }
>
> @@ -78,7 +75,7 @@ int test_4 (int first, int second, float third)
> { dg-end-multiline-output "" } */
> /* { dg-begin-multiline-output "" }
> struct s4 { static int member_1 (int one, const char *two, float three); };
> - ^~~~~~~~
> + ~~~~~~~~~~~~^~~
> { dg-end-multiline-output "" } */
> }
>
> @@ -96,7 +93,7 @@ int test_5 (int first, int second, float third)
> { dg-end-multiline-output "" } */
> /* { dg-begin-multiline-output "" }
> struct s5 { int member_1 (int one, const char *two, float three); };
> - ^~~~~~~~
> + ~~~~~~~~~~~~^~~
> { dg-end-multiline-output "" } */
> }
>
> @@ -113,7 +110,7 @@ int test_6 (int first, int second, float third, s6 *ptr)
> { dg-end-multiline-output "" } */
> /* { dg-begin-multiline-output "" }
> struct s6 { int member_1 (int one, const char *two, float three); };
> - ^~~~~~~~
> + ~~~~~~~~~~~~^~~
> { dg-end-multiline-output "" } */
> }
>
> @@ -153,7 +150,7 @@ int test_8 (int first, int second, float third)
> { dg-end-multiline-output "" } */
> /* { dg-begin-multiline-output "" }
> struct s8 { static int member_1 (int one, T two, float three); };
> - ^~~~~~~~
> + ~~^~~
> { dg-end-multiline-output "" } */
> }
>
> @@ -172,7 +169,43 @@ int test_9 (int first, int second, float third)
> { dg-end-multiline-output "" } */
> /* { dg-begin-multiline-output "" }
> struct s9 { int member_1 (int one, T two, float three); };
> - ^~~~~~~~
> + ~~^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +/* Callback with name. */
> +
> +extern int callee_10 (int one, int (*two)(int, int), float three); // { dg-line callee_10 }
> +
> +int test_10 (int first, int second, float third)
> +{
> + return callee_10 (first, second, third); // { dg-error "invalid conversion from 'int' to 'int \\(\\*\\)\\(int, int\\)'" }
> + /* { dg-begin-multiline-output "" }
> + return callee_10 (first, second, third);
> + ^
> + { dg-end-multiline-output "" } */
> + // { dg-message "initializing argument 2 of 'int callee_10\\(int, int \\(\\*\\)\\(int, int\\), float\\)'" "" { target *-*-* } callee_10 }
> + /* { dg-begin-multiline-output "" }
> + extern int callee_10 (int one, int (*two)(int, int), float three);
> + ~~~~~~^~~~~~~~~~~~~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +/* Callback without name. */
> +
> +extern int callee_11 (int one, int (*)(int, int), float three); // { dg-line callee_11 }
> +
> +int test_11 (int first, int second, float third)
> +{
> + return callee_11 (first, second, third); // { dg-error "invalid conversion from 'int' to 'int \\(\\*\\)\\(int, int\\)'" }
> + /* { dg-begin-multiline-output "" }
> + return callee_11 (first, second, third);
> + ^
> + { dg-end-multiline-output "" } */
> + // { dg-message "initializing argument 2 of 'int callee_11\\(int, int \\(\\*\\)\\(int, int\\), float\\)'" "" { target *-*-* } callee_11 }
> + /* { dg-begin-multiline-output "" }
> + extern int callee_11 (int one, int (*)(int, int), float three);
> + ^~~~~~~~~~~~~~~~~
> { dg-end-multiline-output "" } */
> }
>
>
More information about the Gcc-patches
mailing list