re PR c/34389 (-Wconversion produces wrong warning)
authorManuel López-Ibáñez <manu@gcc.gnu.org>
Wed, 30 Jul 2008 08:30:32 +0000 (08:30 +0000)
committerManuel López-Ibáñez <manu@gcc.gnu.org>
Wed, 30 Jul 2008 08:30:32 +0000 (08:30 +0000)
2008-07-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

PR 34389
* c-typeck.c (build_binary_op): Encapsulate code into...
* c-common.c (shorten_binary_op): ...this new function.
(conversion_warning): Use the new function. Handle non-negative
constant in bitwise-and.
* c-common.h (shorten_binary_op): Declare.
cp/
* typeck.c (build_binary_op): Encapsulate code into
shorten_binary_op.
testsuite/
* gcc.dg/Wconversion-pr34389.c: New.
* g++.dg/warn/Wconversion-pr34389.C: New.

From-SVN: r138296

gcc/ChangeLog
gcc/c-common.c
gcc/c-common.h
gcc/c-typeck.c
gcc/cp/ChangeLog
gcc/cp/typeck.c
gcc/testsuite/ChangeLog
gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C [new file with mode: 0644]
gcc/testsuite/gcc.dg/Wconversion-pr34389.c [new file with mode: 0644]

index 19e2f3b..671b794 100644 (file)
@@ -1,3 +1,12 @@
+2008-07-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
+
+       PR 34389
+       * c-typeck.c (build_binary_op): Encapsulate code into...
+       * c-common.c (shorten_binary_op): ...this new function.
+       (conversion_warning): Use the new function. Handle non-negative
+       constant in bitwise-and.
+       * c-common.h (shorten_binary_op): Declare.
+       
 2008-07-30  Olivier Hainque  <hainque@adacore.com>
 
        * scan.c (make_sstring_space): Add explicit conversions of
index caac53e..dac29ea 100644 (file)
@@ -1447,6 +1447,110 @@ vector_types_convertible_p (const_tree t1, const_tree t2, bool emit_lax_note)
   return false;
 }
 
+/* This is a helper function of build_binary_op.
+
+   For certain operations if both args were extended from the same
+   smaller type, do the arithmetic in that type and then extend.
+
+   BITWISE indicates a bitwise operation.
+   For them, this optimization is safe only if
+   both args are zero-extended or both are sign-extended.
+   Otherwise, we might change the result.
+   Eg, (short)-1 | (unsigned short)-1 is (int)-1
+   but calculated in (unsigned short) it would be (unsigned short)-1.  
+*/
+tree shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise)
+{
+  int unsigned0, unsigned1;
+  tree arg0, arg1;
+  int uns;
+  tree type;
+
+  /* Cast OP0 and OP1 to RESULT_TYPE.  Doing so prevents
+     excessive narrowing when we call get_narrower below.  For
+     example, suppose that OP0 is of unsigned int extended
+     from signed char and that RESULT_TYPE is long long int.
+     If we explicitly cast OP0 to RESULT_TYPE, OP0 would look
+     like
+     
+     (long long int) (unsigned int) signed_char
+
+     which get_narrower would narrow down to
+     
+     (unsigned int) signed char
+     
+     If we do not cast OP0 first, get_narrower would return
+     signed_char, which is inconsistent with the case of the
+     explicit cast.  */
+  op0 = convert (result_type, op0);
+  op1 = convert (result_type, op1);
+
+  arg0 = get_narrower (op0, &unsigned0);
+  arg1 = get_narrower (op1, &unsigned1);
+
+  /* UNS is 1 if the operation to be done is an unsigned one.  */
+  uns = TYPE_UNSIGNED (result_type);
+
+  /* Handle the case that OP0 (or OP1) does not *contain* a conversion
+     but it *requires* conversion to FINAL_TYPE.  */
+  
+  if ((TYPE_PRECISION (TREE_TYPE (op0))
+       == TYPE_PRECISION (TREE_TYPE (arg0)))
+      && TREE_TYPE (op0) != result_type)
+    unsigned0 = TYPE_UNSIGNED (TREE_TYPE (op0));
+  if ((TYPE_PRECISION (TREE_TYPE (op1))
+       == TYPE_PRECISION (TREE_TYPE (arg1)))
+      && TREE_TYPE (op1) != result_type)
+    unsigned1 = TYPE_UNSIGNED (TREE_TYPE (op1));
+  
+  /* Now UNSIGNED0 is 1 if ARG0 zero-extends to FINAL_TYPE.  */
+  
+  /* For bitwise operations, signedness of nominal type
+     does not matter.  Consider only how operands were extended.  */
+  if (bitwise)
+    uns = unsigned0;
+  
+  /* Note that in all three cases below we refrain from optimizing
+     an unsigned operation on sign-extended args.
+     That would not be valid.  */
+  
+  /* Both args variable: if both extended in same way
+     from same width, do it in that width.
+     Do it unsigned if args were zero-extended.  */
+  if ((TYPE_PRECISION (TREE_TYPE (arg0))
+       < TYPE_PRECISION (result_type))
+      && (TYPE_PRECISION (TREE_TYPE (arg1))
+         == TYPE_PRECISION (TREE_TYPE (arg0)))
+      && unsigned0 == unsigned1
+      && (unsigned0 || !uns))
+    return c_common_signed_or_unsigned_type
+      (unsigned0, common_type (TREE_TYPE (arg0), TREE_TYPE (arg1)));
+
+  else if (TREE_CODE (arg0) == INTEGER_CST
+          && (unsigned1 || !uns)
+          && (TYPE_PRECISION (TREE_TYPE (arg1))
+              < TYPE_PRECISION (result_type))
+          && (type
+              = c_common_signed_or_unsigned_type (unsigned1,
+                                                  TREE_TYPE (arg1)))
+          && !POINTER_TYPE_P (type)
+          && int_fits_type_p (arg0, type))
+    return type;
+
+  else if (TREE_CODE (arg1) == INTEGER_CST
+          && (unsigned0 || !uns)
+          && (TYPE_PRECISION (TREE_TYPE (arg0))
+              < TYPE_PRECISION (result_type))
+          && (type
+              = c_common_signed_or_unsigned_type (unsigned0,
+                                                  TREE_TYPE (arg0)))
+          && !POINTER_TYPE_P (type)
+          && int_fits_type_p (arg1, type))
+    return type;
+
+  return result_type;
+}
+
 /* Warns if the conversion of EXPR to TYPE may alter a value.
    This is a helper function for warnings_for_convert_and_check.  */
 
@@ -1511,42 +1615,73 @@ conversion_warning (tree type, tree expr)
     }
   else /* 'expr' is not a constant.  */
     {
+      tree expr_type = TREE_TYPE (expr);
+
       /* Warn for real types converted to integer types.  */
-      if (TREE_CODE (TREE_TYPE (expr)) == REAL_TYPE
+      if (TREE_CODE (expr_type) == REAL_TYPE
           && TREE_CODE (type) == INTEGER_TYPE)
         give_warning = true;
 
-      else if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE
+      else if (TREE_CODE (expr_type) == INTEGER_TYPE
                && TREE_CODE (type) == INTEGER_TYPE)
         {
          /* Don't warn about unsigned char y = 0xff, x = (int) y;  */
          expr = get_unwidened (expr, 0);
+         expr_type = TREE_TYPE (expr);
 
+         /* Don't warn for short y; short x = ((int)y & 0xff);  */
+         if (TREE_CODE (expr) == BIT_AND_EXPR 
+             || TREE_CODE (expr) == BIT_IOR_EXPR 
+             || TREE_CODE (expr) == BIT_XOR_EXPR)
+           {
+           /* It both args were extended from a shortest type, use
+              that type if that is safe.  */
+             expr_type = shorten_binary_op (expr_type, 
+                                            TREE_OPERAND (expr, 0), 
+                                            TREE_OPERAND (expr, 1), 
+                                            /* bitwise */1);
+
+             /* If one of the operands is a non-negative constant
+                that fits in the target type, then the type of the
+                other operand does not matter. */
+             if (TREE_CODE (expr) == BIT_AND_EXPR)
+               {
+                 tree op0 = TREE_OPERAND (expr, 0);
+                 tree op1 = TREE_OPERAND (expr, 1);
+                 if ((TREE_CODE (op0) == INTEGER_CST
+                      && int_fits_type_p (op0, c_common_signed_type (type))
+                      && int_fits_type_p (op0, c_common_unsigned_type (type)))
+                     || (TREE_CODE (op1) == INTEGER_CST
+                      && int_fits_type_p (op1, c_common_signed_type (type))
+                      && int_fits_type_p (op1, c_common_unsigned_type (type))))
+                   return;
+               }
+           }
           /* Warn for integer types converted to smaller integer types.  */
-          if (formal_prec < TYPE_PRECISION (TREE_TYPE (expr))) 
+          if (formal_prec < TYPE_PRECISION (expr_type)) 
            give_warning = true;
 
          /* When they are the same width but different signedness,
             then the value may change.  */
-         else if ((formal_prec == TYPE_PRECISION (TREE_TYPE (expr))
-                   && TYPE_UNSIGNED (TREE_TYPE (expr)) != TYPE_UNSIGNED (type))
+         else if ((formal_prec == TYPE_PRECISION (expr_type)
+                   && TYPE_UNSIGNED (expr_type) != TYPE_UNSIGNED (type))
                   /* Even when converted to a bigger type, if the type is
                      unsigned but expr is signed, then negative values
                      will be changed.  */
-                  || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (TREE_TYPE (expr))))
+                  || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)))
            warning (OPT_Wsign_conversion,
                     "conversion to %qT from %qT may change the sign of the result",
-                    type, TREE_TYPE (expr));
+                    type, expr_type);
         }
 
       /* Warn for integer types converted to real types if and only if
          all the range of values of the integer type cannot be
          represented by the real type.  */
-      else if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE
+      else if (TREE_CODE (expr_type) == INTEGER_TYPE
                && TREE_CODE (type) == REAL_TYPE)
         {
-          tree type_low_bound = TYPE_MIN_VALUE (TREE_TYPE (expr));
-          tree type_high_bound = TYPE_MAX_VALUE (TREE_TYPE (expr));
+          tree type_low_bound = TYPE_MIN_VALUE (expr_type);
+          tree type_high_bound = TYPE_MAX_VALUE (expr_type);
           REAL_VALUE_TYPE real_low_bound = real_value_from_int_cst (0, type_low_bound);
           REAL_VALUE_TYPE real_high_bound = real_value_from_int_cst (0, type_high_bound);
 
@@ -1556,16 +1691,16 @@ conversion_warning (tree type, tree expr)
         }
 
       /* Warn for real types converted to smaller real types.  */
-      else if (TREE_CODE (TREE_TYPE (expr)) == REAL_TYPE
+      else if (TREE_CODE (expr_type) == REAL_TYPE
                && TREE_CODE (type) == REAL_TYPE
-               && formal_prec < TYPE_PRECISION (TREE_TYPE (expr)))
+               && formal_prec < TYPE_PRECISION (expr_type))
         give_warning = true;
 
 
       if (give_warning)
         warning (OPT_Wconversion,
                  "conversion to %qT from %qT may alter its value",
-                 type, TREE_TYPE (expr));
+                 type, expr_type);
     }
 }
 
index 1ff5d66..f600751 100644 (file)
@@ -743,6 +743,9 @@ extern bool same_scalar_type_ignoring_signedness (tree, tree);
 #define c_sizeof(T)  c_sizeof_or_alignof_type (T, true, 1)
 #define c_alignof(T) c_sizeof_or_alignof_type (T, false, 1)
 
+/* Subroutine of build_binary_op, used for certain operations.  */
+extern tree shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise);
+
 /* Subroutine of build_binary_op, used for comparison operations.
    See if the operands have both been converted from subword integer types
    and, if so, perhaps change them both back to their original type.  */
index 160229a..4756e25 100644 (file)
@@ -8316,93 +8316,9 @@ build_binary_op (enum tree_code code, tree orig_op0, tree orig_op1,
 
       if (shorten && none_complex)
        {
-         int unsigned0, unsigned1;
-         tree arg0, arg1;
-         int uns;
-         tree type;
-
-         /* Cast OP0 and OP1 to RESULT_TYPE.  Doing so prevents
-            excessive narrowing when we call get_narrower below.  For
-            example, suppose that OP0 is of unsigned int extended
-            from signed char and that RESULT_TYPE is long long int.
-            If we explicitly cast OP0 to RESULT_TYPE, OP0 would look
-            like
-
-              (long long int) (unsigned int) signed_char
-
-            which get_narrower would narrow down to
-
-              (unsigned int) signed char
-
-            If we do not cast OP0 first, get_narrower would return
-            signed_char, which is inconsistent with the case of the
-            explicit cast.  */
-         op0 = convert (result_type, op0);
-         op1 = convert (result_type, op1);
-
-         arg0 = get_narrower (op0, &unsigned0);
-         arg1 = get_narrower (op1, &unsigned1);
-
-         /* UNS is 1 if the operation to be done is an unsigned one.  */
-         uns = TYPE_UNSIGNED (result_type);
-
          final_type = result_type;
-
-         /* Handle the case that OP0 (or OP1) does not *contain* a conversion
-            but it *requires* conversion to FINAL_TYPE.  */
-
-         if ((TYPE_PRECISION (TREE_TYPE (op0))
-              == TYPE_PRECISION (TREE_TYPE (arg0)))
-             && TREE_TYPE (op0) != final_type)
-           unsigned0 = TYPE_UNSIGNED (TREE_TYPE (op0));
-         if ((TYPE_PRECISION (TREE_TYPE (op1))
-              == TYPE_PRECISION (TREE_TYPE (arg1)))
-             && TREE_TYPE (op1) != final_type)
-           unsigned1 = TYPE_UNSIGNED (TREE_TYPE (op1));
-
-         /* Now UNSIGNED0 is 1 if ARG0 zero-extends to FINAL_TYPE.  */
-
-         /* For bitwise operations, signedness of nominal type
-            does not matter.  Consider only how operands were extended.  */
-         if (shorten == -1)
-           uns = unsigned0;
-
-         /* Note that in all three cases below we refrain from optimizing
-            an unsigned operation on sign-extended args.
-            That would not be valid.  */
-
-         /* Both args variable: if both extended in same way
-            from same width, do it in that width.
-            Do it unsigned if args were zero-extended.  */
-         if ((TYPE_PRECISION (TREE_TYPE (arg0))
-              < TYPE_PRECISION (result_type))
-             && (TYPE_PRECISION (TREE_TYPE (arg1))
-                 == TYPE_PRECISION (TREE_TYPE (arg0)))
-             && unsigned0 == unsigned1
-             && (unsigned0 || !uns))
-           result_type
-             = c_common_signed_or_unsigned_type
-             (unsigned0, common_type (TREE_TYPE (arg0), TREE_TYPE (arg1)));
-         else if (TREE_CODE (arg0) == INTEGER_CST
-                  && (unsigned1 || !uns)
-                  && (TYPE_PRECISION (TREE_TYPE (arg1))
-                      < TYPE_PRECISION (result_type))
-                  && (type
-                      = c_common_signed_or_unsigned_type (unsigned1,
-                                                          TREE_TYPE (arg1)))
-                  && !POINTER_TYPE_P (type)
-                  && int_fits_type_p (arg0, type))
-           result_type = type;
-         else if (TREE_CODE (arg1) == INTEGER_CST
-                  && (unsigned0 || !uns)
-                  && (TYPE_PRECISION (TREE_TYPE (arg0))
-                      < TYPE_PRECISION (result_type))
-                  && (type
-                      = c_common_signed_or_unsigned_type (unsigned0,
-                                                          TREE_TYPE (arg0)))
-                  && !POINTER_TYPE_P (type)
-                  && int_fits_type_p (arg1, type))
-           result_type = type;
+         result_type = shorten_binary_op (result_type, op0, op1, 
+                                          shorten == -1);
        }
 
       /* Shifts can be shortened if shifting right.  */
index e05b0cb..0e236b1 100644 (file)
@@ -1,3 +1,9 @@
+2008-07-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
+
+       PR 34389
+       * typeck.c (build_binary_op): Encapsulate code into
+       shorten_binary_op.
+       
 2008-07-29  Jakub Jelinek  <jakub@redhat.com>
 
        PR c++/36852
index fcf52dc..ba1d028 100644 (file)
@@ -3810,61 +3810,9 @@ cp_build_binary_op (enum tree_code code, tree orig_op0, tree orig_op1,
 
       if (shorten && none_complex)
        {
-         int unsigned0, unsigned1;
-         tree arg0 = get_narrower (op0, &unsigned0);
-         tree arg1 = get_narrower (op1, &unsigned1);
-         /* UNS is 1 if the operation to be done is an unsigned one.  */
-         int uns = TYPE_UNSIGNED (result_type);
-         tree type;
-
          final_type = result_type;
-
-         /* Handle the case that OP0 does not *contain* a conversion
-            but it *requires* conversion to FINAL_TYPE.  */
-
-         if (op0 == arg0 && TREE_TYPE (op0) != final_type)
-           unsigned0 = TYPE_UNSIGNED (TREE_TYPE (op0));
-         if (op1 == arg1 && TREE_TYPE (op1) != final_type)
-           unsigned1 = TYPE_UNSIGNED (TREE_TYPE (op1));
-
-         /* Now UNSIGNED0 is 1 if ARG0 zero-extends to FINAL_TYPE.  */
-
-         /* For bitwise operations, signedness of nominal type
-            does not matter.  Consider only how operands were extended.  */
-         if (shorten == -1)
-           uns = unsigned0;
-
-         /* Note that in all three cases below we refrain from optimizing
-            an unsigned operation on sign-extended args.
-            That would not be valid.  */
-
-         /* Both args variable: if both extended in same way
-            from same width, do it in that width.
-            Do it unsigned if args were zero-extended.  */
-         if ((TYPE_PRECISION (TREE_TYPE (arg0))
-              < TYPE_PRECISION (result_type))
-             && (TYPE_PRECISION (TREE_TYPE (arg1))
-                 == TYPE_PRECISION (TREE_TYPE (arg0)))
-             && unsigned0 == unsigned1
-             && (unsigned0 || !uns))
-           result_type = c_common_signed_or_unsigned_type
-             (unsigned0, common_type (TREE_TYPE (arg0), TREE_TYPE (arg1)));
-         else if (TREE_CODE (arg0) == INTEGER_CST
-                  && (unsigned1 || !uns)
-                  && (TYPE_PRECISION (TREE_TYPE (arg1))
-                      < TYPE_PRECISION (result_type))
-                  && (type = c_common_signed_or_unsigned_type
-                      (unsigned1, TREE_TYPE (arg1)),
-                      int_fits_type_p (arg0, type)))
-           result_type = type;
-         else if (TREE_CODE (arg1) == INTEGER_CST
-                  && (unsigned0 || !uns)
-                  && (TYPE_PRECISION (TREE_TYPE (arg0))
-                      < TYPE_PRECISION (result_type))
-                  && (type = c_common_signed_or_unsigned_type
-                      (unsigned0, TREE_TYPE (arg0)),
-                      int_fits_type_p (arg1, type)))
-           result_type = type;
+         result_type = shorten_binary_op (result_type, op0, op1, 
+                                          shorten == -1);
        }
 
       /* Comparison operations are shortened too but differently.
index 93b95f1..46820ff 100644 (file)
@@ -1,3 +1,9 @@
+2008-07-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
+
+       PR 34389
+       * gcc.dg/Wconversion-pr34389.c: New.
+       * g++.dg/warn/Wconversion-pr34389.C: New.
+       
 2008-07-29  Steve Ellcey  <sje@cup.hp.com>
 
        * gcc.dg/pr32370.c: Force 64 bits on IA64.
diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C b/gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C
new file mode 100644 (file)
index 0000000..f3cd310
--- /dev/null
@@ -0,0 +1,53 @@
+/* PR 34389 */
+/* { dg-do compile } */
+/* { dg-options "-Wconversion -Wsign-conversion" } */
+
+short  mask1(short x)
+{
+  short y = 0x7fff;
+  return x & y; /* { dg-bogus "conversion" "conversion" { xfail *-*-* } 8 } */
+}
+
+short  mask2(short ssx)
+{
+  short ssy;
+  short ssz;
+
+  ssz = ((int)ssx) & 0x7fff;
+  ssy = ((int)ssx) | 0x7fff;
+  ssz = ((int)ssx) ^ 0x7fff;
+  return ssx & 0x7fff;
+}
+
+short  mask3(int si, unsigned int ui)
+{
+  short ss;
+  unsigned short us;
+
+  ss = si & 0x7fff;
+  ss = si & 0xAAAA; /* { dg-warning "conversion" } */
+  ss = ui & 0x7fff;
+  ss = ui & 0xAAAA; /* { dg-warning "conversion" } */
+
+  us = si & 0x7fff;
+  us = si & 0xAAAA; /* { dg-warning "conversion" } */
+  us = ui & 0x7fff;
+  us = ui & 0xAAAA; /* { dg-warning "conversion" } */
+
+  return ss;
+}
+
+short  mask4(int x, int y)
+{
+  return x & y; /* { dg-warning "conversion" } */
+}
+
+short  mask5(int x)
+{
+  return x & -1; /* { dg-warning "conversion" } */
+}
+
+short  mask6(short x)
+{
+  return x & -1;
+}
diff --git a/gcc/testsuite/gcc.dg/Wconversion-pr34389.c b/gcc/testsuite/gcc.dg/Wconversion-pr34389.c
new file mode 100644 (file)
index 0000000..45cdf19
--- /dev/null
@@ -0,0 +1,53 @@
+/* PR 34389 */
+/* { dg-do compile } */
+/* { dg-options "-Wconversion -Wsign-conversion" } */
+
+short  mask1(short x)
+{
+  short y = 0x7fff;
+  return x & y;
+}
+
+short  mask2(short ssx)
+{
+  short ssy;
+  short ssz;
+
+  ssz = ((int)ssx) & 0x7fff;
+  ssy = ((int)ssx) | 0x7fff;
+  ssz = ((int)ssx) ^ 0x7fff;
+  return ssx & 0x7fff;
+}
+
+short  mask3(int si, unsigned int ui)
+{
+  short ss;
+  unsigned short us;
+
+  ss = si & 0x7fff;
+  ss = si & 0xAAAA; /* { dg-warning "conversion" } */
+  ss = ui & 0x7fff;
+  ss = ui & 0xAAAA; /* { dg-warning "conversion" } */
+
+  us = si & 0x7fff;
+  us = si & 0xAAAA; /* { dg-warning "conversion" } */
+  us = ui & 0x7fff;
+  us = ui & 0xAAAA; /* { dg-warning "conversion" } */
+
+  return ss;
+}
+
+short  mask4(int x, int y)
+{
+  return x & y; /* { dg-warning "conversion" } */
+}
+
+short  mask5(int x)
+{
+  return x & -1; /* { dg-warning "conversion" } */
+}
+
+short  mask6(short x)
+{
+  return x & -1;
+}