c-decl.c (finish_enum): Simplify code to determine minimum and maximum values of the enum...

2000-01-05 11:25 -0800  Zack Weinberg  <zack@rabi.columbia.edu>

	* c-decl.c (finish_enum): Simplify code to determine minimum and
	maximum values of the enum, and calculate the type.  Remove check
	for FUNCTION_DECLs in the values list, which cannot happen.  Replace
	the DECL_INITIAL of each enumeration constant with a copy converted
	to the enumeration type.  When updating variant types, don't bother
	updating the type itself.

	* c-typeck.c (build_binary_op): Simplify conditional expressions
	when weeding out spurious signed-unsigned warnings.  Add new
	spurious warning category: if the unsigned quantity is an enum
	and its maximum value fits in signed_type(result_type).  Update
	commentary.
	(build_conditional_expr): Warn here if one alternative is signed
	and the other is unsigned.

From-SVN: r31244

4 files changed, 151 insertions, 82 deletions

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 460c08b..4f8b02f 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,20 @@
+2000-01-05 11:25 -0800  Zack Weinberg  <zack@rabi.columbia.edu>
+
+	* c-decl.c (finish_enum): Simplify code to determine minimum and
+	maximum values of the enum, and calculate the type.  Remove check
+	for FUNCTION_DECLs in the values list, which cannot happen.  Replace
+	the DECL_INITIAL of each enumeration constant with a copy converted
+	to the enumeration type.  When updating variant types, don't bother
+	updating the type itself.
+
+	* c-typeck.c (build_binary_op): Simplify conditional expressions
+	when weeding out spurious signed-unsigned warnings.  Add new
+	spurious warning category: if the unsigned quantity is an enum
+	and its maximum value fits in signed_type(result_type).  Update
+	commentary.
+	(build_conditional_expr): Warn here if one alternative is signed
+	and the other is unsigned.
+
 2000-01-05  Nick Clifton  <nickc@cygnus.com>
 
 	* config/fr30/fr30.h: Remove extraneous comments.
diff --git a/gcc/c-decl.c b/gcc/c-decl.c
index 2eba0e4..850ac6d 100644
--- a/gcc/c-decl.c
+++ b/gcc/c-decl.c
@@ -5664,8 +5664,8 @@ finish_enum (enumtype, values, attributes)
 {
   register tree pair, tem;
   tree minnode = 0, maxnode = 0;
-  int lowprec, highprec, precision;
-  int toplevel = global_binding_level == current_binding_level;
+  int precision, unsign;
+  int toplevel = (global_binding_level == current_binding_level);
 
   if (in_parm_level_p ())
     warning ("enum defined inside parms");
@@ -5677,67 +5677,62 @@ finish_enum (enumtype, values, attributes)
   if (values == error_mark_node)
     minnode = maxnode = integer_zero_node;
   else
-    for (pair = values; pair; pair = TREE_CHAIN (pair))
-      {
-	tree value = TREE_VALUE (pair);
-	if (pair == values)
-	  minnode = maxnode = TREE_VALUE (pair);
-	else
-	  {
-	    if (tree_int_cst_lt (maxnode, value))
-	      maxnode = value;
-	    if (tree_int_cst_lt (value, minnode))
-	      minnode = value;
-	  }
-      }
-
-  TYPE_MIN_VALUE (enumtype) = minnode;
-  TYPE_MAX_VALUE (enumtype) = maxnode;
-
-  /* An enum can have some negative values; then it is signed.  */
-  TREE_UNSIGNED (enumtype) = tree_int_cst_sgn (minnode) >= 0;
-
-  /* Determine the precision this type needs.  */
-
-  lowprec = min_precision (minnode, TREE_UNSIGNED (enumtype));
-  highprec = min_precision (maxnode, TREE_UNSIGNED (enumtype));
-  precision = MAX (lowprec, highprec);
-
-  if (TYPE_PACKED (enumtype) || precision > TYPE_PRECISION (integer_type_node))
     {
-      tree narrowest = type_for_size (precision, 1);
-      if (narrowest == 0)
+      minnode = maxnode = TREE_VALUE (values);
+      for (pair = TREE_CHAIN (values); pair; pair = TREE_CHAIN (pair))
 	{
-	  warning ("enumeration values exceed range of largest integer");
-	  narrowest = long_long_integer_type_node;
+	  tree value = TREE_VALUE (pair);
+	  if (tree_int_cst_lt (maxnode, value))
+	    maxnode = value;
+	  if (tree_int_cst_lt (value, minnode))
+	    minnode = value;
 	}
+    }
 
-      TYPE_PRECISION (enumtype) = TYPE_PRECISION (narrowest);
+  /* 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));
+  if (!TYPE_PACKED (enumtype))
+    precision = MAX (precision, TYPE_PRECISION (integer_type_node));
+  if (type_for_size (precision, unsign) == 0)
+    {
+      warning ("enumeration values exceed range of largest integer");
+      precision = TYPE_PRECISION (long_long_integer_type_node);
     }
-  else
-    TYPE_PRECISION (enumtype) = TYPE_PRECISION (integer_type_node);
 
+  TYPE_MIN_VALUE (enumtype) = minnode;
+  TYPE_MAX_VALUE (enumtype) = maxnode;
+  TYPE_PRECISION (enumtype) = precision;
+  TREE_UNSIGNED (enumtype) = unsign;
   TYPE_SIZE (enumtype) = 0;
   layout_type (enumtype);
 
   if (values != error_mark_node)
     {
-      /* Change the type of the enumerators to be the enum type.
-	 Formerly this was done only for enums that fit in an int,
-	 but the comment said it was done only for enums wider than int.
-	 It seems necessary to do this for wide enums,
-	 and best not to change what's done for ordinary narrower ones.  */
+      /* Change the type of the enumerators to be the enum type.  We
+	 need to do this irrespective of the size of the enum, for
+	 proper type checking.  Replace the DECL_INITIALs of the
+	 enumerators, and the value slots of the list, with copies
+	 that have the enum type; they cannot be modified in place
+	 because they may be shared (e.g.  integer_zero_node) Finally,
+	 change the purpose slots to point to the names of the decls.  */
       for (pair = values; pair; pair = TREE_CHAIN (pair))
 	{
-	  TREE_TYPE (TREE_PURPOSE (pair)) = enumtype;
-	  DECL_SIZE (TREE_PURPOSE (pair)) = TYPE_SIZE (enumtype);
-	  if (TREE_CODE (TREE_PURPOSE (pair)) != FUNCTION_DECL)
-	    DECL_ALIGN (TREE_PURPOSE (pair)) = TYPE_ALIGN (enumtype);
-	}
+	  tree enu = TREE_PURPOSE (pair);
 
-      /* Replace the decl nodes in VALUES with their names.  */
-      for (pair = values; pair; pair = TREE_CHAIN (pair))
-	TREE_PURPOSE (pair) = DECL_NAME (TREE_PURPOSE (pair));
+	  TREE_TYPE (enu) = enumtype;
+	  DECL_SIZE (enu) = TYPE_SIZE (enumtype);
+	  DECL_ALIGN (enu) = TYPE_ALIGN (enumtype);
+	  DECL_MODE (enu) = TYPE_MODE (enumtype);
+	  DECL_INITIAL (enu) = convert (enumtype, DECL_INITIAL (enu));
+
+	  TREE_PURPOSE (pair) = DECL_NAME (enu);
+	  TREE_VALUE (pair) = DECL_INITIAL (enu);
+	}
 
       TYPE_VALUES (enumtype) = values;
     }
@@ -5745,6 +5740,8 @@ finish_enum (enumtype, values, attributes)
   /* Fix up all variant types of this enum type.  */
   for (tem = TYPE_MAIN_VARIANT (enumtype); tem; tem = TYPE_NEXT_VARIANT (tem))
     {
+      if (tem == enumtype)
+	continue;
       TYPE_VALUES (tem) = TYPE_VALUES (enumtype);
       TYPE_MIN_VALUE (tem) = TYPE_MIN_VALUE (enumtype);
       TYPE_MAX_VALUE (tem) = TYPE_MAX_VALUE (enumtype);
diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c
index 8b6a0d6..0529d2a 100644
--- a/gcc/c-typeck.c
+++ b/gcc/c-typeck.c
@@ -2390,8 +2390,6 @@ build_binary_op (code, orig_op0, orig_op1, convert_p)
 	      tree primop0 = get_narrower (op0, &unsignedp0);
 	      tree primop1 = get_narrower (op1, &unsignedp1);
 
-	      /* Avoid spurious warnings for comparison with enumerators.  */
- 
 	      xop0 = orig_op0;
 	      xop1 = orig_op1;
 	      STRIP_TYPE_NOPS (xop0);
@@ -2407,28 +2405,41 @@ build_binary_op (code, orig_op0, orig_op1, convert_p)
 		 all the values of the unsigned type.  */
 	      if (! TREE_UNSIGNED (result_type))
 		/* OK */;
-              /* Do not warn if both operands are unsigned.  */
+              /* Do not warn if both operands are the same signedness.  */
               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) and it is non-negative.  */
-	      else if ((op0_signed && TREE_CODE (xop0) == INTEGER_CST
-			&& tree_int_cst_sgn (xop0) >= 0)
-		       || (op1_signed && TREE_CODE (xop1) == INTEGER_CST
-			   && tree_int_cst_sgn (xop1) >= 0))
-		/* 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 (xop1) == INTEGER_CST
-			    && int_fits_type_p (xop1, signed_type (result_type)))
-			   || (op1_signed && TREE_CODE (xop0) == INTEGER_CST
-			       && int_fits_type_p (xop0, signed_type (result_type)))))
-		/* OK */;
 	      else
-		warning ("comparison between signed and unsigned");
+		{
+		  tree sop, uop;
+		  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) and it is non-negative.  */
+		  if (TREE_CODE (sop) == INTEGER_CST
+		      && tree_int_cst_sgn (sop) >= 0)
+		    /* 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, 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)),
+					       signed_type (result_type)))
+		    /* OK */;
+		  else
+		    warning ("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
@@ -3362,6 +3373,37 @@ build_conditional_expr (ifexp, op1, op2)
            && (code2 == INTEGER_TYPE || code2 == REAL_TYPE))
     {
       result_type = common_type (type1, type2);
+
+      /* If -Wsign-compare, warn here if type1 and type2 have
+	 different signedness.  We'll promote the signed to unsigned
+	 and later code won't know it used to be different.
+	 Do this check on the original types, so that explicit casts
+	 will be considered, but default promotions won't.  */
+      if ((warn_sign_compare < 0 ? extra_warnings : warn_sign_compare)
+	  && !skip_evaluation)
+	{
+	  int unsigned_op1 = TREE_UNSIGNED (TREE_TYPE (orig_op1));
+	  int unsigned_op2 = TREE_UNSIGNED (TREE_TYPE (orig_op2));
+
+	  if (unsigned_op1 ^ unsigned_op2)
+	    {
+	      /* Do not warn if the result type is signed, since the
+		 signed type will only be chosen if it can represent
+		 all the values of the unsigned type.  */
+	      if (! TREE_UNSIGNED (result_type))
+		/* OK */;
+	      /* Do not warn if the signed quantity is an unsuffixed
+		 integer literal (or some static constant expression
+		 involving such literals) and it is non-negative.  */
+	      else if ((unsigned_op2 && TREE_CODE (op1) == INTEGER_CST
+			&& tree_int_cst_sgn (op1) >= 0)
+		       || (unsigned_op1 && TREE_CODE (op2) == INTEGER_CST
+			   && tree_int_cst_sgn (op2) >= 0))
+		/* OK */;
+	      else
+		warning ("signed and unsigned type in conditional expression");
+	    }
+	}
     }
   else if (code1 == VOID_TYPE || code2 == VOID_TYPE)
     {
diff --git a/gcc/testsuite/gcc.dg/compare1.c b/gcc/testsuite/gcc.dg/compare1.c
index 3be9d95..44c4df8 100644
--- a/gcc/testsuite/gcc.dg/compare1.c
+++ b/gcc/testsuite/gcc.dg/compare1.c
@@ -4,23 +4,36 @@
 /* { dg-do compile } */
 /* { dg-options "-Wsign-compare" } */
 
-int target_flags = 1;
+int tf = 1;
 
-enum machine_mode 
+/* This enumeration has an explicit negative value and is therefore signed.  */
+enum mm1 
 {
-  VOIDmode , PQImode , QImode , PHImode , HImode ,
-  PSImode , SImode , PDImode , DImode , TImode , OImode , QFmode ,
-  HFmode , TQFmode , SFmode , DFmode , XFmode , TFmode , QCmode ,
-  HCmode , SCmode , DCmode , XCmode , TCmode , CQImode , CHImode ,
-  CSImode , CDImode , CTImode , COImode , BLKmode , CCmode , CCXmode,
-  CC_NOOVmode, CCX_NOOVmode, CCFPmode, CCFPEmode , MAX_MACHINE_MODE 
+  VOID, SI, DI, MAX = -1
 };
 
-#define Pmode ( target_flags ? DImode : SImode )
+/* This enumeration fits entirely in a signed int, but is unsigned anyway.  */
+enum mm2
+{
+  VOID2, SI2, DI2, MAX2
+};
+
+int f(enum mm1 x)
+{
+  return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
+}
 
-int main()
+int g(enum mm1 x)
 {
-  enum machine_mode mode = DImode;
+  return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
+}
 
-  return (mode == Pmode); /* { dg-bogus "warning:" "comparison between signed and unsigned" } */
+int h(enum mm2 x)
+{
+  return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
+}
+
+int i(enum mm2 x)
+{
+  return x == (tf?DI2:-1); /* { dg-warning "signed and unsigned" "case 4" } */
 }