re PR c++/8715 ('~' operator for unsigned char and conversion to bool)

2008-08-06  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

	PR 8715
	* c-common.c (warn_for_sign_compare): New. Handle separately the
	case that 'constant' is zero.
	* c-typeck.c (build_binary_op): Move code to c-common.c
cp/
	* typeck.c (cp_build_binary_op): Move code to c-common.c.
testsuite/
	* gcc.dg/pr8715.c: New.
	* g++.dg/warn/pr8715.C: New.

From-SVN: r138814

9 files changed, 195 insertions, 226 deletions

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a819795..4722653 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2008-08-06  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
+
+	PR 8715
+	* c-common.c (warn_for_sign_compare): New. Handle separately the
+	case that 'constant' is zero.
+	* c-typeck.c (build_binary_op): Move code to c-common.c
+	
 2008-08-06  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
 
 	* config/alpha/alpha.c (alpha_preferred_reload_class,
diff --git a/gcc/c-common.c b/gcc/c-common.c
index dac29ea..d0ff04b 100644
--- a/gcc/c-common.c
+++ b/gcc/c-common.c
@@ -8202,4 +8202,145 @@ warn_for_div_by_zero (tree divisor)
     warning (OPT_Wdiv_by_zero, "division by zero");
 }
 
+/* Subroutine of build_binary_op. Give warnings for comparisons
+   between signed and unsigned quantities that may fail. Do the
+   checking based on the original operand trees ORIG_OP0 and ORIG_OP1,
+   so that casts will be considered, but default promotions won't
+   be. 
+
+   The arguments of this function map directly to local variables
+   of build_binary_op.  */
+
+void 
+warn_for_sign_compare (tree orig_op0, tree orig_op1, 
+		       tree op0, tree op1, 
+		       tree result_type, enum tree_code resultcode)
+{
+  int op0_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op0));
+  int op1_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op1));
+  int unsignedp0, unsignedp1;
+  
+  /* In C++, check for comparison of different enum types.  */
+  if (c_dialect_cxx()
+      && TREE_CODE (TREE_TYPE (orig_op0)) == ENUMERAL_TYPE
+      && TREE_CODE (TREE_TYPE (orig_op1)) == ENUMERAL_TYPE
+      && TYPE_MAIN_VARIANT (TREE_TYPE (orig_op0))
+      != TYPE_MAIN_VARIANT (TREE_TYPE (orig_op1)))
+    {
+      warning (OPT_Wsign_compare, "comparison between types %qT and %qT",
+               TREE_TYPE (orig_op0), TREE_TYPE (orig_op1));
+    }
+
+  /* Do not warn if the comparison is being done in a signed type,
+     since the signed type will only be chosen if it can represent
+     all the values of the unsigned type.  */
+  if (!TYPE_UNSIGNED (result_type))
+    /* OK */;
+  /* Do not warn if both operands are unsigned.  */
+  else if (op0_signed == op1_signed)
+    /* OK */;
+  else
+    {
+      tree sop, uop;
+      bool ovf;
+      
+      if (op0_signed)
+        sop = orig_op0, uop = orig_op1;
+      else 
+        sop = orig_op1, uop = orig_op0;
+
+      STRIP_TYPE_NOPS (sop); 
+      STRIP_TYPE_NOPS (uop);
+
+      /* Do not warn if the signed quantity is an unsuffixed integer
+         literal (or some static constant expression involving such
+         literals or a conditional expression involving such literals)
+         and it is non-negative.  */
+      if (tree_expr_nonnegative_warnv_p (sop, &ovf))
+        /* OK */;
+      /* Do not warn if the comparison is an equality operation, the
+         unsigned quantity is an integral constant, and it would fit
+         in the result if the result were signed.  */
+      else if (TREE_CODE (uop) == INTEGER_CST
+               && (resultcode == EQ_EXPR || resultcode == NE_EXPR)
+               && int_fits_type_p (uop, c_common_signed_type (result_type)))
+        /* OK */;
+      /* In C, do not warn if the unsigned quantity is an enumeration
+         constant and its maximum value would fit in the result if the
+         result were signed.  */
+      else if (!c_dialect_cxx() && TREE_CODE (uop) == INTEGER_CST
+               && TREE_CODE (TREE_TYPE (uop)) == ENUMERAL_TYPE
+               && int_fits_type_p (TYPE_MAX_VALUE (TREE_TYPE (uop)),
+                                   c_common_signed_type (result_type)))
+        /* OK */;
+      else 
+        warning (OPT_Wsign_compare, 
+                 "comparison between signed and unsigned integer expressions");
+    }
+  
+  /* Warn if two unsigned values are being compared in a size larger
+     than their original size, and one (and only one) is the result of
+     a `~' operator.  This comparison will always fail.
+     
+     Also warn if one operand is a constant, and the constant does not
+     have all bits set that are set in the ~ operand when it is
+     extended.  */
+
+  op0 = get_narrower (op0, &unsignedp0);
+  op1 = get_narrower (op1, &unsignedp1);
+  
+  if ((TREE_CODE (op0) == BIT_NOT_EXPR)
+      ^ (TREE_CODE (op1) == BIT_NOT_EXPR))
+    {
+      if (TREE_CODE (op0) == BIT_NOT_EXPR)
+	op0 = get_narrower (TREE_OPERAND (op0, 0), &unsignedp0);
+      if (TREE_CODE (op1) == BIT_NOT_EXPR)
+	op1 = get_narrower (TREE_OPERAND (op1, 0), &unsignedp1);
+
+      if (host_integerp (op0, 0) || host_integerp (op1, 0))
+        {
+          tree primop;
+          HOST_WIDE_INT constant, mask;
+          int unsignedp;
+          unsigned int bits;
+          
+          if (host_integerp (op0, 0))
+            {
+              primop = op1;
+              unsignedp = unsignedp1;
+              constant = tree_low_cst (op0, 0);
+            }
+          else
+            {
+              primop = op0;
+              unsignedp = unsignedp0;
+              constant = tree_low_cst (op1, 0);
+            }
+          
+          bits = TYPE_PRECISION (TREE_TYPE (primop));
+          if (bits < TYPE_PRECISION (result_type)
+              && bits < HOST_BITS_PER_LONG && unsignedp)
+            {
+              mask = (~ (HOST_WIDE_INT) 0) << bits;
+              if ((mask & constant) != mask)
+		{
+		  if (constant == 0)
+		    warning (OPT_Wsign_compare, 
+			     "promoted ~unsigned is always non-zero");
+		  else
+		    warning (OPT_Wsign_compare, 
+			     "comparison of promoted ~unsigned with constant");
+		}
+            }
+        }
+      else if (unsignedp0 && unsignedp1
+               && (TYPE_PRECISION (TREE_TYPE (op0))
+                   < TYPE_PRECISION (result_type))
+               && (TYPE_PRECISION (TREE_TYPE (op1))
+                   < TYPE_PRECISION (result_type)))
+        warning (OPT_Wsign_compare, 
+                 "comparison of promoted ~unsigned with unsigned");
+    }
+}
+
 #include "gt-c-common.h"
diff --git a/gcc/c-common.h b/gcc/c-common.h
index f600751..0f2a359 100644
--- a/gcc/c-common.h
+++ b/gcc/c-common.h
@@ -928,6 +928,10 @@ extern void warn_about_parentheses (enum tree_code, enum tree_code,
 				    enum tree_code);
 extern void warn_for_unused_label (tree label);
 extern void warn_for_div_by_zero (tree divisor);
+extern void warn_for_sign_compare (tree orig_op0, tree orig_op1, 
+				   tree op0, tree op1, 
+				   tree result_type, 
+				   enum tree_code resultcode);
 
 /* In c-gimplify.c  */
 extern void c_genericize (tree);
diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c
index 4756e25..bacff90 100644
--- a/gcc/c-typeck.c
+++ b/gcc/c-typeck.c
@@ -8372,124 +8372,10 @@ build_binary_op (enum tree_code code, tree orig_op0, tree orig_op1,
 	  converted = 1;
 	  resultcode = xresultcode;
 
-	  if (warn_sign_compare && skip_evaluation == 0)
-	    {
-	      int op0_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op0));
-	      int op1_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op1));
-	      int unsignedp0, unsignedp1;
-	      tree primop0 = get_narrower (op0, &unsignedp0);
-	      tree primop1 = get_narrower (op1, &unsignedp1);
-
-	      xop0 = orig_op0;
-	      xop1 = orig_op1;
-	      STRIP_TYPE_NOPS (xop0);
-	      STRIP_TYPE_NOPS (xop1);
-
-	      /* Give warnings for comparisons between signed and unsigned
-		 quantities that may fail.
-
-		 Do the checking based on the original operand trees, so that
-		 casts will be considered, but default promotions won't be.
-
-		 Do not warn if the comparison is being done in a signed type,
-		 since the signed type will only be chosen if it can represent
-		 all the values of the unsigned type.  */
-	      if (!TYPE_UNSIGNED (result_type))
-		/* OK */;
-	      /* Do not warn if both operands are the same signedness.  */
-	      else if (op0_signed == op1_signed)
-		/* OK */;
-	      else
-		{
-		  tree sop, uop;
-		  bool ovf;
-
-		  if (op0_signed)
-		    sop = xop0, uop = xop1;
-		  else
-		    sop = xop1, uop = xop0;
-
-		  /* Do not warn if the signed quantity is an
-		     unsuffixed integer literal (or some static
-		     constant expression involving such literals or a
-		     conditional expression involving such literals)
-		     and it is non-negative.  */
-		  if (tree_expr_nonnegative_warnv_p (sop, &ovf))
-		    /* OK */;
-		  /* Do not warn if the comparison is an equality operation,
-		     the unsigned quantity is an integral constant, and it
-		     would fit in the result if the result were signed.  */
-		  else if (TREE_CODE (uop) == INTEGER_CST
-			   && (resultcode == EQ_EXPR || resultcode == NE_EXPR)
-			   && int_fits_type_p
-			   (uop, c_common_signed_type (result_type)))
-		    /* OK */;
-		  /* Do not warn if the unsigned quantity is an enumeration
-		     constant and its maximum value would fit in the result
-		     if the result were signed.  */
-		  else if (TREE_CODE (uop) == INTEGER_CST
-			   && TREE_CODE (TREE_TYPE (uop)) == ENUMERAL_TYPE
-			   && int_fits_type_p
-			   (TYPE_MAX_VALUE (TREE_TYPE (uop)),
-			    c_common_signed_type (result_type)))
-		    /* OK */;
-		  else
-		    warning (OPT_Wsign_compare, "comparison between signed and unsigned");
-		}
-
-	      /* Warn if two unsigned values are being compared in a size
-		 larger than their original size, and one (and only one) is the
-		 result of a `~' operator.  This comparison will always fail.
-
-		 Also warn if one operand is a constant, and the constant
-		 does not have all bits set that are set in the ~ operand
-		 when it is extended.  */
-
-	      if ((TREE_CODE (primop0) == BIT_NOT_EXPR)
-		  != (TREE_CODE (primop1) == BIT_NOT_EXPR))
-		{
-		  if (TREE_CODE (primop0) == BIT_NOT_EXPR)
-		    primop0 = get_narrower (TREE_OPERAND (primop0, 0),
-					    &unsignedp0);
-		  else
-		    primop1 = get_narrower (TREE_OPERAND (primop1, 0),
-					    &unsignedp1);
-
-		  if (host_integerp (primop0, 0) || host_integerp (primop1, 0))
-		    {
-		      tree primop;
-		      HOST_WIDE_INT constant, mask;
-		      int unsignedp, bits;
-
-		      if (host_integerp (primop0, 0))
-			{
-			  primop = primop1;
-			  unsignedp = unsignedp1;
-			  constant = tree_low_cst (primop0, 0);
-			}
-		      else
-			{
-			  primop = primop0;
-			  unsignedp = unsignedp0;
-			  constant = tree_low_cst (primop1, 0);
-			}
-
-		      bits = TYPE_PRECISION (TREE_TYPE (primop));
-		      if (bits < TYPE_PRECISION (result_type)
-			  && bits < HOST_BITS_PER_WIDE_INT && unsignedp)
-			{
-			  mask = (~(HOST_WIDE_INT) 0) << bits;
-			  if ((mask & constant) != mask)
-			    warning (OPT_Wsign_compare, "comparison of promoted ~unsigned with constant");
-			}
-		    }
-		  else if (unsignedp0 && unsignedp1
-			   && (TYPE_PRECISION (TREE_TYPE (primop0))
-			       < TYPE_PRECISION (result_type))
-			   && (TYPE_PRECISION (TREE_TYPE (primop1))
-			       < TYPE_PRECISION (result_type)))
-		    warning (OPT_Wsign_compare, "comparison of promoted ~unsigned with unsigned");
-		}
+	  if (warn_sign_compare && !skip_evaluation)
+            {
+              warn_for_sign_compare (orig_op0, orig_op1, op0, op1, 
+                                     result_type, resultcode);
 	    }
 	}
     }
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 8ece21d..c55e4b5 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,8 @@
+2008-08-06  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
+
+	PR 8715
+	* typeck.c (cp_build_binary_op): Move code to c-common.c.
+
 2008-08-05  Jason Merrill  <jason@redhat.com>
 
 	PR c++/37016
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index fd3dba9..792a77c 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -3839,115 +3839,11 @@ cp_build_binary_op (enum tree_code code, tree orig_op0, tree orig_op1,
 	  && warn_sign_compare
 	  /* Do not warn until the template is instantiated; we cannot
 	     bound the ranges of the arguments until that point.  */
-	  && !processing_template_decl)
+	  && !processing_template_decl
+          && (complain & tf_warning))
 	{
-	  int op0_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op0));
-	  int op1_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op1));
-
-	  int unsignedp0, unsignedp1;
-	  tree primop0 = get_narrower (op0, &unsignedp0);
-	  tree primop1 = get_narrower (op1, &unsignedp1);
-
-	  /* Check for comparison of different enum types.  */
-	  if (TREE_CODE (TREE_TYPE (orig_op0)) == ENUMERAL_TYPE
-	      && TREE_CODE (TREE_TYPE (orig_op1)) == ENUMERAL_TYPE
-	      && TYPE_MAIN_VARIANT (TREE_TYPE (orig_op0))
-		 != TYPE_MAIN_VARIANT (TREE_TYPE (orig_op1))
-	      && (complain & tf_warning))
-	    {
-	      warning (OPT_Wsign_compare, "comparison between types %q#T and %q#T",
-		       TREE_TYPE (orig_op0), TREE_TYPE (orig_op1));
-	    }
-
-	  /* Give warnings for comparisons between signed and unsigned
-	     quantities that may fail.  */
-	  /* Do the checking based on the original operand trees, so that
-	     casts will be considered, but default promotions won't be.  */
-
-	  /* Do not warn if the comparison is being done in a signed type,
-	     since the signed type will only be chosen if it can represent
-	     all the values of the unsigned type.  */
-	  if (!TYPE_UNSIGNED (result_type))
-	    /* OK */;
-	  /* Do not warn if both operands are unsigned.  */
-	  else if (op0_signed == op1_signed)
-	    /* OK */;
-	  /* Do not warn if the signed quantity is an unsuffixed
-	     integer literal (or some static constant expression
-	     involving such literals or a conditional expression
-	     involving such literals) and it is non-negative.  */
-	  else if ((op0_signed && tree_expr_nonnegative_p (orig_op0))
-		   || (op1_signed && tree_expr_nonnegative_p (orig_op1)))
-	    /* OK */;
-	  /* Do not warn if the comparison is an equality operation,
-	     the unsigned quantity is an integral constant and it does
-	     not use the most significant bit of result_type.  */
-	  else if ((resultcode == EQ_EXPR || resultcode == NE_EXPR)
-		   && ((op0_signed && TREE_CODE (orig_op1) == INTEGER_CST
-			&& int_fits_type_p (orig_op1, c_common_signed_type
-					    (result_type)))
-			|| (op1_signed && TREE_CODE (orig_op0) == INTEGER_CST
-			    && int_fits_type_p (orig_op0, c_common_signed_type
-						(result_type)))))
-	    /* OK */;
-	  else if (complain & tf_warning)
-	    warning (OPT_Wsign_compare, 
-		     "comparison between signed and unsigned integer expressions");
-
-	  /* Warn if two unsigned values are being compared in a size
-	     larger than their original size, and one (and only one) is the
-	     result of a `~' operator.  This comparison will always fail.
-
-	     Also warn if one operand is a constant, and the constant does not
-	     have all bits set that are set in the ~ operand when it is
-	     extended.  */
-
-	  if ((TREE_CODE (primop0) == BIT_NOT_EXPR)
-	      ^ (TREE_CODE (primop1) == BIT_NOT_EXPR))
-	    {
-	      if (TREE_CODE (primop0) == BIT_NOT_EXPR)
-		primop0 = get_narrower (TREE_OPERAND (op0, 0), &unsignedp0);
-	      if (TREE_CODE (primop1) == BIT_NOT_EXPR)
-		primop1 = get_narrower (TREE_OPERAND (op1, 0), &unsignedp1);
-
-	      if (host_integerp (primop0, 0) || host_integerp (primop1, 0))
-		{
-		  tree primop;
-		  HOST_WIDE_INT constant, mask;
-		  int unsignedp;
-		  unsigned int bits;
-
-		  if (host_integerp (primop0, 0))
-		    {
-		      primop = primop1;
-		      unsignedp = unsignedp1;
-		      constant = tree_low_cst (primop0, 0);
-		    }
-		  else
-		    {
-		      primop = primop0;
-		      unsignedp = unsignedp0;
-		      constant = tree_low_cst (primop1, 0);
-		    }
-
-		  bits = TYPE_PRECISION (TREE_TYPE (primop));
-		  if (bits < TYPE_PRECISION (result_type)
-		      && bits < HOST_BITS_PER_LONG && unsignedp)
-		    {
-		      mask = (~ (HOST_WIDE_INT) 0) << bits;
-		      if ((mask & constant) != mask
-			  && (complain & tf_warning))
-			warning (OPT_Wsign_compare, "comparison of promoted ~unsigned with constant");
-		    }
-		}
-	      else if (unsignedp0 && unsignedp1
-		       && (TYPE_PRECISION (TREE_TYPE (primop0))
-			   < TYPE_PRECISION (result_type))
-		       && (TYPE_PRECISION (TREE_TYPE (primop1))
-			   < TYPE_PRECISION (result_type))
-		       && (complain & tf_warning))
-		warning (OPT_Wsign_compare, "comparison of promoted ~unsigned with unsigned");
-	    }
+            warn_for_sign_compare (orig_op0, orig_op1, op0, op1, 
+                                   result_type, resultcode);
 	}
     }
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a8b0b149..ad00fee 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2008-08-06  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
+
+	PR 8715
+	* gcc.dg/pr8715.c: New.
+	* g++.dg/warn/pr8715.C: New.
+	
 2008-08-06  Marc Gauthier  <marc@tensilica.com>
 
 	* lib/target-supports.exp (check_profiling_available): Match more
diff --git a/gcc/testsuite/g++.dg/warn/pr8715.C b/gcc/testsuite/g++.dg/warn/pr8715.C
new file mode 100644
index 0000000..330c148
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr8715.C
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare" } */
+
+int foo()
+{
+  unsigned char b = '1';
+
+  bool x = ~b; /* { dg-warning "promoted ~unsigned is always non-zero" } */
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/pr8715.c b/gcc/testsuite/gcc.dg/pr8715.c
new file mode 100644
index 0000000..e45e77c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr8715.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare -std=c99" } */
+
+#include <stdbool.h>
+
+int foo()
+{
+  unsigned char b = '1';
+
+  bool x = ~b; /* { dg-warning "promoted ~unsigned is always non-zero" } */
+
+  return 0;
+}