Re: c-family PATCH to improve -Wsign-compare (PR c/81417)

[David Malcolm <dmalcolm at redhat dot com>]

On Thu, 2017-07-13 at 16:39 -0400, Eric Gallager wrote:
> On 7/13/17, David Malcolm <dmalcolm@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).