Re: Patch PR c++/40239

[Dodji Seketeli <dodji at redhat dot com>]

On Tue, Jan 12, 2010 at 11:31:42AM -0500, Jason Merrill wrote:
> On 01/12/2010 05:58 AM, Dodji Seketeli wrote:
> >+	      /* direct-initialize the target with the temporary.  */
> >+	      if (TREE_CODE (next) == TARGET_EXPR)
> >+		TARGET_EXPR_DIRECT_INIT_P (next) = true;
> 
> The comment is misleading; setting TARGET_EXPR_DIRECT_INIT_P means
> there isn't a temporary involved.

Agreed. The patch below fixes that.

> I suppose we ought to enforce
> that by checking in cp_gimplify_expr that we don't see any
> TARGET_EXPR with that flag set (because they all get folded away by
> cp_gimplify_init_expr).

During the gimplifying the right hand side of a MODIFY_EXPR it seems
gimplify_init_constructor visits TARGET_EXPRs
(by calling gimplify_init_ctor_preeval and eventually cp_gimplify_expr)
at least once (to turn some values into temporaries). It does so before
actually visiting the INIT_EXPR nodes to fold the TARGET_EXPRs away.
So asserting that no TARGET_EXPR has the flag TARGET_EXPR_DIRECT_INIT_P
set doe not seem to work.

> Otherwise OK.

If it's still okay, I'll queue the patch for 4.6 then.

Thanks.

        Dodji

commit 54bfe138fd82bc53b9020edf3f3e77ec8785dbf5
Author: Dodji Seketeli <dodji@redhat.com>
Date:   Mon Jan 11 16:57:19 2010 +0100

    Fix PR c++/40239
    
    gcc/cp/ChangeLog:
    	* typeck2.c (process_init_constructor_record):
    	value-initialize members that are are not explicitely
    	initialized.
    
    gcc/testsuite/ChangeLog:
    	* gcc/testsuite/g++.dg/init/aggr5.C: New test.
    	* gcc/testsuite/g++.dg/init/aggr5.C: New test.

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 66ff3c1..2bdff96 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1143,12 +1143,18 @@ process_init_constructor_record (tree type, tree init)
       else if (TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (field)))
 	{
 	  /* If this type needs constructors run for
-	     default-initialization, we can't rely on the back end to do it
+	     value-initialization, we can't rely on the back end to do it
 	     for us, so build up TARGET_EXPRs.  If the type in question is
 	     a class, just build one up; if it's an array, recurse.  */
 	  if (MAYBE_CLASS_TYPE_P (TREE_TYPE (field)))
-	    next = build_functional_cast (TREE_TYPE (field), NULL_TREE,
-                                          tf_warning_or_error);
+	    {
+	      next = build_functional_cast (TREE_TYPE (field), NULL_TREE,
+					    tf_warning_or_error);
+	      /* direct-initialize the target. No temporary is going involve
+	         to be involved.  */
+	      if (TREE_CODE (next) == TARGET_EXPR)
+		TARGET_EXPR_DIRECT_INIT_P (next) = true;
+	    }
 	  else
 	    next = build_constructor (init_list_type_node, NULL);
 
diff --git a/gcc/testsuite/g++.dg/init/aggr5.C b/gcc/testsuite/g++.dg/init/aggr5.C
new file mode 100644
index 0000000..2284595
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/aggr5.C
@@ -0,0 +1,11 @@
+// Origin PR c++/40239
+// { dg-do "compile" }
+
+struct B { B() { } private: B(B const&); };
+struct A { int a; B b; };
+
+int
+main()
+{
+  A a = {0};
+}
diff --git a/gcc/testsuite/g++.dg/init/aggr6.C b/gcc/testsuite/g++.dg/init/aggr6.C
new file mode 100644
index 0000000..98628d2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/aggr6.C
@@ -0,0 +1,11 @@
+// Origin PR c++/40239
+// { dg-do compile }
+
+struct B { B() { } private: B(B const&); };
+struct A { int a; B b; };
+
+int
+main()
+{
+  A a = {};
+}