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]

Re: [PR c++/20103] failure to gimplify constructors for addressable types


On Mar 11, 2005, Alexandre Oliva <aoliva@redhat.com> wrote:

> gimplify_and_add calls gimplify_stmt for the stmt list, that calls
> calls gimplify_expr, that calls gimplify_decl_expr, that calls
> gimple_add_tmp_var again.

I don't see any benefit in calling gimple_add_tmp_var *after*
gimplifying the initializer, so I moved the call before that, which
enabled me to get the C++-specific compound-literal-expr gimplifier to
not call gimple_add_tmp_var at all.

As for handling compound-literal-expr cleanups, this patch does so by
using target_expr gimplification code, replacing the
compound-literal-expr with a target_expr when the
compound-literal-expr is found to need a cleanup, and immediately
gimplifying that.  This avoids duplicating the code from a number of
static functions in gimplify.c, but I'm open to other approaches, such
as exporting gimple_push_cleanup from gimplify.c.

Tested on x86_64-linux-gnu.  Ok to install?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20103
	* gimplify.c (gimplify_decl_expr): Add temp variable to binding
	before gimplifying its initializer.

Index: gcc/gimplify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.118
diff -u -p -r2.118 gimplify.c
--- gcc/gimplify.c 14 Mar 2005 20:01:40 -0000 2.118
+++ gcc/gimplify.c 17 Mar 2005 07:56:22 -0000
@@ -990,6 +990,15 @@ gimplify_decl_expr (tree *stmt_p)
     {
       tree init = DECL_INITIAL (decl);
 
+      /* This decl isn't mentioned in the enclosing block, so add it
+	 to the list of temps.  FIXME it seems a bit of a kludge to
+	 say that anonymous artificial vars aren't pushed, but
+	 everything else is.  c_gimplify_compound_literal_expr may
+	 have already added this tmp var, so don't do it again in this
+	 case.  */
+      if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
+	gimple_add_tmp_var (decl);
+
       if (!TREE_CONSTANT (DECL_SIZE (decl)))
 	{
 	  /* This is a variable-sized decl.  Simplify its size and mark it
@@ -1043,12 +1052,6 @@ gimplify_decl_expr (tree *stmt_p)
 	       as they may contain a label address.  */
 	    walk_tree (&init, force_labels_r, NULL, NULL);
 	}
-
-      /* This decl isn't mentioned in the enclosing block, so add it to the
-	 list of temps.  FIXME it seems a bit of a kludge to say that
-	 anonymous artificial vars aren't pushed, but everything else is.  */
-      if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
-	gimple_add_tmp_var (decl);
     }
 
   return GS_ALL_DONE;
Index: gcc/cp/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20103
	* cp-tree.h (build_compound_literal): Declare.
	* semantics.c (finish_compound_literal): Move most of the code...
	* tree.c (build_compound_literal): ... here.  New function.
	(lvalue_p_1): Handle COMPOUND_LITERAL_EXPR.
	(stabilize_init): Likewise.
	* pt.c (tsubst_copy_and_build): Likewise.
	* call.c (build_over_call): Likewise.
	* class.c (fixed_type_or_null): Likewise.
	* cp-gimplify.c (cp_gimplify_init_expr): Likewise.
	(cp_gimplify_compound_literal_expr): New.
	(cp_gimplify_expr): Use it.
	* cvt.c (force_rvalue, ocp_convert): Handle COMPOUND_LITERAL_EXPR.
	* typeck.c (build_x_unary_op): Likewise.
	(cxx_mark_addressable): Likewise.
	(maybe_warn_about_returning_address_of_local): Likewise.

Index: gcc/cp/cp-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v
retrieving revision 1.1110
diff -u -p -r1.1110 cp-tree.h
--- gcc/cp/cp-tree.h 14 Mar 2005 14:33:17 -0000 1.1110
+++ gcc/cp/cp-tree.h 17 Mar 2005 07:56:31 -0000
@@ -4219,6 +4219,7 @@ extern tree build_min_nt			(enum tree_co
 extern tree build_min_non_dep			(enum tree_code, tree, ...);
 extern tree build_cplus_new			(tree, tree);
 extern tree get_target_expr			(tree);
+extern tree build_compound_literal		(tree, tree);
 extern tree build_cplus_array_type		(tree, tree);
 extern tree hash_tree_cons			(tree, tree, tree);
 extern tree hash_tree_chain			(tree, tree);
Index: gcc/cp/semantics.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v
retrieving revision 1.464
diff -u -p -r1.464 semantics.c
--- gcc/cp/semantics.c 14 Mar 2005 14:33:34 -0000 1.464
+++ gcc/cp/semantics.c 17 Mar 2005 07:56:33 -0000
@@ -1980,23 +1980,16 @@ finish_compound_literal (tree type, tree
   compound_literal = build_constructor (NULL_TREE, initializer_list);
   /* Mark it as a compound-literal.  */
   TREE_HAS_CONSTRUCTOR (compound_literal) = 1;
+
   if (processing_template_decl)
+    /* This causes template substitution to run digest_init on the
+       CONSTRUCTOR.  */
     TREE_TYPE (compound_literal) = type;
   else
-    {
-      /* Check the initialization.  */
-      compound_literal = digest_init (type, compound_literal, NULL);
-      /* If the TYPE was an array type with an unknown bound, then we can
-	 figure out the dimension now.  For example, something like:
-
-	   `(int []) { 2, 3 }'
-
-	 implies that the array has two elements.  */
-      if (TREE_CODE (type) == ARRAY_TYPE && !COMPLETE_TYPE_P (type))
-	complete_array_type (type, compound_literal, 1);
-    }
+    /* Check the initialization.  */
+    compound_literal = digest_init (type, compound_literal, NULL);
 
-  return compound_literal;
+  return build_compound_literal (type, compound_literal);
 }
 
 /* Return the declaration for the function-name variable indicated by
Index: gcc/cp/tree.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/tree.c,v
retrieving revision 1.427
diff -u -p -r1.427 tree.c
--- gcc/cp/tree.c 21 Feb 2005 23:12:27 -0000 1.427
+++ gcc/cp/tree.c 17 Mar 2005 07:56:35 -0000
@@ -154,6 +154,7 @@ lvalue_p_1 (tree ref,
 			 treat_class_rvalues_as_lvalues);
 
     case TARGET_EXPR:
+    case COMPOUND_LITERAL_EXPR:
       return treat_class_rvalues_as_lvalues ? clk_class : clk_none;
 
     case CALL_EXPR:
@@ -365,6 +366,43 @@ get_target_expr (tree init)
   return build_target_expr_with_type (init, TREE_TYPE (init));
 }
 
+/* Build a COMPOUND_LITERAL_EXPR.  TYPE is the type given in the
+   compound literal, which may be an incomplete array type completed
+   by the initializer; COMPOUND_LITERAL is a CONSTRUCTOR that
+   initializes the compound literal.  */
+
+tree
+build_compound_literal (tree type, tree compound_literal)
+{
+  /* We do not use start_decl here because we have a type, not a declarator;
+     and do not use finish_decl because the decl should be stored inside
+     the COMPOUND_LITERAL_EXPR rather than added elsewhere as a DECL_EXPR.  */
+  tree decl;
+  tree complit;
+  tree stmt;
+
+  if (type == error_mark_node || compound_literal == error_mark_node)
+    return error_mark_node;
+
+  /* If the TYPE was an array type with an unknown bound, then we can
+     figure out the dimension now.  For example, something like:
+
+     `(int []) { 2, 3 }'
+
+     implies that the array has two elements.  */
+  if (!processing_template_decl
+      && TREE_CODE (type) == ARRAY_TYPE && !COMPLETE_TYPE_P (type))
+    complete_array_type (type, compound_literal, 1);
+
+  decl = build_local_temp (type);
+  DECL_INITIAL (decl) = compound_literal;
+
+  stmt = build_stmt (DECL_EXPR, decl);
+  complit = build1 (COMPOUND_LITERAL_EXPR, TREE_TYPE (decl), stmt);
+  TREE_SIDE_EFFECTS (complit) = 1;
+
+  return complit;
+}
 
 static tree
 build_cplus_array_type_1 (tree elt_type, tree index_type)
@@ -2241,6 +2279,8 @@ stabilize_init (tree init, tree *initp)
 	t = TREE_OPERAND (t, 1);
       if (TREE_CODE (t) == TARGET_EXPR)
 	t = TARGET_EXPR_INITIAL (t);
+      if (TREE_CODE (t) == COMPOUND_LITERAL_EXPR)
+	t = DECL_INITIAL (COMPOUND_LITERAL_EXPR_DECL (t));
       if (TREE_CODE (t) == COMPOUND_EXPR)
 	t = expr_last (t);
       if (TREE_CODE (t) == CONSTRUCTOR
Index: gcc/cp/pt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/pt.c,v
retrieving revision 1.984
diff -u -p -r1.984 pt.c
--- gcc/cp/pt.c 14 Mar 2005 14:51:14 -0000 1.984
+++ gcc/cp/pt.c 17 Mar 2005 07:56:42 -0000
@@ -8832,6 +8832,11 @@ tsubst_copy_and_build (tree t, 
       return build_throw
 	(RECUR (TREE_OPERAND (t, 0)));
 
+    case COMPOUND_LITERAL_EXPR:
+      return build_compound_literal
+	(RECUR (TREE_TYPE (COMPOUND_LITERAL_EXPR_DECL (t))),
+	 RECUR (DECL_INITIAL (COMPOUND_LITERAL_EXPR_DECL (t))));
+
     case CONSTRUCTOR:
       {
 	tree r;
Index: gcc/cp/call.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.531
diff -u -p -r1.531 call.c
--- gcc/cp/call.c 24 Feb 2005 21:55:10 -0000 1.531
+++ gcc/cp/call.c 17 Mar 2005 07:56:46 -0000
@@ -4850,12 +4850,14 @@ build_over_call (struct z_candidate *can
          temp or an INIT_EXPR otherwise.  */
       if (integer_zerop (TREE_VALUE (args)))
 	{
-	  if (TREE_CODE (arg) == TARGET_EXPR)
+	  if (TREE_CODE (arg) == TARGET_EXPR
+	      || TREE_CODE (arg) == COMPOUND_LITERAL_EXPR)
 	    return arg;
 	  else if (TYPE_HAS_TRIVIAL_INIT_REF (DECL_CONTEXT (fn)))
 	    return build_target_expr_with_type (arg, DECL_CONTEXT (fn));
 	}
       else if (TREE_CODE (arg) == TARGET_EXPR
+	       || TREE_CODE (arg) == COMPOUND_LITERAL_EXPR
 	       || TYPE_HAS_TRIVIAL_INIT_REF (DECL_CONTEXT (fn)))
 	{
 	  tree to = stabilize_reference
Index: gcc/cp/class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.709
diff -u -p -r1.709 class.c
--- gcc/cp/class.c 7 Mar 2005 23:08:57 -0000 1.709
+++ gcc/cp/class.c 17 Mar 2005 07:56:52 -0000
@@ -5234,6 +5234,7 @@ fixed_type_or_null (tree instance, int* 
 	}
       /* fall through...  */
     case TARGET_EXPR:
+    case COMPOUND_LITERAL_EXPR:
     case PARM_DECL:
     case RESULT_DECL:
       if (IS_AGGR_TYPE (TREE_TYPE (instance)))
Index: gcc/cp/cp-gimplify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cp-gimplify.c,v
retrieving revision 1.17
diff -u -p -r1.17 cp-gimplify.c
--- gcc/cp/cp-gimplify.c 22 Dec 2004 18:00:38 -0000 1.17
+++ gcc/cp/cp-gimplify.c 17 Mar 2005 07:56:52 -0000
@@ -1,6 +1,6 @@
 /* C++-specific tree lowering bits; see also c-gimplify.c and tree-gimple.c.
 
-   Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
+   Copyright (C) 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
    Contributed by Jason Merrill <jason@redhat.com>
 
 This file is part of GCC.
@@ -125,6 +125,8 @@ cp_gimplify_init_expr (tree *expr_p, tre
     from = TARGET_EXPR_INITIAL (from);
   if (TREE_CODE (from) == CLEANUP_POINT_EXPR)
     from = TREE_OPERAND (from, 0);
+  if (TREE_CODE (from) == COMPOUND_LITERAL_EXPR)
+    from = DECL_INITIAL (COMPOUND_LITERAL_EXPR_DECL (from));
 
   /* Look through any COMPOUND_EXPRs.  */
   sub = expr_last (from);
@@ -170,6 +172,32 @@ gimplify_must_not_throw_expr (tree *expr
     *expr_p = stmt;
 }
 
+/* Gimplify a compound literal expression.  This just means adding the
+   DECL_EXPR before the current EXPR_STMT and using its anonymous decl
+   instead, unless cleanups are needed.  */
+
+static enum gimplify_status
+cp_gimplify_compound_literal_expr (tree *expr_p, tree *pre_p, tree *post_p)
+{
+  tree decl_s = COMPOUND_LITERAL_EXPR_DECL_STMT (*expr_p);
+  tree decl = DECL_EXPR_DECL (decl_s);
+  tree init = DECL_INITIAL (decl);
+  tree cleanup = cxx_maybe_build_cleanup (decl);
+  
+  if (cleanup)
+    {
+      DECL_INITIAL (decl) = NULL_TREE;
+      *expr_p = build4 (TARGET_EXPR, TREE_TYPE (decl), decl,
+			init, cleanup, NULL_TREE);
+      return gimplify_expr (expr_p, pre_p, post_p, is_gimple_val, fb_rvalue);
+    }
+
+  /* If no cleanups are needed, we can do things in a simpler way.  */
+  gimplify_and_add (decl_s, pre_p);
+  *expr_p = decl;
+  return GS_OK;
+}
+
 /* Do C++-specific gimplification.  Args are as for gimplify_expr.  */
 
 int
@@ -254,6 +282,11 @@ cp_gimplify_expr (tree *expr_p, tree *pr
       ret = GS_OK;
       break;
 
+    case COMPOUND_LITERAL_EXPR:
+      cp_gimplify_compound_literal_expr (expr_p, pre_p, post_p);
+      ret = GS_OK;
+      break;
+
     default:
       ret = c_gimplify_expr (expr_p, pre_p, post_p);
       break;
Index: gcc/cp/cvt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cvt.c,v
retrieving revision 1.180
diff -u -p -r1.180 cvt.c
--- gcc/cp/cvt.c 9 Feb 2005 02:53:37 -0000 1.180
+++ gcc/cp/cvt.c 17 Mar 2005 07:56:53 -0000
@@ -575,7 +575,8 @@ convert_from_reference (tree val)
 tree
 force_rvalue (tree expr)
 {
-  if (IS_AGGR_TYPE (TREE_TYPE (expr)) && TREE_CODE (expr) != TARGET_EXPR)
+  if (IS_AGGR_TYPE (TREE_TYPE (expr)) && TREE_CODE (expr) != TARGET_EXPR
+      && TREE_CODE (expr) != COMPOUND_LITERAL_EXPR)
     expr = ocp_convert (TREE_TYPE (expr), expr,
 			CONV_IMPLICIT|CONV_FORCE_TEMP, LOOKUP_NORMAL);
   else
@@ -640,6 +641,16 @@ ocp_convert (tree type, tree expr, int c
 	  TREE_TYPE (e) = TREE_TYPE (TARGET_EXPR_SLOT (e)) = type;
 	  return e;
 	}
+      else if (TREE_CODE (e) == COMPOUND_LITERAL_EXPR)
+	{
+	  /* Don't build a NOP_EXPR of class type.  Instead, change the
+	     type of the temporary.  Only allow this for cv-qual changes,
+	     though.  */
+	  gcc_assert (same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (e)),
+				   TYPE_MAIN_VARIANT (type)));
+	  TREE_TYPE (e) = TREE_TYPE (COMPOUND_LITERAL_EXPR_DECL (e)) = type;
+	  return e;
+	}	  
       else
 	{
 	  /* We shouldn't be treating objects of ADDRESSABLE type as
Index: gcc/cp/typeck.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck.c,v
retrieving revision 1.618
diff -u -p -r1.618 typeck.c
--- gcc/cp/typeck.c 9 Mar 2005 07:28:10 -0000 1.618
+++ gcc/cp/typeck.c 17 Mar 2005 07:56:57 -0000
@@ -3574,7 +3574,8 @@ build_x_unary_op (enum tree_code code, t
 	      PTRMEM_OK_P (xarg) = ptrmem;
 	    }	      
         }
-      else if (TREE_CODE (xarg) == TARGET_EXPR)
+      else if (TREE_CODE (xarg) == TARGET_EXPR
+	       || TREE_CODE (xarg) == COMPOUND_LITERAL_EXPR)
 	warning ("taking address of temporary");
       exp = build_unary_op (ADDR_EXPR, xarg, 0);
     }
@@ -4307,7 +4308,12 @@ cxx_mark_addressable (tree exp)
 
       case TARGET_EXPR:
 	TREE_ADDRESSABLE (x) = 1;
-	cxx_mark_addressable (TREE_OPERAND (x, 0));
+	cxx_mark_addressable (TARGET_EXPR_SLOT (x));
+	return true;
+
+      case COMPOUND_LITERAL_EXPR:
+	TREE_ADDRESSABLE (x) = 1;
+	cxx_mark_addressable (COMPOUND_LITERAL_EXPR_DECL (x));
 	return true;
 
       default:
@@ -5970,7 +5976,8 @@ maybe_warn_about_returning_address_of_lo
   if (TREE_CODE (valtype) == REFERENCE_TYPE)
     {
       if (TREE_CODE (whats_returned) == AGGR_INIT_EXPR
-	  || TREE_CODE (whats_returned) == TARGET_EXPR)
+	  || TREE_CODE (whats_returned) == TARGET_EXPR
+	  || TREE_CODE (whats_returned) == COMPOUND_LITERAL_EXPR)
 	{
 	  warning ("returning reference to temporary");
 	  return;
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20103
	* g++.dg/tree-ssa/pr20103.C: New test.
	* g++.dg/template/complit1.C: Expect error like ext/complit1.C.
	* g++.dg/ext/complit4.C: New test.

Index: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
===================================================================
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20103.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/tree-ssa/pr20103.C 17 Mar 2005 07:57:11 -0000
@@ -0,0 +1,59 @@
+// PR c++/20103
+
+// { dg-do compile }
+
+// { dg-options "" }
+
+// Gimplification used to fail for (B){x}, because create_tmp_var
+// required a non-addressable type, and we didn't wrap the constructor
+// in a target_expr, ensuring it's moved to a separate decl.
+
+// Whether it is an lvalue, like in C, or an rvalue, is up to the ISO
+// C++ Commitete to decide in the second version of the C++ Standard.
+// We're going with rvalues for now.
+
+struct A
+{
+    A(const A&);
+};
+
+struct B
+{
+    A a;
+};
+
+void foo(B);
+void bar(B&); // { dg-error "in passing argument" }
+void bac(const B&);
+void bap(const B*);
+
+void baz(A &x)
+{
+    foo ((B){x});
+    bar ((B){x}); // { dg-error "non-const reference" }
+    bac ((B){x});
+    bap (&(B){x}); // { dg-error "address of temporary" }
+
+    foo ((const B&)(B){x});
+    bar ((B&)(B){x}); // { dg-error "invalid cast" }
+    bac ((const B&)(B){x});
+    bap (&(const B&)(B){x});
+}
+
+template <typename T, typename U>
+void baz(T &x)
+{
+    foo ((U){x});
+    bar ((U){x}); // { dg-error "non-const reference" }
+    bac ((U){x});
+    bap (&(U){x}); // { dg-error "address of temporary" }
+
+    foo ((const U&)(U){x});
+    bar ((U&)(U){x}); // { dg-error "invalid cast" }
+    bac ((const U&)(U){x});
+    bap (&(const U&)(U){x});
+}
+
+void bazT(A &x) {
+  baz<A,B>(x);
+}
Index: gcc/testsuite/g++.dg/template/complit1.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.dg/template/complit1.C,v
retrieving revision 1.3
diff -u -p -r1.3 complit1.C
--- gcc/testsuite/g++.dg/template/complit1.C 28 Dec 2002 07:48:08 -0000 1.3
+++ gcc/testsuite/g++.dg/template/complit1.C 17 Mar 2005 07:57:11 -0000
@@ -6,6 +6,6 @@ template <int D> struct C {
 };
 
 template<int D>
-C<D>::C() : d((int[]){1,2,3}) {}
+C<D>::C() : d((int[]){1,2,3}) {}  // { dg-error "assignment of arrays" }
 
-template class C<1>;
+template class C<1>; // { dg-error "instantiated from" }
Index: gcc/testsuite/g++.dg/ext/complit4.C
===================================================================
RCS file: gcc/testsuite/g++.dg/ext/complit4.C
diff -N gcc/testsuite/g++.dg/ext/complit4.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/ext/complit4.C 17 Mar 2005 07:57:11 -0000
@@ -0,0 +1,21 @@
+// { dg-do compile }
+// { dg-options "" }
+// { dg-final { scan-assembler "_ZN1sD1Ev.*_ZN1sD1Ev.*_ZN1sD1Ev" } }
+
+// Make sure we issue 3 dtor calls: one for the t::x, one for the s
+// temporary in normal execution flow and one for the same s temporary
+// in the EH cleanup should the dtor of t::x throw.
+
+struct s {
+  int i;
+  ~s();
+};
+
+struct t {
+  s x;
+  ~t() {}
+};
+
+void f () {
+  (t){(s){1}};
+}
-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

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