This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PATCH to statement-expression handling for c++/27115


Background: the C++ front end handles class-type COND_EXPR with TARGET_EXPRs both inside and outside the ?:. When they get expanded, the inner ones are folded into the outer one and we end up with both sides being expanded into the same object.

For statement-expressions, the C++ front end would build a TARGET_EXPR around the final expression, then move it to the outside and convert the final expression into an initialization of the temporary. Because the initialization was separated from the TARGET_EXPR, we made the init slot of the TARGET_EXPR void to express that the temporary is initialized in some complicated way.

This didn't work very well with non-trivial cases where we want to initialize some other object with the value of the statement-expression; because the TARGET_EXPR was separated from the initialization, we weren't able to fold away the TARGET_EXPR and its temporary. cp_gimplify_init_expr tried to deal with this by looking into the initializer to find where the temporary is actually initialized, but it only handles certain cases.

I've fixed this by simplifying things: instead of doing the split initialization thing we just leave the value expression as an expression, and improve gimplify_modify_expr_rhs and voidify_wrapper_expr to handle initialization from the various forms of a statement-expression.

One ugly thing about this is that several of the intermediate containers between the value expression and the outside of the statement-expression have void type even though things inside and outside of them have non-void type. This works because voidify_wrapper_expr only cares about the types on the outermost and innermost nodes. It isn't really possible to correct the types while the C++ front end still uses the _STMT nodes, so I think this is acceptable.

Tested x86_64-pc-linux-gnu, applied to trunk.
2006-08-21  Jason Merrill  <jason@redhat.com>

	PR c++/27115
	* gimplify.c (voidify_wrapper_expr): Handle STATEMENT_LIST as a
	wrapper.  Loop to handle nested wrappers.
	(gimplify_bind_expr): Remove temp parameter.
	(gimplify_modify_expr_rhs): Handle CLEANUP_POINT_EXPR, BIND_EXPR
	and STATEMENT_LIST on the rhs.
	(gimplify_statement_list): Voidify the STATEMENT_LIST.
	(gimplify_expr): Pass pre_p to gimplify_statement_list.
	(gimplify_target_expr): Remove special BIND_EXPR handling.
	* cp/semantics.c (finish_stmt_expr_expr): Don't try to voidify here,
	just leave the expression as it is.
	(finish_stmt_expr): If the statement-expression has class type,
	wrap it in a TARGET_EXPR.
	* cp/cp-gimplify.c (cp_gimplify_init_expr): Don't bother with
	CLEANUP_POINT_EXPR.
	* cp/except.c (build_throw): Give the CLEANUP_POINT_EXPR void type.

Index: gimplify.c
===================================================================
*** gimplify.c	(revision 116184)
--- gimplify.c	(working copy)
*************** voidify_wrapper_expr (tree wrapper, tree
*** 957,1027 ****
  {
    if (!VOID_TYPE_P (TREE_TYPE (wrapper)))
      {
!       tree *p, sub = wrapper;
  
!     restart:
!       /* Set p to point to the body of the wrapper.  */
!       switch (TREE_CODE (sub))
! 	{
! 	case BIND_EXPR:
! 	  /* For a BIND_EXPR, the body is operand 1.  */
! 	  p = &BIND_EXPR_BODY (sub);
! 	  break;
! 
! 	default:
! 	  p = &TREE_OPERAND (sub, 0);
! 	  break;
! 	}
! 
!       /* Advance to the last statement.  Set all container types to void.  */
!       if (TREE_CODE (*p) == STATEMENT_LIST)
! 	{
! 	  tree_stmt_iterator i = tsi_last (*p);
! 	  p = tsi_end_p (i) ? NULL : tsi_stmt_ptr (i);
! 	}
!       else
  	{
! 	  for (; TREE_CODE (*p) == COMPOUND_EXPR; p = &TREE_OPERAND (*p, 1))
  	    {
  	      TREE_SIDE_EFFECTS (*p) = 1;
  	      TREE_TYPE (*p) = void_type_node;
  	    }
  	}
  
        if (p == NULL || IS_EMPTY_STMT (*p))
! 	;
!       /* Look through exception handling.  */
!       else if (TREE_CODE (*p) == TRY_FINALLY_EXPR
! 	       || TREE_CODE (*p) == TRY_CATCH_EXPR)
! 	{
! 	  sub = *p;
! 	  goto restart;
! 	}
!       /* The C++ frontend already did this for us.  */
!       else if (TREE_CODE (*p) == INIT_EXPR
! 	       || TREE_CODE (*p) == TARGET_EXPR)
! 	temp = TREE_OPERAND (*p, 0);
!       /* If we're returning a dereference, move the dereference
! 	 outside the wrapper.  */
!       else if (TREE_CODE (*p) == INDIRECT_REF)
! 	{
! 	  tree ptr = TREE_OPERAND (*p, 0);
! 	  temp = create_tmp_var (TREE_TYPE (ptr), "retval");
! 	  *p = build2 (MODIFY_EXPR, TREE_TYPE (ptr), temp, ptr);
! 	  temp = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (temp)), temp);
! 	  /* If this is a BIND_EXPR for a const inline function, it might not
! 	     have TREE_SIDE_EFFECTS set.  That is no longer accurate.  */
! 	  TREE_SIDE_EFFECTS (wrapper) = 1;
  	}
        else
  	{
! 	  if (!temp)
! 	    temp = create_tmp_var (TREE_TYPE (wrapper), "retval");
! 	  *p = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, *p);
! 	  TREE_SIDE_EFFECTS (wrapper) = 1;
  	}
  
-       TREE_TYPE (wrapper) = void_type_node;
        return temp;
      }
  
--- 957,1027 ----
  {
    if (!VOID_TYPE_P (TREE_TYPE (wrapper)))
      {
!       tree type = TREE_TYPE (wrapper);
!       tree *p;
  
!       /* Set p to point to the body of the wrapper.  Loop until we find
! 	 something that isn't a wrapper.  */
!       for (p = &wrapper; p && *p; )
  	{
! 	  switch (TREE_CODE (*p))
  	    {
+ 	    case BIND_EXPR:
+ 	      TREE_SIDE_EFFECTS (*p) = 1;
+ 	      TREE_TYPE (*p) = void_type_node;
+ 	      /* For a BIND_EXPR, the body is operand 1.  */
+ 	      p = &BIND_EXPR_BODY (*p);
+ 	      break;
+ 
+ 	    case CLEANUP_POINT_EXPR:
+ 	    case TRY_FINALLY_EXPR:
+ 	    case TRY_CATCH_EXPR:
  	      TREE_SIDE_EFFECTS (*p) = 1;
  	      TREE_TYPE (*p) = void_type_node;
+ 	      p = &TREE_OPERAND (*p, 0);
+ 	      break;
+ 
+ 	    case STATEMENT_LIST:
+ 	      {
+ 		tree_stmt_iterator i = tsi_last (*p);
+ 		TREE_SIDE_EFFECTS (*p) = 1;
+ 		TREE_TYPE (*p) = void_type_node;
+ 		p = tsi_end_p (i) ? NULL : tsi_stmt_ptr (i);
+ 	      }
+ 	      break;
+ 
+ 	    case COMPOUND_EXPR:
+ 	      /* Advance to the last statement.  Set all container types to void.  */
+ 	      for (; TREE_CODE (*p) == COMPOUND_EXPR; p = &TREE_OPERAND (*p, 1))
+ 		{
+ 		  TREE_SIDE_EFFECTS (*p) = 1;
+ 		  TREE_TYPE (*p) = void_type_node;
+ 		}
+ 	      break;
+ 
+ 	    default:
+ 	      goto out;
  	    }
  	}
  
+     out:
        if (p == NULL || IS_EMPTY_STMT (*p))
! 	temp = NULL_TREE;
!       else if (temp)
! 	{
! 	  /* The wrapper is on the RHS of an assignment that we're pushing
! 	     down.  */
! 	  gcc_assert (TREE_CODE (temp) == INIT_EXPR
! 		      || TREE_CODE (temp) == MODIFY_EXPR);
! 	  TREE_OPERAND (temp, 1) = *p;
! 	  *p = temp;
  	}
        else
  	{
! 	  temp = create_tmp_var (type, "retval");
! 	  *p = build2 (INIT_EXPR, type, temp, *p);
  	}
  
        return temp;
      }
  
*************** build_stack_save_restore (tree *save, tr
*** 1050,1062 ****
  /* Gimplify a BIND_EXPR.  Just voidify and recurse.  */
  
  static enum gimplify_status
! gimplify_bind_expr (tree *expr_p, tree temp, tree *pre_p)
  {
    tree bind_expr = *expr_p;
    bool old_save_stack = gimplify_ctxp->save_stack;
    tree t;
  
!   temp = voidify_wrapper_expr (bind_expr, temp);
  
    /* Mark variables seen in this bind expr.  */
    for (t = BIND_EXPR_VARS (bind_expr); t ; t = TREE_CHAIN (t))
--- 1050,1062 ----
  /* Gimplify a BIND_EXPR.  Just voidify and recurse.  */
  
  static enum gimplify_status
! gimplify_bind_expr (tree *expr_p, tree *pre_p)
  {
    tree bind_expr = *expr_p;
    bool old_save_stack = gimplify_ctxp->save_stack;
    tree t;
  
!   tree temp = voidify_wrapper_expr (bind_expr, NULL);
  
    /* Mark variables seen in this bind expr.  */
    for (t = BIND_EXPR_VARS (bind_expr); t ; t = TREE_CHAIN (t))
*************** gimplify_modify_expr_rhs (tree *expr_p, 
*** 3403,3408 ****
--- 3403,3422 ----
  	ret = GS_UNHANDLED;
  	break;
  
+ 	/* If we're initializing from a container, push the initialization
+ 	   inside it.  */
+       case CLEANUP_POINT_EXPR:
+       case BIND_EXPR:
+       case STATEMENT_LIST:
+ 	{
+ 	  tree wrap = *from_p;
+ 	  tree t = voidify_wrapper_expr (wrap, *expr_p);
+ 	  gcc_assert (t == *expr_p);
+ 
+ 	  *expr_p = wrap;
+ 	  return GS_OK;
+ 	}
+ 	
        default:
  	ret = GS_UNHANDLED;
  	break;
*************** gimplify_compound_expr (tree *expr_p, tr
*** 3676,3683 ****
     enlightened front-end, or by shortcut_cond_expr.  */
  
  static enum gimplify_status
! gimplify_statement_list (tree *expr_p)
  {
    tree_stmt_iterator i = tsi_start (*expr_p);
  
    while (!tsi_end_p (i))
--- 3690,3699 ----
     enlightened front-end, or by shortcut_cond_expr.  */
  
  static enum gimplify_status
! gimplify_statement_list (tree *expr_p, tree *pre_p)
  {
+   tree temp = voidify_wrapper_expr (*expr_p, NULL);
+ 
    tree_stmt_iterator i = tsi_start (*expr_p);
  
    while (!tsi_end_p (i))
*************** gimplify_statement_list (tree *expr_p)
*** 3698,3703 ****
--- 3714,3726 ----
  	tsi_next (&i);
      }
  
+   if (temp)
+     {
+       append_to_statement_list (*expr_p, pre_p);
+       *expr_p = temp;
+       return GS_OK;
+     }
+ 
    return GS_ALL_DONE;
  }
  
*************** gimplify_target_expr (tree *expr_p, tree
*** 4179,4194 ****
  	ret = gimplify_expr (&init, pre_p, post_p, is_gimple_stmt, fb_none);
        else
  	{
!           /* Special handling for BIND_EXPR can result in fewer temps.  */
! 	  ret = GS_OK;
!           if (TREE_CODE (init) == BIND_EXPR)
! 	    gimplify_bind_expr (&init, temp, pre_p);
! 	  if (init != temp)
! 	    {
! 	      init = build2 (INIT_EXPR, void_type_node, temp, init);
! 	      ret = gimplify_expr (&init, pre_p, post_p, is_gimple_stmt,
! 				   fb_none);
! 	    }
  	}
        if (ret == GS_ERROR)
  	return GS_ERROR;
--- 4202,4210 ----
  	ret = gimplify_expr (&init, pre_p, post_p, is_gimple_stmt, fb_none);
        else
  	{
! 	  init = build2 (INIT_EXPR, void_type_node, temp, init);
! 	  ret = gimplify_expr (&init, pre_p, post_p, is_gimple_stmt,
! 			       fb_none);
  	}
        if (ret == GS_ERROR)
  	return GS_ERROR;
*************** gimplify_expr (tree *expr_p, tree *pre_p
*** 5502,5508 ****
  	  break;
  
  	case BIND_EXPR:
! 	  ret = gimplify_bind_expr (expr_p, NULL, pre_p);
  	  break;
  
  	case LOOP_EXPR:
--- 5518,5524 ----
  	  break;
  
  	case BIND_EXPR:
! 	  ret = gimplify_bind_expr (expr_p, pre_p);
  	  break;
  
  	case LOOP_EXPR:
*************** gimplify_expr (tree *expr_p, tree *pre_p
*** 5649,5655 ****
  	  break;
  
  	case STATEMENT_LIST:
! 	  ret = gimplify_statement_list (expr_p);
  	  break;
  
  	case WITH_SIZE_EXPR:
--- 5665,5671 ----
  	  break;
  
  	case STATEMENT_LIST:
! 	  ret = gimplify_statement_list (expr_p, pre_p);
  	  break;
  
  	case WITH_SIZE_EXPR:
Index: cp/except.c
===================================================================
*** cp/except.c	(revision 116184)
--- cp/except.c	(working copy)
*************** build_throw (tree exp)
*** 744,750 ****
        /* Wrap the initialization in a CLEANUP_POINT_EXPR so that cleanups
  	 for temporaries within the initialization are run before the one
  	 for the exception object, preserving LIFO order.  */
!       exp = build1 (CLEANUP_POINT_EXPR, TREE_TYPE (exp), exp);
  
        if (elided)
  	exp = build2 (TRY_CATCH_EXPR, void_type_node, exp,
--- 744,750 ----
        /* Wrap the initialization in a CLEANUP_POINT_EXPR so that cleanups
  	 for temporaries within the initialization are run before the one
  	 for the exception object, preserving LIFO order.  */
!       exp = build1 (CLEANUP_POINT_EXPR, void_type_node, exp);
  
        if (elided)
  	exp = build2 (TRY_CATCH_EXPR, void_type_node, exp,
Index: cp/cp-gimplify.c
===================================================================
*** cp/cp-gimplify.c	(revision 116184)
--- cp/cp-gimplify.c	(working copy)
*************** cp_gimplify_init_expr (tree *expr_p, tre
*** 391,408 ****
    tree to = TREE_OPERAND (*expr_p, 0);
    tree sub;
  
-   /* If we are initializing something from a TARGET_EXPR, strip the
-      TARGET_EXPR and initialize it directly.  */
    /* What about code that pulls out the temp and uses it elsewhere?  I
       think that such code never uses the TARGET_EXPR as an initializer.  If
       I'm wrong, we'll abort because the temp won't have any RTL.  In that
       case, I guess we'll need to replace references somehow.  */
    if (TREE_CODE (from) == TARGET_EXPR)
      from = TARGET_EXPR_INITIAL (from);
-   if (TREE_CODE (from) == CLEANUP_POINT_EXPR)
-     from = TREE_OPERAND (from, 0);
  
!   /* Look through any COMPOUND_EXPRs.  */
    sub = expr_last (from);
  
    /* If we are initializing from an AGGR_INIT_EXPR, drop the INIT_EXPR and
--- 391,405 ----
    tree to = TREE_OPERAND (*expr_p, 0);
    tree sub;
  
    /* What about code that pulls out the temp and uses it elsewhere?  I
       think that such code never uses the TARGET_EXPR as an initializer.  If
       I'm wrong, we'll abort because the temp won't have any RTL.  In that
       case, I guess we'll need to replace references somehow.  */
    if (TREE_CODE (from) == TARGET_EXPR)
      from = TARGET_EXPR_INITIAL (from);
  
!   /* Look through any COMPOUND_EXPRs, since build_compound_expr pushes them
!      inside the TARGET_EXPR.  */
    sub = expr_last (from);
  
    /* If we are initializing from an AGGR_INIT_EXPR, drop the INIT_EXPR and
Index: cp/semantics.c
===================================================================
*** cp/semantics.c	(revision 116184)
--- cp/semantics.c	(working copy)
*************** finish_stmt_expr_expr (tree expr, tree s
*** 1612,1681 ****
       of the last statement is the value of the entire expression.  */
    if (expr)
      {
!       tree type;
!       type = TREE_TYPE (expr);
!       if (!dependent_type_p (type) && !VOID_TYPE_P (type))
  	{
! 	  expr = decay_conversion (expr);
  	  if (error_operand_p (expr))
  	    return error_mark_node;
  	  type = TREE_TYPE (expr);
  	}
        /* The type of the statement-expression is the type of the last
  	 expression.  */
        TREE_TYPE (stmt_expr) = type;
-       /* We must take particular care if TYPE is a class type.  In
- 	 particular if EXPR creates a temporary of class type, then it
- 	 must be destroyed at the semicolon terminating the last
- 	 statement -- but we must make a copy before that happens.
- 
- 	 This problem is solved by using a TARGET_EXPR to initialize a
- 	 new temporary variable.  The TARGET_EXPR itself is placed
- 	 outside the statement-expression.  However, the last
- 	 statement in the statement-expression is transformed from
- 	 EXPR to (approximately) T = EXPR, where T is the new
- 	 temporary variable.  Thus, the lifetime of the new temporary
- 	 extends to the full-expression surrounding the
- 	 statement-expression.  */
-       if (!processing_template_decl && !VOID_TYPE_P (type))
- 	{
- 	  tree target_expr;
- 	  if (CLASS_TYPE_P (type)
- 	      && !TYPE_HAS_TRIVIAL_INIT_REF (type))
- 	    {
- 	      target_expr = build_target_expr_with_type (expr, type);
- 	      expr = TARGET_EXPR_INITIAL (target_expr);
- 	    }
- 	  else
- 	    {
- 	      /* Normally, build_target_expr will not create a
- 		 TARGET_EXPR for scalars.  However, we need the
- 		 temporary here, in order to solve the scoping
- 		 problem described above.  */
- 	      target_expr = force_target_expr (type, expr);
- 	      expr = TARGET_EXPR_INITIAL (target_expr);
- 	      expr = build2 (INIT_EXPR,
- 			     type,
- 			     TARGET_EXPR_SLOT (target_expr),
- 			     expr);
- 	    }
- 	  TARGET_EXPR_INITIAL (target_expr) = NULL_TREE;
- 	  /* Save away the TARGET_EXPR in the TREE_TYPE field of the
- 	     STATEMENT_EXPR.  We will retrieve it in
- 	     finish_stmt_expr.  */
- 	  TREE_TYPE (stmt_expr) = target_expr;
- 	}
      }
  
-   /* Having modified EXPR to reflect the extra initialization, we now
-      treat it just like an ordinary statement.  */
-   expr = finish_expr_stmt (expr);
- 
-   /* Mark the last statement so that we can recognize it as such at
-      template-instantiation time.  */
-   if (expr && processing_template_decl)
-     EXPR_STMT_STMT_EXPR_RESULT (expr) = 1;
- 
    return stmt_expr;
  }
  
--- 1612,1657 ----
       of the last statement is the value of the entire expression.  */
    if (expr)
      {
!       tree type = TREE_TYPE (expr);
! 
!       if (processing_template_decl)
! 	{
! 	  expr = build_stmt (EXPR_STMT, expr);
! 	  expr = add_stmt (expr);
! 	  /* Mark the last statement so that we can recognize it as such at
! 	     template-instantiation time.  */
! 	  EXPR_STMT_STMT_EXPR_RESULT (expr) = 1;
! 	}
!       else if (VOID_TYPE_P (type))
! 	{
! 	  /* Just treat this like an ordinary statement.  */
! 	  expr = finish_expr_stmt (expr);
! 	}
!       else
  	{
! 	  /* It actually has a value we need to deal with.  First, force it
! 	     to be an rvalue so that we won't need to build up a copy
! 	     constructor call later when we try to assign it to something.  */
! 	  expr = force_rvalue (expr);
  	  if (error_operand_p (expr))
  	    return error_mark_node;
+ 
+ 	  /* Update for array-to-pointer decay.  */
  	  type = TREE_TYPE (expr);
+ 
+ 	  /* Wrap it in a CLEANUP_POINT_EXPR and add it to the list like a
+ 	     normal statement, but don't convert to void or actually add
+ 	     the EXPR_STMT.  */
+ 	  if (TREE_CODE (expr) != CLEANUP_POINT_EXPR)
+ 	    expr = maybe_cleanup_point_expr (expr);
+ 	  add_stmt (expr);
  	}
+ 
        /* The type of the statement-expression is the type of the last
  	 expression.  */
        TREE_TYPE (stmt_expr) = type;
      }
  
    return stmt_expr;
  }
  
*************** finish_stmt_expr (tree stmt_expr, bool h
*** 1696,1701 ****
--- 1672,1678 ----
  
    type = TREE_TYPE (stmt_expr);
    result = pop_stmt_list (stmt_expr);
+   TREE_TYPE (result) = type;
  
    if (processing_template_decl)
      {
*************** finish_stmt_expr (tree stmt_expr, bool h
*** 1703,1714 ****
        TREE_SIDE_EFFECTS (result) = 1;
        STMT_EXPR_NO_SCOPE (result) = has_no_scope;
      }
!   else if (!TYPE_P (type))
      {
!       gcc_assert (TREE_CODE (type) == TARGET_EXPR);
!       TARGET_EXPR_INITIAL (type) = result;
!       TREE_TYPE (result) = void_type_node;
!       result = type;
      }
  
    return result;
--- 1680,1692 ----
        TREE_SIDE_EFFECTS (result) = 1;
        STMT_EXPR_NO_SCOPE (result) = has_no_scope;
      }
!   else if (CLASS_TYPE_P (type))
      {
!       /* Wrap the statement-expression in a TARGET_EXPR so that the
! 	 temporary object created by the final expression is destroyed at
! 	 the end of the full-expression containing the
! 	 statement-expression.  */
!       result = force_target_expr (type, result);
      }
  
    return result;
Index: testsuite/g++.dg/ext/stmtexpr8.C
===================================================================
*** testsuite/g++.dg/ext/stmtexpr8.C	(revision 0)
--- testsuite/g++.dg/ext/stmtexpr8.C	(revision 0)
***************
*** 0 ****
--- 1,28 ----
+ // PR c++/27115
+ 
+ // { dg-do run }
+ // { dg-options "" }
+ 
+ struct A
+ {
+   int i;
+   A (int j) : i(j) {}
+   A (const A &j) : i(j.i) {}
+   A& operator= (const A &j) { i = j.i; return *this; }
+ };
+ 
+ A foo(int j)
+ {
+   return ({ j ? A(1) : A(0); });
+ }
+ 
+ int main()
+ {
+   return foo(1).i-1;
+ }
+ 
+ void foo2()
+ {
+   A b = ({ A a(1); a; });
+ }
+ 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]