On Thu, Jul 13, 2017 at 04:59:20PM -0400, David Malcolm wrote:
On Thu, 2017-07-13 at 16:39 -0400, Eric Gallager wrote:
On 7/13/17, David Malcolm <dmalcolm@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).
Sure, here goes:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-07-14 Marek Polacek <polacek@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..7ff11821453 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 (orig_op0),
+ TREE_TYPE (orig_op1));
}
/* 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..0e01453e7d8 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: 'unsigned int' and 'int'" } */
+}
+
+int
+fn3 (signed long int a, unsigned long int b)
+{
+ return b < a; /* { dg-warning "comparison between signed and unsigned integer expressions: 'long unsigned int' and 'long int'" } */
+}
+
+int
+fn4 (signed short int a, unsigned int b)
+{
+ return b < a; /* { dg-warning "comparison between signed and unsigned integer expressions: 'unsigned int' and 'short int'" } */
+}