This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PATCH RFA: C++: Don't warn about signed/unsigned comparisons with enums


For code like this:

enum E { A, B, C };
extern void f1 (enum E);
void
f2 (enum E v1, enum E v2)
{
  unsigned int unsigned_i;
  int signed_i;

  for (unsigned_i = v1; unsigned_i <= v2; ++unsigned_i)
    f1 ((enum E) unsigned_i);
  for (signed_i = v1; signed_i <= v2; ++signed_i)
    f1 ((enum E) signed_i);
}

when using -Wsign-compare, the C frontend warns about the second loop
and the C++ frontend warns about the first loop.

Both frontends turn the enum into an unsigned type, because it has no
nonnegative values.  In the C frontend variables of the enum type
effectively have type "unsigned int", so the C frontend issues a
-Wsign-compare warning for the second loop, which compares the "int"
signed_i with the "unsigned int" v2.

The C++ frontend is different.  "v2" does have the enum type.  When the
frontend sees "unsigned_i <= v2", it applies the default integer
promotions to both sides.  This promotes "v2" to "int", not "unsigned
int", per the standard.  The code then looks like "unsigned_i <= (int)
v2".  Since "unsigned_i" has type "unsigned int", this causes a
-Wsign-compare warning.

These differing warnings are a problem for the gcc-in-cxx branch.
Although the enum does technically have an unsigned type, I don't think
it would be useful to have the C++ frontend start warning about the case
of "signed_i <= v2".  That would introduce new warnings into existing
code, which would be OK except that these new warnings would be useless.
If the enum had values large enough to not fit into "int", then it would
be promoted to "unsigned int", not "int", and the warning would trigger.
So this new warning would never warn about a real problem.

So, I propose specifically removing the "unsigned_i <= v2" warning.
This can be done by identifying the specific cases where it occurs:
namely, an unsigned enum promoted to int.  That is what this patch does.

This solves all but one of these problems on the gcc-in-cxx branch.  The
remaining case involves a ?: expression buried in a statement expression
(from TYPE_MODE) and the cast to int gets pushed into the ?: operands by
fold_unary.  I'm not yet sure what to do about that case.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for mainline?
Or do you have a different suggestion?

Ian


gcc/ChangeLog:

2009-06-17  Ian Lance Taylor  <iant@google.com>

	* c-common.c (warn_for_sign_compare): In C++, don't warn about
	comparing an unsigned quantity to an enum value cast to int.

gcc/testsuite/ChangeLog:

2009-06-17  Ian Lance Taylor  <iant@google.com>

	* g++.dg/warn/Wsign-compare-3.C: New testcase.


Index: testsuite/g++.dg/warn/Wsign-compare-3.C
===================================================================
--- testsuite/g++.dg/warn/Wsign-compare-3.C	(revision 0)
+++ testsuite/g++.dg/warn/Wsign-compare-3.C	(revision 0)
@@ -0,0 +1,13 @@
+// { dg-do compile }
+// { dg-options "-Wsign-compare" }
+
+enum E { A, B, C };
+extern void f1(int);
+void
+f2(E v1, E v2)
+{
+  for (unsigned int i = v1; i <= v2; ++i)
+    f1(i);
+  for (int i = v1; i <= v2; ++i)
+    f1(i);
+}
Index: c-common.c
===================================================================
--- c-common.c	(revision 148613)
+++ c-common.c	(working copy)
@@ -9037,6 +9037,15 @@ warn_for_sign_compare (location_t locati
                && int_fits_type_p (TYPE_MAX_VALUE (TREE_TYPE (uop)),
 				   c_common_signed_type (base_type)))
         /* OK */;
+      /* In C++, do not warn if the signed quantity is an unsigned
+	 enum promoted to int because of the C++ type promotion
+	 rules.  */
+      else if (c_dialect_cxx ()
+	       && TREE_CODE (sop) == NOP_EXPR
+	       && TREE_TYPE (sop) == integer_type_node
+	       && TREE_CODE (TREE_TYPE (TREE_OPERAND (sop, 0))) == ENUMERAL_TYPE
+	       && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (sop, 0))))
+	/* OK */;
       else 
         warning_at (location,
 		    OPT_Wsign_compare, 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]