diff mbox

C++ PATCH for c++/59659 (compile-hog with list-initialization of member array)

Message ID 52E29915.1090704@redhat.com
State New
Headers show

Commit Message

Jason Merrill Jan. 24, 2014, 4:47 p.m. UTC 
On 01/15/2014 02:08 PM, Jason Merrill wrote:
> 1) Because atomics do have user-provided constructors, we were going
> down the constructor path and not realizing that it just gave us
> zero-initialization.  Fixed by calling maybe_constant_value and avoiding
> adding extra zero-initialization to the end of the CONSTRUCTOR.
>
> 2) If we aren't dealing with zero-initialization, when we
> value-initialize a bunch of elements at the end of an array, we really
> ought to use a loop rather than initializing them individually.  Fixed
> by using RANGE_EXPR and build_vec_init.

We really aren't ready to use RANGE_EXPR, as it turns out; a lot of 
places need to be changed to handle it, and this doesn't seem like the 
time in the development cycle for that sort of change.  So I'm reverting 
#2 until the next stage 1, to fix c++/59886.

Comments

Jakub Jelinek Jan. 24, 2014, 5:53 p.m. UTC | #1 
On Fri, Jan 24, 2014 at 11:47:17AM -0500, Jason Merrill wrote:
> On 01/15/2014 02:08 PM, Jason Merrill wrote:
> >1) Because atomics do have user-provided constructors, we were going
> >down the constructor path and not realizing that it just gave us
> >zero-initialization.  Fixed by calling maybe_constant_value and avoiding
> >adding extra zero-initialization to the end of the CONSTRUCTOR.
> >
> >2) If we aren't dealing with zero-initialization, when we
> >value-initialize a bunch of elements at the end of an array, we really
> >ought to use a loop rather than initializing them individually.  Fixed
> >by using RANGE_EXPR and build_vec_init.
> 
> We really aren't ready to use RANGE_EXPR, as it turns out; a lot of
> places need to be changed to handle it, and this doesn't seem like
> the time in the development cycle for that sort of change.  So I'm
> reverting #2 until the next stage 1, to fix c++/59886.

Wouldn't my patch then be useful short-term partial fix for 4.9 and
we could revert it when RANGE_EXPR are committed again to 5.0?

	Jakub
Jason Merrill Jan. 24, 2014, 7:01 p.m. UTC | #2 
On 01/24/2014 12:53 PM, Jakub Jelinek wrote:
> Wouldn't my patch then be useful short-term partial fix for 4.9 and
> we could revert it when RANGE_EXPR are committed again to 5.0?

Your patch helps your testcase, but not the original testcase or my 
reduced testcase, so I'm not sure it's worth the risk at this point.

Jason
diff mbox

Patch

commit c3a3e3bd9f8a4f03cd79b706d8005ca2f4c02cd1
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jan 23 15:51:05 2014 -0500

    	PR c++/59886
    	PR c++/59659
    	* typeck2.c (process_init_constructor_array): Don't create
    	RANGE_EXPR yet.

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 897570a..a3fe2e3 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1201,42 +1201,40 @@  process_init_constructor_array (tree type, tree init,
       flags |= picflag_from_initializer (ce->value);
     }
 
-  /* No more initializers. If the array is unbounded, or we've initialized
-     all the elements, we are done. Otherwise, we must add initializers
-     ourselves.  */
-  if (!unbounded && i < len)
-    {
-      tree next;
+  /* No more initializers. If the array is unbounded, we are done. Otherwise,
+     we must add initializers ourselves.  */
+  if (!unbounded)
+    for (; i < len; ++i)
+      {
+	tree next;
 
-      if (type_build_ctor_call (TREE_TYPE (type)))
-	{
-	  /* If this type needs constructors run for default-initialization,
-	     we can't rely on the back end to do it for us, so make the
-	     initialization explicit by list-initializing from {}.  */
-	  next = build_constructor (init_list_type_node, NULL);
-	  next = massage_init_elt (TREE_TYPE (type), next, complain);
-	  if (initializer_zerop (next))
-	    /* The default zero-initialization is fine for us; don't
-	       add anything to the CONSTRUCTOR.  */
-	    next = NULL_TREE;
-	}
-      else if (!zero_init_p (TREE_TYPE (type)))
-	next = build_zero_init (TREE_TYPE (type),
-				/*nelts=*/NULL_TREE,
-				/*static_storage_p=*/false);
-      else
-	/* The default zero-initialization is fine for us; don't
-	   add anything to the CONSTRUCTOR.  */
-	next = NULL_TREE;
+	if (type_build_ctor_call (TREE_TYPE (type)))
+	  {
+	    /* If this type needs constructors run for default-initialization,
+	       we can't rely on the back end to do it for us, so make the
+	       initialization explicit by list-initializing from {}.  */
+	    next = build_constructor (init_list_type_node, NULL);
+	    next = massage_init_elt (TREE_TYPE (type), next, complain);
+	    if (initializer_zerop (next))
+	      /* The default zero-initialization is fine for us; don't
+		 add anything to the CONSTRUCTOR.  */
+	      next = NULL_TREE;
+	  }
+	else if (!zero_init_p (TREE_TYPE (type)))
+	  next = build_zero_init (TREE_TYPE (type),
+				  /*nelts=*/NULL_TREE,
+				  /*static_storage_p=*/false);
+	else
+	  /* The default zero-initialization is fine for us; don't
+	     add anything to the CONSTRUCTOR.  */
+	  next = NULL_TREE;
 
-      if (next)
-	{
-	  flags |= picflag_from_initializer (next);
-	  tree index = build2 (RANGE_EXPR, sizetype, size_int (i),
-			       size_int (len - 1));
-	  CONSTRUCTOR_APPEND_ELT (v, index, next);
-	}
-    }
+	if (next)
+	  {
+	    flags |= picflag_from_initializer (next);
+	    CONSTRUCTOR_APPEND_ELT (v, size_int (i), next);
+	  }
+      }
 
   CONSTRUCTOR_ELTS (init) = v;
   return flags;
diff --git a/gcc/testsuite/g++.dg/init/aggr10.C b/gcc/testsuite/g++.dg/init/aggr10.C
new file mode 100644
index 0000000..e494e20
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/aggr10.C
@@ -0,0 +1,6 @@ 
+// PR c++/59886
+
+struct A { A (); ~A (); };
+struct B { A b[4]; };
+struct C { B c[5]; };
+const C e = {};