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.
>
> OK for trunk (with my "diagnostic messages" maintainer hat on).
>
> Thanks
> Dave
>
>> 2017-07-13 Marek Polacek <pola...@redhat.com>
>>
>> PR c/81417
>> * c-warn.c (warn_for_sign_compare): Print the types.
>>
>> * c-c++-common/Wsign-compare-1.c: New test.
>>
>> diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
>> index b9378c2dbe2..c903c080a33 100644
>> --- gcc/c-family/c-warn.c
>> +++ gcc/c-family/c-warn.c
>> @@ -1891,9 +1891,10 @@ warn_for_sign_compare (location_t location,
>> c_common_signed_type
>> (base_type)))
>> /* OK */;
>> else
>> - warning_at (location,
>> - OPT_Wsign_compare,
>> - "comparison between signed and unsigned integer
>> expressions");
>> + warning_at (location, OPT_Wsign_compare,
>> + "comparison between signed and unsigned integer
>> "
>> + "expressions: %qT and %qT", TREE_TYPE (sop),
>> + TREE_TYPE (uop));
>> }
>>
>> /* Warn if two unsigned values are being compared in a size larger
>> diff --git gcc/testsuite/c-c++-common/Wsign-compare-1.c
>> gcc/testsuite/c-c++-common/Wsign-compare-1.c
>> index e69de29bb2d..e53f87aa9a3 100644
>> --- gcc/testsuite/c-c++-common/Wsign-compare-1.c
>> +++ gcc/testsuite/c-c++-common/Wsign-compare-1.c
>> @@ -0,0 +1,27 @@
>> +/* PR c/81417 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-Wsign-compare" } */
>> +
>> +int
>> +fn1 (signed int a, unsigned int b)
>> +{
>> + return a < b; /* { dg-warning "comparison between signed and
>> unsigned integer expressions: 'int' and 'unsigned int'" } */
>> +}
>> +
>> +int
>> +fn2 (signed int a, unsigned int b)
>> +{
>> + return b < a; /* { dg-warning "comparison between signed and
>> unsigned integer expressions: 'int' and 'unsigned int'" } */
>> +}
>> +
>> +int
>> +fn3 (signed long int a, unsigned long int b)
>> +{
>> + return b < a; /* { dg-warning "comparison between signed and
>> unsigned integer expressions: 'long int' and 'long unsigned int'" }
>> */
>> +}
>> +
>> +int
>> +fn4 (signed short int a, unsigned int b)
>> +{
>> + return b < a; /* { dg-warning "comparison between signed and
>> unsigned integer expressions: 'short int' and 'unsigned int'" } */
>> +}
>>
>> Marek
>
