Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2017-07-31 at 18:05 +0200, Marek Polacek wrote:
> On Mon, Jul 31, 2017 at 09:54:03AM -0600, Martin Sebor wrote:
> > On 07/31/2017 08:14 AM, Marek Polacek wrote:
> > > This patch improves the diagnostic of -Wsign-compare for ?: by
> > > also printing
> > > the types, similarly to my recent patch.  But we can do even
> > > better here if we
> > > actually point to the operand in question, so I passed the
> > > locations of the
> > > operands from the parser.  So instead of
> > > 
> > > x.c:8:16: warning: signed and unsigned type in conditional
> > > expression [-Wsign-compare]
> > >    return x ? y : -1;
> > >                 ^
> > > you'll now see:
> > > 
> > > x.c:8:18: warning: operand of conditional expression changes
> > > signedness: 'int' to 'unsigned int' [-Wsign-compare]
> > >    return x ? y : -1;
> > >                   ^
> > 
> > I like that this is more informative than the last warning you
> > committed for this bug: it says what type the operand is converted
> > to.  The last one only shows what the types of the operands are but
> > leaves users guessing as to what that might mean (integer promotion
> > rules are often poorly understood).  Where the last warning prints
> > 
> >   comparison of integer expressions of different signedness: ‘int’
> > and
> > ‘unsigned int’
> > 
> > it would be nice to go back and add this detail to it as well, and
> > have it print something like this instead:
> > 
> >   comparison of integer expressions of different signedness changes
> > type of
> > the second operand from ‘int’ to ‘unsigned int’
> > 
> > Where constant expressions are involved it would also be helpful
> > to show the result of the conversion.  For instance:
> > 
> >   comparison between ‘int’ and ‘unsigned int’ changes the value of
> > the
> > second operand from ‘-1’ to ‘4294967296’
> 
> Hmm, interesting.  I could do that.  How do other people feel about
> this?
> 
> 	Marek

I think that printing values is very helpful for convincing the user
that they need to listen to the compiler.

That said, we could do a better job of printing values (and boundary
values), especially for large values near a power of two; Martin filed
that as PR 80437; I've added some thoughts on that to that PR.