c++: temporary lifetime with aggregate init [PR94041]

In C++98 the lifetime of temporaries in aggregate initialization was
unclear, but C++11 DR201 clarified that only temporaries created for
default-initialization of an array element with no corresponding
initializer-clause are destroyed immediately; all others persist until the
end of the full-expression.

But we never implemented that, and continued treating individual element
initializations as full-expressions, such as in my patch for PR50866 in
r180442.  This blocked my attempted fix for PR66139, which extended the use
of split_nonconstant_init, and thus the bug, to aggregate initialization of
temporaries within an expression.

The longer temporary lifetime creates further EH region overlap problems
like the ones that wrap_temporary_cleanups addresses: in aggr7.C, we start
out with a normal nesting of

A1
 c.b1
   A2
    c.b2
     ...
   ~A2
~A1

where on the way in, throwing from one of the inits will clean up from the
previous inits.  But once c.b2 is initialized, throwing from ~A2 must not
clean up c.b1; instead it needs to clean up c.  So as in build_new_1, we
deal with this by guarding the B cleanups with flags that are cleared once c
is fully constructed; throwing from one of the ~A still hits that region,
but does not call ~B.  And then wrap_temporary_cleanups deals with calling
~C, but we need to tell it not to wrap the subobject cleanups.

The change from expressing the subobject cleanups with CLEANUP_STMT to
TARGET_EXPR was also necessary because we need them to collate with the ~A
in gimplify_cleanup_point_expr; the CLEANUP_STMT representation only worked
with treating subobject initializations as full-expressions.

	PR c++/94041

gcc/cp/ChangeLog:

	* decl.c (check_initializer): Remove obsolete comment.
	(wrap_cleanups_r): Don't wrap CLEANUP_EH_ONLY.
	(initialize_local_var): Change assert to test.
	* typeck2.c (maybe_push_temp_cleanup): New.
	(split_nonconstant_init_1): Use it.
	(split_nonconstant_init): Clear cleanup flags.

gcc/testsuite/ChangeLog:

	* g++.dg/init/aggr7-eh.C: New test.
	* g++.dg/cpp0x/initlist122.C: Also test aggregate variable.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0b71c00..9f759ce 100644 (file)
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7257,11 +7257,6 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
 
       if (init && TREE_CODE (init) != TREE_VEC)
        {
-         /* In aggregate initialization of a variable, each element
-            initialization is a full-expression because there is no
-            enclosing expression.  */
-         gcc_assert (stmts_are_full_exprs_p ());
-
          init_code = store_init_value (decl, init, cleanups, flags);
 
          if (DECL_INITIAL (decl)
@@ -7428,7 +7423,8 @@ wrap_cleanups_r (tree *stmt_p, int *walk_subtrees, void *data)
       tree guard = (tree)data;
       tree tcleanup = TARGET_EXPR_CLEANUP (*stmt_p);
 
-      if (tcleanup && !expr_noexcept_p (tcleanup, tf_none))
+      if (tcleanup && !CLEANUP_EH_ONLY (*stmt_p)
+         && !expr_noexcept_p (tcleanup, tf_none))
        {
          tcleanup = build2 (TRY_CATCH_EXPR, void_type_node, tcleanup, guard);
          /* Tell honor_protect_cleanup_actions to handle this as a separate
@@ -7500,11 +7496,11 @@ initialize_local_var (tree decl, tree init)
     {
       tree rinit = (TREE_CODE (init) == INIT_EXPR
                    ? TREE_OPERAND (init, 1) : NULL_TREE);
-      if (rinit && !TREE_SIDE_EFFECTS (rinit))
+      if (rinit && !TREE_SIDE_EFFECTS (rinit)
+         && TREE_OPERAND (init, 0) == decl)
        {
          /* Stick simple initializers in DECL_INITIAL so that
             -Wno-init-self works (c++/34772).  */
-         gcc_assert (TREE_OPERAND (init, 0) == decl);
          DECL_INITIAL (decl) = rinit;
 
          if (warn_init_self && TYPE_REF_P (type))

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 3d4d35e..54b1d0d 100644 (file)
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -459,13 +459,34 @@ cxx_incomplete_type_error (location_t loc, const_tree value, const_tree type)
 }
 
 \f
+/* We've just initialized subobject SUB; also insert a TARGET_EXPR with an
+   EH-only cleanup for SUB.  Because of EH region nesting issues, we need to
+   make the cleanup conditional on a flag that we will clear once the object is
+   fully initialized, so push a new flag onto FLAGS.  */
+
+static void
+maybe_push_temp_cleanup (tree sub, vec<tree,va_gc> **flags)
+{
+  if (tree cleanup
+      = cxx_maybe_build_cleanup (sub, tf_warning_or_error))
+    {
+      tree tx = get_target_expr (boolean_true_node);
+      tree flag = TARGET_EXPR_SLOT (tx);
+      CLEANUP_EH_ONLY (tx) = true;
+      TARGET_EXPR_CLEANUP (tx) = build3 (COND_EXPR, void_type_node,
+                                        flag, cleanup, void_node);
+      add_stmt (tx);
+      vec_safe_push (*flags, flag);
+    }
+}
+
 /* The recursive part of split_nonconstant_init.  DEST is an lvalue
    expression to which INIT should be assigned.  INIT is a CONSTRUCTOR.
    Return true if the whole of the value was initialized by the
    generated statements.  */
 
 static bool
-split_nonconstant_init_1 (tree dest, tree init, bool nested)
+split_nonconstant_init_1 (tree dest, tree init, bool nested, vec<tree,va_gc> **flags)
 {
   unsigned HOST_WIDE_INT idx, tidx = HOST_WIDE_INT_M1U;
   tree field_index, value;
@@ -501,9 +522,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested)
          if (nested)
            /* Also clean up the whole array if something later in an enclosing
               init-list throws.  */
-           if (tree cleanup = cxx_maybe_build_cleanup (dest,
-                                                       tf_warning_or_error))
-           finish_eh_cleanup (cleanup);
+           maybe_push_temp_cleanup (dest, flags);
          return true;
        }
       /* FALLTHRU */
@@ -533,7 +552,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested)
                sub = build3 (COMPONENT_REF, inner_type, dest, field_index,
                              NULL_TREE);
 
-             if (!split_nonconstant_init_1 (sub, value, true)
+             if (!split_nonconstant_init_1 (sub, value, true, flags)
                      /* For flexible array member with initializer we
                         can't remove the initializer, because only the
                         initializer determines how many elements the
@@ -616,11 +635,8 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested)
                      code = build2 (INIT_EXPR, inner_type, sub, value);
                    }
                  code = build_stmt (input_location, EXPR_STMT, code);
-                 code = maybe_cleanup_point_expr_void (code);
                  add_stmt (code);
-                 if (tree cleanup
-                     = cxx_maybe_build_cleanup (sub, tf_warning_or_error))
-                   finish_eh_cleanup (cleanup);
+                 maybe_push_temp_cleanup (sub, flags);
                }
 
              num_split_elts++;
@@ -687,10 +703,29 @@ split_nonconstant_init (tree dest, tree init)
     init = TARGET_EXPR_INITIAL (init);
   if (TREE_CODE (init) == CONSTRUCTOR)
     {
+      /* Subobject initializers are not full-expressions.  */
+      auto fe = (make_temp_override
+                (current_stmt_tree ()->stmts_are_full_exprs_p, 0));
+
       init = cp_fully_fold_init (init);
       code = push_stmt_list ();
-      if (split_nonconstant_init_1 (dest, init, false))
+
+      /* Collect flags for disabling subobject cleanups once the complete
+        object is fully constructed.  */
+      vec<tree, va_gc> *flags = make_tree_vector ();
+
+      if (split_nonconstant_init_1 (dest, init, false, &flags))
        init = NULL_TREE;
+
+      for (tree f : flags)
+       {
+         /* See maybe_push_temp_cleanup.  */
+         tree d = f;
+         tree i = boolean_false_node;
+         add_stmt (build2 (MODIFY_EXPR, TREE_TYPE (d), d, i));
+       }
+      release_tree_vector (flags);
+
       code = pop_stmt_list (code);
       if (VAR_P (dest) && !is_local_temp (dest))
        {

diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist122.C b/gcc/testsuite/g++.dg/cpp0x/initlist122.C
index 002bc1e..81953a4 100644 (file)
--- a/gcc/testsuite/g++.dg/cpp0x/initlist122.C
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist122.C
@@ -6,11 +6,19 @@ struct Temp { ~Temp() { gone = true; } };
 struct A{ A() {}; A(const Temp&) noexcept {};  };
 struct B{ ~B() {}; };
 struct Pair{ A a; B b; };
-
 void foo(const Pair&) noexcept { if (gone) __builtin_abort(); }
 
+B bar() { if (gone) __builtin_abort(); return {}; }
+
 int main()
 {
-  foo({A(Temp{}), B()});
+  Pair p{A(Temp{}), bar()};
+
+  if (!gone) __builtin_abort ();
+
+  gone = false;
+
+  foo({A(Temp{})});
+
   if (!gone) __builtin_abort ();
 }

diff --git a/gcc/testsuite/g++.dg/init/aggr7-eh.C b/gcc/testsuite/g++.dg/init/aggr7-eh.C
new file mode 100644 (file)
index 0000000..db45e15
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/aggr7-eh.C
@@ -0,0 +1,62 @@
+// PR c++/50866, adjusted
+// { dg-do run }
+
+#if __cplusplus > 201100L
+#define THROWING noexcept(false)
+#else
+#define THROWING
+#endif
+
+extern "C" void abort ();
+
+int a;
+int d = -1;
+struct A {
+  A() { ++a; }
+  A(const A&);
+  ~A() THROWING {
+    --a;
+    if (a == d)
+      throw (short)a;
+  }
+};
+int b;
+int t;
+struct B {
+  B(const char *, const A& = A())
+  {
+    if (b == t)
+      throw b;
+    ++b;
+    if (a != b) abort ();
+  }
+  B(const B&);
+  ~B()
+  {
+    --b;
+  }
+};
+struct C {
+  B b1, b2, b3;
+};
+void f()
+{
+  try
+    {
+      C c = { "a","b","c" };
+      if (a != 0) abort ();
+      if (b != 3) abort ();
+    }
+  catch (int i) { }
+  catch (short s) { }
+  if (a != 0) abort ();
+  if (b != 0) abort ();
+}
+
+int main()
+{
+  for (t = 0; t <= 3; ++t)
+    f();
+  for (d = 0; d <= 2; ++d)
+    f();
+}