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

[Marek Polacek <polacek at redhat dot com>]

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