diff mbox

[4.6,regression] . New error: case label does not reduce

Message ID 4F3B75DE.9030406@st.com
State New
Headers show

Commit Message

Christian Bruel Feb. 15, 2012, 9:07 a.m. UTC 
Hello,

This patch fixes a regression when folding a cast cond expression to a
constant in case label.

The problem came from the removal of the lines in
c-common.c:check_case_value

  /* ??? Can we ever get nops here for a valid case value?  We
     shouldn't for C.  */
  STRIP_TYPE_NOPS (value);

 so the check for INTEGER_CST) now fails.

The NOP is stripped after folding in 'c_fully_fold' and recreated in the
fly even if no change of type is needed (the constant's type was
converted to an integer, same than the case's type).

Removal of this NOP_EXPR if !CAN_HAVE_LOCATION_P fixes the problem.
looks safe from my testing, because the loc is inserted using
'protected_set_expr_location', whereas no loc for a constant case
doesn't seem to introduce new problems with the debugging information.
But in case I also added a few paranoid gcc_assert.

The motivating failing case is added in the attached testsuite part,

The test now compiles and generates the expected error with -pedantic or
-pedantic-error

Bootstrapped/Regression tested on x86

Thanks for any comment, and if OK to go for 4.6 and trunk

Many thanks

Christian

Comments

Joseph Myers Feb. 15, 2012, 5:03 p.m. UTC | #1 
On Wed, 15 Feb 2012, Christian Bruel wrote:

> Removal of this NOP_EXPR if !CAN_HAVE_LOCATION_P fixes the problem.
> looks safe from my testing, because the loc is inserted using
> 'protected_set_expr_location', whereas no loc for a constant case
> doesn't seem to introduce new problems with the debugging information.
> But in case I also added a few paranoid gcc_assert.

Is it safe to set TREE_NO_WARNING without that check?  I'd have thought 
the check was to avoid setting TREE_NO_WARNING on a shared node when 
warnings are to be disabled in only one context where that node is used.
diff mbox

Patch

2010-02-15  Christian Bruel  <christian.bruel@st.com>

	* gcc/c-common.c (c_fully_fold_internal): Don't create a new NOP expr.
	* gcc/fold-const.c (try_move_mult_to_index): assert CAN_HAVE_LOCATION_P.
	(fold_comparison): Likewise.
	* gcc/tree.c (build_case_label): Likewise.


Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 184254)
+++ c-family/c-common.c	(working copy)
@@ -1435,12 +1435,9 @@ 
      have been done by this point, so remove them again.  */
   nowarning |= TREE_NO_WARNING (ret);
   STRIP_TYPE_NOPS (ret);
-  if (nowarning && !TREE_NO_WARNING (ret))
-    {
-      if (!CAN_HAVE_LOCATION_P (ret))
-	ret = build1 (NOP_EXPR, TREE_TYPE (ret), ret);
-      TREE_NO_WARNING (ret) = 1;
-    }
+  if (nowarning)
+    TREE_NO_WARNING (ret) = 1;
+
   if (ret != expr)
     protected_set_expr_location (ret, loc);
   return ret;
Index: tree.c
===================================================================
--- tree.c	(revision 184254)
+++ tree.c	(working copy)
@@ -1678,6 +1678,7 @@ 
   tree t = make_node (CASE_LABEL_EXPR);
 
   TREE_TYPE (t) = void_type_node;
+  gcc_assert (CAN_HAVE_LOCATION_P (t));
   SET_EXPR_LOCATION (t, DECL_SOURCE_LOCATION (label_decl));
 
   CASE_LOW (t) = low_value;
Index: fold-const.c
===================================================================
--- fold-const.c	(revision 184254)
+++ fold-const.c	(working copy)
@@ -6972,6 +6972,7 @@ 
 
   pref = TREE_OPERAND (addr, 0);
   ret = copy_node (pref);
+  gcc_assert (CAN_HAVE_LOCATION_P (ret));
   SET_EXPR_LOCATION (ret, loc);
   pos = ret;
 
@@ -9427,6 +9428,7 @@ 
 	      if (save_p)
 		{
 		  tem = save_expr (build2 (code, type, cval1, cval2));
+		  gcc_assert (CAN_HAVE_LOCATION_P (tem));
 		  SET_EXPR_LOCATION (tem, loc);
 		  return tem;
 		}