Ping.
On Fri, Jul 14, 2017 at 03:27:21PM +0200, Marek Polacek wrote:
> On Thu, Jul 13, 2017 at 04:59:20PM -0400, David Malcolm wrote:
> > On Thu, 2017-07-13 at 16:39 -0400, Eric Gallager wrote:
> > > On 7/13/17, David Malcolm <dmalc...@redhat.com> wrote:
> > > > On Thu, 2017-07-13 at 18:33 +0200, Marek Polacek wrote:
> > > > > A tiny patch for -Wsign-compare so that is also prints the types
> > > > > when
> > > > > reporting a warning.
> > > > >
> > > > > Bootstrapped/regtested on x86_64-linux and ppc64le-redhat-linux,
> > > > > ok
> > > > > for trunk?
> > > >
> > > > Looks like it always display the types in the order signed then
> > > > unsigned, which matches the text of the diagnostic, but not
> > > > necessarily
> > > > the ordering within the expression, which might be confusing if
> > > > someone's comparing e.g.
> > > >
> > > > unsigned_a < signed_b
> > > >
> > >
> > > Good catch, I forgot about that case when opening the original bug
> > > that Marek posted this patch for...
> > >
> > > > But we already hardcode the ordering within the text of the
> > > > diagnostic,
> > > > so that feels excessively nit-picky.
> > >
> > > I don't think it's being excessively nit-picky; I think it'd make
> > > more
> > > sense to match the ordering of the expression. That's what clang
> > > does:
> > >
> > > $ cat Wsign_compare.c
> > > /* { dg-do compile } */
> > >
> > > int foo(signed int a, unsigned int b)
> > > {
> > > return (a < b);
> > > }
> > >
> > > int bar(unsigned int c, signed int d)
> > > {
> > > return (c < d);
> > > }
> > >
> > > $ /sw/opt/llvm-3.1/bin/clang -c -Wsign-compare Wsign_compare.c
> > > Wsign_compare.c:5:12: warning: comparison of integers of different
> > > signs: 'int' and 'unsigned int' [-Wsign-compare]
> > > return (a < b);
> > > ~ ^ ~
> > > Wsign_compare.c:10:12: warning: comparison of integers of different
> > > signs: 'unsigned int' and 'int' [-Wsign-compare]
> > > return (c < d);
> > > ~ ^ ~
> > > 2 warnings generated.
> >
> > That's much nicer.
> >
> > >
> > > >
> > > > OK for trunk (with my "diagnostic messages" maintainer hat on).
> >
> > Marek: I take it back; can you update the patch accordingly, please?
> >
> > (Note to self: always doublecheck the linked PR for context).
>
> Sure, here goes:
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-07-14 Marek Polacek <pola...@redhat.com>
>
> PR c/81417
> * c-warn.c (warn_for_sign_compare): Print the types.
>
> * c-c++-common/Wsign-compare-1.c: New test.
>
> diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
> index b9378c2dbe2..7ff11821453 100644
> --- gcc/c-family/c-warn.c
> +++ gcc/c-family/c-warn.c
> @@ -1891,9 +1891,10 @@ warn_for_sign_compare (location_t location,
> c_common_signed_type (base_type)))
> /* OK */;
> else
> - warning_at (location,
> - OPT_Wsign_compare,
> - "comparison between signed and unsigned integer
> expressions");
> + warning_at (location, OPT_Wsign_compare,
> + "comparison between signed and unsigned integer "
> + "expressions: %qT and %qT", TREE_TYPE (orig_op0),
> + TREE_TYPE (orig_op1));
> }
>
> /* Warn if two unsigned values are being compared in a size larger
> diff --git gcc/testsuite/c-c++-common/Wsign-compare-1.c
> gcc/testsuite/c-c++-common/Wsign-compare-1.c
> index e69de29bb2d..0e01453e7d8 100644
> --- gcc/testsuite/c-c++-common/Wsign-compare-1.c
> +++ gcc/testsuite/c-c++-common/Wsign-compare-1.c
> @@ -0,0 +1,27 @@
> +/* PR c/81417 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wsign-compare" } */
> +
> +int
> +fn1 (signed int a, unsigned int b)
> +{
> + return a < b; /* { dg-warning "comparison between signed and unsigned
> integer expressions: 'int' and 'unsigned int'" } */
> +}
> +
> +int
> +fn2 (signed int a, unsigned int b)
> +{
> + return b < a; /* { dg-warning "comparison between signed and unsigned
> integer expressions: 'unsigned int' and 'int'" } */
> +}
> +
> +int
> +fn3 (signed long int a, unsigned long int b)
> +{
> + return b < a; /* { dg-warning "comparison between signed and unsigned
> integer expressions: 'long unsigned int' and 'long int'" } */
> +}
> +
> +int
> +fn4 (signed short int a, unsigned int b)
> +{
> + return b < a; /* { dg-warning "comparison between signed and unsigned
> integer expressions: 'unsigned int' and 'short int'" } */
> +}
Marek
