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]

Re: PR c/11492 bogus -Wsign-compare warning


PING.

2008/10/20 Manuel López-Ibáñez <lopezibanez@gmail.com>:
> I have updated the patch to a recent revision, bootstrapped and
> regression tested on x86_64-unknown-linux-gnu with
> --enable-languages=all
>
> (The C++ testcase was already approved by Mark Mitchell.)
>
> OK for trunk?
>
> 2008-10-20  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>       PR 11492
>       * c-common.c (min_precision): Move to...
>       * tree.c (tree_int_cst_min_precision): ... to here. Renamed.
>       * tree.h (tree_int_cst_min_precision): Declare.
>       * c-common.h (min_precision): Delete declaration.
>       * fold-const.c (tree_binary_nonnegative_warnv_p): Handle
>       multiplication of non-negative integer constants.
>       * c-decl.c (check_bitfield_type_and_width): Rename min_precision to
>       tree_int_cst_min_precision.
>       (finish_enum): Likewise.
> cp/
>       * class.c (check_bitfield_decl): Rename min_precision to
>       tree_int_cst_min_precision.
>       * decl.c (finish_enum): Likewise.
> testsuite/
>       * gcc.dg/pr11492.c: New.
>       * g++.dg/warn/pr11492.C: New.
>
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 141210)
+++ gcc/tree.c	(working copy)
@@ -5033,10 +5033,37 @@ tree_int_cst_sgn (const_tree t)
     return -1;
   else
     return 1;
 }
 
+/* Return the minimum number of bits needed to represent VALUE in a
+   signed or unsigned type, UNSIGNEDP says which.  */
+
+unsigned int
+tree_int_cst_min_precision (tree value, bool unsignedp)
+{
+  int log;
+
+  /* If the value is negative, compute its negative minus 1.  The latter
+     adjustment is because the absolute value of the largest negative value
+     is one larger than the largest positive value.  This is equivalent to
+     a bit-wise negation, so use that operation instead.  */
+
+  if (tree_int_cst_sgn (value) < 0)
+    value = fold_build1 (BIT_NOT_EXPR, TREE_TYPE (value), value);
+
+  /* Return the number of bits needed, taking into account the fact
+     that we need one more bit for a signed than unsigned type.  */
+
+  if (integer_zerop (value))
+    log = 0;
+  else
+    log = tree_floor_log2 (value);
+
+  return log + 1 + !unsignedp;
+}
+
 /* Compare two constructor-element-type constants.  Return 1 if the lists
    are known to be equal; otherwise return 0.  */
 
 int
 simple_cst_list_equal (const_tree l1, const_tree l2)
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 141210)
+++ gcc/tree.h	(working copy)
@@ -4020,10 +4020,11 @@ extern int tree_int_cst_compare (const_t
 extern int host_integerp (const_tree, int);
 extern HOST_WIDE_INT tree_low_cst (const_tree, int);
 extern int tree_int_cst_msb (const_tree);
 extern int tree_int_cst_sgn (const_tree);
 extern int tree_int_cst_sign_bit (const_tree);
+extern unsigned int tree_int_cst_min_precision (tree, bool);
 extern bool tree_expr_nonnegative_p (tree);
 extern bool tree_expr_nonnegative_warnv_p (tree, bool *);
 extern bool may_negate_without_overflow_p (const_tree);
 extern tree strip_array_types (tree);
 
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 141210)
+++ gcc/fold-const.c	(working copy)
@@ -14018,19 +14018,42 @@ tree_binary_nonnegative_warnv_p (enum tr
 	}
 
       /* zero_extend(x) * zero_extend(y) is non-negative if x and y are
 	 both unsigned and their total bits is shorter than the result.  */
       if (TREE_CODE (type) == INTEGER_TYPE
-	  && TREE_CODE (op0) == NOP_EXPR
-	  && TREE_CODE (op1) == NOP_EXPR)
+	  && (TREE_CODE (op0) == NOP_EXPR || TREE_CODE (op0) == INTEGER_CST)
+	  && (TREE_CODE (op1) == NOP_EXPR || TREE_CODE (op1) == INTEGER_CST))
 	{
-	  tree inner1 = TREE_TYPE (TREE_OPERAND (op0, 0));
-	  tree inner2 = TREE_TYPE (TREE_OPERAND (op1, 0));
-	  if (TREE_CODE (inner1) == INTEGER_TYPE && TYPE_UNSIGNED (inner1)
-	      && TREE_CODE (inner2) == INTEGER_TYPE && TYPE_UNSIGNED (inner2))
-	    return TYPE_PRECISION (inner1) + TYPE_PRECISION (inner2)
-		   < TYPE_PRECISION (type);
+	  tree inner0 = (TREE_CODE (op0) == NOP_EXPR) 
+	    ? TREE_TYPE (TREE_OPERAND (op0, 0))
+	    : TREE_TYPE (op0);
+	  tree inner1 = (TREE_CODE (op1) == NOP_EXPR) 
+	    ? TREE_TYPE (TREE_OPERAND (op1, 0))
+	    : TREE_TYPE (op1);
+
+	  bool unsigned0 = TYPE_UNSIGNED (inner0);
+	  bool unsigned1 = TYPE_UNSIGNED (inner1);
+
+	  if (TREE_CODE (op0) == INTEGER_CST)
+	    unsigned0 = unsigned0 || tree_int_cst_sgn (op0) >= 0;
+
+	  if (TREE_CODE (op1) == INTEGER_CST)
+	    unsigned1 = unsigned1 || tree_int_cst_sgn (op1) >= 0;
+
+	  if (TREE_CODE (inner0) == INTEGER_TYPE && unsigned0
+	      && TREE_CODE (inner1) == INTEGER_TYPE && unsigned1)
+	    {
+	      unsigned int precision0 = (TREE_CODE (op0) == INTEGER_CST)
+		? tree_int_cst_min_precision (op0, /*unsignedp=*/true)
+		: TYPE_PRECISION (inner0);
+
+	      unsigned int precision1 = (TREE_CODE (op1) == INTEGER_CST)
+		? tree_int_cst_min_precision (op1, /*unsignedp=*/true)
+		: TYPE_PRECISION (inner1);
+
+	      return precision0 + precision1 < TYPE_PRECISION (type);
+	    }
 	}
       return false;
 
     case BIT_AND_EXPR:
     case MAX_EXPR:
Index: gcc/testsuite/gcc.dg/pr11492.c
===================================================================
--- gcc/testsuite/gcc.dg/pr11492.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr11492.c	(revision 0)
@@ -0,0 +1,12 @@
+/* PR11492 */
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare" } */
+int main( void )
+{
+  unsigned int a;
+  unsigned char b;
+  for ( a = 0, b = 2; a > b * 1000; a++ ) /* { dg-bogus "comparison between signed and unsigned integer" } */
+    { ; }
+
+  return 0;
+}
Index: gcc/testsuite/g++.dg/warn/pr11492.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr11492.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr11492.C	(revision 0)
@@ -0,0 +1,12 @@
+// PR11492 
+// { dg-do compile }
+// { dg-options "-Wsign-compare" }
+int main( void )
+{
+  unsigned int a;
+  unsigned char b;
+  for ( a = 0, b = 2; a > b * 1000; a++ ) /* { dg-bogus "comparison between signed and unsigned integer" } */
+    { ; }
+
+  return 0;
+}
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 141210)
+++ gcc/cp/class.c	(working copy)
@@ -2726,14 +2726,15 @@ check_bitfield_decl (tree field)
 	       && TREE_CODE (type) != ENUMERAL_TYPE
 	       && TREE_CODE (type) != BOOLEAN_TYPE)
 	warning (0, "width of %q+D exceeds its type", field);
       else if (TREE_CODE (type) == ENUMERAL_TYPE
 	       && (0 > compare_tree_int (w,
-					 min_precision (TYPE_MIN_VALUE (type),
-							TYPE_UNSIGNED (type)))
+					 tree_int_cst_min_precision
+					 (TYPE_MIN_VALUE (type),
+					  TYPE_UNSIGNED (type)))
 		   ||  0 > compare_tree_int (w,
-					     min_precision
+					     tree_int_cst_min_precision
 					     (TYPE_MAX_VALUE (type),
 					      TYPE_UNSIGNED (type)))))
 	warning (0, "%q+D is too small to hold all values of %q#T", field, type);
     }
 
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 141210)
+++ gcc/cp/decl.c	(working copy)
@@ -10965,15 +10965,15 @@ finish_enum (tree enumtype)
        the enumeration had a single enumerator with value 0.  */
     minnode = maxnode = integer_zero_node;
 
   /* Compute the number of bits require to represent all values of the
      enumeration.  We must do this before the type of MINNODE and
-     MAXNODE are transformed, since min_precision relies on the
-     TREE_TYPE of the value it is passed.  */
+     MAXNODE are transformed, since tree_int_cst_min_precision relies
+     on the TREE_TYPE of the value it is passed.  */
   unsignedp = tree_int_cst_sgn (minnode) >= 0;
-  lowprec = min_precision (minnode, unsignedp);
-  highprec = min_precision (maxnode, unsignedp);
+  lowprec = tree_int_cst_min_precision (minnode, unsignedp);
+  highprec = tree_int_cst_min_precision (maxnode, unsignedp);
   precision = MAX (lowprec, highprec);
 
   if (!fixed_underlying_type_p)
     {
       /* Determine the underlying type of the enumeration.
Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(revision 141210)
+++ gcc/c-decl.c	(working copy)
@@ -3876,12 +3876,12 @@ check_bitfield_type_and_width (tree *typ
 
   if (TREE_CODE (*type) == ENUMERAL_TYPE)
     {
       struct lang_type *lt = TYPE_LANG_SPECIFIC (*type);
       if (!lt
-	  || w < min_precision (lt->enum_min, TYPE_UNSIGNED (*type))
-	  || w < min_precision (lt->enum_max, TYPE_UNSIGNED (*type)))
+	  || w < tree_int_cst_min_precision (lt->enum_min, TYPE_UNSIGNED (*type))
+	  || w < tree_int_cst_min_precision (lt->enum_max, TYPE_UNSIGNED (*type)))
 	warning (0, "%qs is narrower than values of its type", name);
     }
 }
 
 
@@ -5873,12 +5873,12 @@ finish_enum (tree enumtype, tree values,
   /* Construct the final type of this enumeration.  It is the same
      as one of the integral types - the narrowest one that fits, except
      that normally we only go as narrow as int - and signed iff any of
      the values are negative.  */
   unsign = (tree_int_cst_sgn (minnode) >= 0);
-  precision = MAX (min_precision (minnode, unsign),
-		   min_precision (maxnode, unsign));
+  precision = MAX (tree_int_cst_min_precision (minnode, unsign),
+		   tree_int_cst_min_precision (maxnode, unsign));
 
   if (TYPE_PACKED (enumtype) || precision > TYPE_PRECISION (integer_type_node))
     {
       tem = c_common_type_for_size (precision, unsign);
       if (tem == NULL)
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 141210)
+++ gcc/c-common.c	(working copy)
@@ -2793,38 +2793,10 @@ c_register_builtin_type (tree type, cons
     TYPE_NAME (type) = decl;
   pushdecl (decl);
 
   registered_builtin_types = tree_cons (0, type, registered_builtin_types);
 }
-
-
-/* Return the minimum number of bits needed to represent VALUE in a
-   signed or unsigned type, UNSIGNEDP says which.  */
-
-unsigned int
-min_precision (tree value, int unsignedp)
-{
-  int log;
-
-  /* If the value is negative, compute its negative minus 1.  The latter
-     adjustment is because the absolute value of the largest negative value
-     is one larger than the largest positive value.  This is equivalent to
-     a bit-wise negation, so use that operation instead.  */
-
-  if (tree_int_cst_sgn (value) < 0)
-    value = fold_build1 (BIT_NOT_EXPR, TREE_TYPE (value), value);
-
-  /* Return the number of bits needed, taking into account the fact
-     that we need one more bit for a signed than unsigned type.  */
-
-  if (integer_zerop (value))
-    log = 0;
-  else
-    log = tree_floor_log2 (value);
-
-  return log + 1 + !unsignedp;
-}
 
 /* Print an error message for invalid operands to arith operation
    CODE with TYPE0 for operand 0, and TYPE1 for operand 1.
    LOCATION is the location of the message.  */
 
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h	(revision 141210)
+++ gcc/c-common.h	(working copy)
@@ -744,11 +744,10 @@ extern tree shorten_binary_op (tree resu
    See if the operands have both been converted from subword integer types
    and, if so, perhaps change them both back to their original type.  */
 extern tree shorten_compare (tree *, tree *, tree *, enum tree_code *);
 
 extern tree pointer_int_sum (enum tree_code, tree, tree);
-extern unsigned int min_precision (tree, int);
 
 /* Add qualifiers to a type, in the fashion for C.  */
 extern tree c_build_qualified_type (tree, int);
 
 /* Build tree nodes and builtin functions common to both C and C++ language

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