Re: [PATCH 01/17] Add param-type-mismatch.c/C testcases as a baseline

2017-09-01 Thread Jeff Law
On 07/24/2017 02:04 PM, David Malcolm wrote:
> Amongst other things, this patch kit aims to improve our handling
> of mismatched types within function calls.
Certainly a worthy goal. It's silly that someone trying to parse the
diagnostic we currently give to have to manually walk the arguments and
figure out where the mismatch is.  Underlining the mismatched parameter
at the call site and in the declaration would be a huge step forward.

I'd support getting these tests into the testsuite independent of the
BLT work.  They may need to be xfailed for a period of time.

If you wanted to go forward on that, be my guest.

jeff


Re: [PATCH 01/17] Add param-type-mismatch.c/C testcases as a baseline

2017-07-24 Thread Eric Gallager
On Mon, Jul 24, 2017 at 4:04 PM, David Malcolm  wrote:
> Amongst other things, this patch kit aims to improve our handling
> of mismatched types within function calls.
>
> For example, given:
>
>   extern int callee_1 (int one, const char *two, float three);
>
>   int test_1 (int first, int second, float third)
>   {
> return callee_1 (first, second, third);
>   }
>
> the C++ FE currently reports:
>
>   error: invalid conversion from 'int' to 'const char*' [-fpermissive]
>return callee_1 (first, second, third);
> ^
>   note:   initializing argument 2 of 'int callee_1(int, const char*, float)'
>extern int callee_1 (int one, const char *two, float three);
>   ^~~~
>
> when it ought to underline the pertinent parts of the code:
>
>   error: invalid conversion from 'int' to 'const char*' [-fpermissive]
>return callee_1 (first, second, third);
>^~
>   note:   initializing argument 2 of 'int callee_1(int, const char*, float)'
>extern int callee_1 (int one, const char *two, float three);
>  ^~~
>
> The C FE currently does better, underlining the pertinent argument at
> the callsite (due to the "vec arg_loc" passed around
> when the call is created); but, like the C++ frontend, it doesn't
> underline the pertinent parameter at the decl of the callee.
>
> This patch adds a pair of test cases to exercise various cases of
> this in the C and C++ frontends, to establish a baseline for how
> we currently handle these; the "TODO" comments in the test cases
> note the aspects where we could do better (some of which are fixed
> by this kit).
>
> Patches 6 and 7 of the kit fix the parameter highlighting, for
> C and C++ respectively (making use of -fblt).

Speaking of mismatched types for arguments to function calls, while
you're at it, could you
update gcc.dg/Wtraditional-conversion-2.c to use underlining, too?
Currently it just checks
the message and not any of the location information.

>
> gcc/testsuite/ChangeLog:
> * g++.dg/diagnostic/param-type-mismatch.C: New test acse.
> * gcc.dg/param-type-mismatch.c: New test case.
> ---
>  .../g++.dg/diagnostic/param-type-mismatch.C| 162 
> +
>  gcc/testsuite/gcc.dg/param-type-mismatch.c |  63 
>  2 files changed, 225 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
>  create mode 100644 gcc/testsuite/gcc.dg/param-type-mismatch.c
>
> diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C 
> b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
> new file mode 100644
> index 000..864ead1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch.C
> @@ -0,0 +1,162 @@
> +// { dg-options "-fdiagnostics-show-caret" }
> +
> +/* 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.  */
> +
> +/* decl, with argname.  */
> +
> +extern int callee_1 (int one, const char *two, float three); // { dg-line 
> callee_1 }
> +
> +int test_1 (int first, int second, float third)
> +{
> +  return callee_1 (first, second, third); // { dg-error "invalid conversion 
> from 'int' to 'const char\\*'" }
> +  /* { dg-begin-multiline-output "" }
> +   return callee_1 (first, second, third);
> +^
> + { dg-end-multiline-output "" } */
> +  // { 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 "" } */
> +}
> +
> +/* decl, without argname.  */
> +
> +extern int callee_2 (int, const char *, float); // { dg-line callee_2 }
> +
> +int test_2 (int first, int second, float third)
> +{
> +  return callee_2 (first, second, third); // { dg-error "invalid conversion 
> from 'int' to 'const char\\*'" }
> +  /* { dg-begin-multiline-output "" }
> +   return callee_2 (first, second, third);
> +^
> + { dg-end-multiline-output "" } */
> +  // { 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 "" } */
> +}
> +
> +/* defn, with argname.  */
> +
> +static int callee_3 (int one, const char *two, float three) // { dg-line 
> callee_3 }
> +{
> +  return callee_2 (one, two, three);
> +}
> +
> +int test_3 (int first, int second, float third)
> +{
> +  return callee_3 (first, second