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
