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).
