[PATCH] Fix -Wsign-compare ICE with complex type (PR c/35430)

[PATCH] Fix -Wsign-compare ICE with complex type (PR c/35430)

[Jakub Jelinek]

Hi!

This is a bug Andrew posted a patch for half a year ago.  At that time
it had to be fixed separately in the C and C++ FEs, for C FE Joseph acked
it, for C++ FE Mark requested that the warning stuff is moved into a
separate function (that's apparently what Manuel did in August, now
warn_for_sign_compare is used by both C and C++ FEs), that basetype
is used instead of undertype (the patch below uses base_type) and
to verify the remaining places where result_type is used around that
spot.  The only such place is when BIT_NOT_EXPR is involved, but
BIT_NOT_EXPR of a host integer would be folded away and ~cplx_expr
is CONJ_EXPR, not BIT_NOT_EXPR.  BTW, I haven't been able to
trigger the last "comparison of promoted ~unsigned with unsigned"
at all even with non-complex types.  Using TYPE_UNSIGNED (base_type)
instead of TYPE_UNSIGNED (result_type) is unnecessary, as complex types copy
the unsigned flag from the component type.

Ok for trunk if this passes bootstrap/regtest?

2008-11-04  Jakub Jelinek  <jakub@redhat.com>
	    Andrew Pinski  <andrew_pinski@playstation.sony.com>

	PR c/35430
	* c-common.c (warn_for_sign_compare): For complex result_type
	use component's type.

	* gcc.dg/pr35430.c: New test.
	* g++.dg/warn/Wsign-compare-2.C: New test.

--- gcc/c-common.c.jj	2008-11-03 17:22:06.000000000 +0100
+++ gcc/c-common.c	2008-11-04 11:09:58.000000000 +0100
@@ -8235,7 +8235,7 @@ warn_for_sign_compare (location_t locati
       && 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)))
+	 != TYPE_MAIN_VARIANT (TREE_TYPE (orig_op1)))
     {
       warning_at (location,
 		  OPT_Wsign_compare, "comparison between types %qT and %qT",
@@ -8252,9 +8252,9 @@ warn_for_sign_compare (location_t locati
     /* OK */;
   else
     {
-      tree sop, uop;
+      tree sop, uop, base_type;
       bool ovf;
-      
+
       if (op0_signed)
         sop = orig_op0, uop = orig_op1;
       else 
@@ -8262,6 +8262,8 @@ warn_for_sign_compare (location_t locati
 
       STRIP_TYPE_NOPS (sop); 
       STRIP_TYPE_NOPS (uop);
+      base_type = (TREE_CODE (result_type) == COMPLEX_TYPE
+		   ? TREE_TYPE (result_type) : result_type);
 
       /* Do not warn if the signed quantity is an unsuffixed integer
          literal (or some static constant expression involving such
@@ -8274,7 +8276,7 @@ warn_for_sign_compare (location_t locati
          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)))
+	       && int_fits_type_p (uop, c_common_signed_type (base_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
@@ -8282,7 +8284,7 @@ warn_for_sign_compare (location_t locati
       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)))
+				   c_common_signed_type (base_type)))
         /* OK */;
       else 
         warning_at (location,
--- gcc/testsuite/gcc.dg/pr35430.c.jj	2008-11-04 11:41:45.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr35430.c	2008-11-04 11:45:51.000000000 +0100
@@ -0,0 +1,10 @@
+/* PR c/35430 */
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare" } */
+
+void
+foo (__complex__ int i)
+{
+  i == 0u;
+  i == ~1u;	/* { dg-warning "comparison between signed and unsigned integer expressions" } */
+}
--- gcc/testsuite/g++.dg/warn/Wsign-compare-2.C.jj	2008-11-04 11:41:29.000000000 +0100
+++ gcc/testsuite/g++.dg/warn/Wsign-compare-2.C	2008-11-04 11:46:37.000000000 +0100
@@ -0,0 +1,10 @@
+// PR c/35430
+// { dg-do compile }
+// { dg-options "-Wsign-compare" }
+
+void
+foo (__complex__ int i)
+{
+  i == 0u;
+  i == ~1u;	// { dg-warning "comparison between signed and unsigned integer expressions" }
+}

	Jakub

Re: [PATCH] Fix -Wsign-compare ICE with complex type (PR c/35430)

[Diego Novillo]

On Tue, Nov 4, 2008 at 06:07, Jakub Jelinek <jakub@redhat.com> wrote:

> 2008-11-04  Jakub Jelinek  <jakub@redhat.com>
>            Andrew Pinski  <andrew_pinski@playstation.sony.com>
>
>        PR c/35430
>        * c-common.c (warn_for_sign_compare): For complex result_type
>        use component's type.
>
>        * gcc.dg/pr35430.c: New test.
>        * g++.dg/warn/Wsign-compare-2.C: New test.

OK.

Diego.

Re: [PATCH] Fix -Wsign-compare ICE with complex type (PR c/35430)

[Manuel López-Ibáñez]

2008/11/4 Jakub Jelinek <jakub@redhat.com>:
> The only such place is when BIT_NOT_EXPR is involved, but
> BIT_NOT_EXPR of a host integer would be folded away and ~cplx_expr
> is CONJ_EXPR, not BIT_NOT_EXPR.
[...]
> Using TYPE_UNSIGNED (base_type)
> instead of TYPE_UNSIGNED (result_type) is unnecessary, as complex types copy
> the unsigned flag from the component type.

Wouldn't be appropriate to include part of this explanation as
comments in the code?

Cheers,

Manuel.

Re: [PATCH] Fix -Wsign-compare ICE with complex type (PR c/35430)

[Jakub Jelinek]

On Tue, Nov 04, 2008 at 02:14:27PM +0100, Manuel López-Ibáñez wrote:
> 2008/11/4 Jakub Jelinek <jakub@redhat.com>:
> > The only such place is when BIT_NOT_EXPR is involved, but
> > BIT_NOT_EXPR of a host integer would be folded away and ~cplx_expr
> > is CONJ_EXPR, not BIT_NOT_EXPR.
> [...]
> > Using TYPE_UNSIGNED (base_type)
> > instead of TYPE_UNSIGNED (result_type) is unnecessary, as complex types copy
> > the unsigned flag from the component type.
> 
> Wouldn't be appropriate to include part of this explanation as
> comments in the code?

I don't think so.

	Jakub