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 <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.
>
> 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 <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..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'" } */
+}
Marek
