c++: temporary lifetime with array aggr init [PR94041]
authorJason Merrill <jason@redhat.com>
Sat, 1 Jan 2022 21:00:22 +0000 (16:00 -0500)
committerJason Merrill <jason@redhat.com>
Fri, 7 Jan 2022 00:23:17 +0000 (19:23 -0500)
The previous patch fixed temporary lifetime for aggregate initialization of
classes; this one extends that fix to arrays.  This specifically reverses my
r74790, the patch for PR12253, which was made wrong when these semantics
were specified in DR201.

Since the array cleanup region encloses the regions for any temporaries, we
don't need to add an additional region for the array object itself in either
initialize_local_var or split_nonconstant_init; we do, however, need to tell
split_nonconstant_init how to disable the cleanup once an enclosing object
is fully constructed, at which point we want to run that destructor instead.

PR c++/94041

gcc/cp/ChangeLog:

* decl.c (initialize_local_var): Fix comment.
* init.c (build_new_1): Do stabilize array init.
(build_vec_init): Use TARGET_EXPR for cleanup.  Initialization
of an element from an explicit initializer is not a
full-expression.
* tree.c (expand_vec_init_expr): Pass flags through.
* typeck2.c (split_nonconstant_init_1): Handle VEC_INIT_EXPR.
(split_nonconstant_init): Handle array cleanups.
* cp-tree.h: Adjust.

gcc/testsuite/ChangeLog:

* g++.dg/init/array12.C:
* g++.dg/init/aggr7-eh2.C: New test.
* g++.dg/init/aggr7-eh3.C: New test.

gcc/cp/cp-tree.h
gcc/cp/decl.c
gcc/cp/init.c
gcc/cp/tree.c
gcc/cp/typeck2.c
gcc/testsuite/g++.dg/init/aggr7-eh2.C [new file with mode: 0644]
gcc/testsuite/g++.dg/init/aggr7-eh3.C [new file with mode: 0644]
gcc/testsuite/g++.dg/init/array12.C

index 0a3697f..c75ecaf 100644 (file)
@@ -7011,7 +7011,8 @@ extern tree build_new                             (location_t,
                                                 int, tsubst_flags_t);
 extern tree get_temp_regvar                    (tree, tree);
 extern tree build_vec_init                     (tree, tree, tree, bool, int,
-                                                 tsubst_flags_t);
+                                                tsubst_flags_t,
+                                                vec<tree, va_gc> ** = nullptr);
 extern tree build_delete                       (location_t, tree, tree,
                                                 special_function_kind,
                                                 int, int, tsubst_flags_t);
@@ -7779,7 +7780,8 @@ extern bool array_of_runtime_bound_p              (tree);
 extern bool vla_type_p                         (tree);
 extern tree build_array_copy                   (tree);
 extern tree build_vec_init_expr                        (tree, tree, tsubst_flags_t);
-extern tree expand_vec_init_expr               (tree, tree, tsubst_flags_t);
+extern tree expand_vec_init_expr               (tree, tree, tsubst_flags_t,
+                                                vec<tree,va_gc>** = nullptr);
 extern void diagnose_non_constexpr_vec_init    (tree);
 extern tree hash_tree_cons                     (tree, tree, tree);
 extern tree hash_tree_chain                    (tree, tree);
index 9f759ce..b16a4f9 100644 (file)
@@ -7518,8 +7518,7 @@ initialize_local_var (tree decl, tree init)
 
          /* If we're only initializing a single object, guard the
             destructors of any temporaries used in its initializer with
-            its destructor.  This isn't right for arrays because each
-            element initialization is a full-expression.  */
+            its destructor.  But arrays are handled in build_vec_init.  */
          if (cleanup && TREE_CODE (type) != ARRAY_TYPE)
            wrap_temporary_cleanups (init, cleanup);
 
index 2a7dfe2..7c7b810 100644 (file)
@@ -4292,7 +4292,9 @@ finish_length_check (tree atype, tree iterator, tree obase, unsigned n)
 tree
 build_vec_init (tree base, tree maxindex, tree init,
                bool explicit_value_init_p,
-               int from_array, tsubst_flags_t complain)
+               int from_array,
+               tsubst_flags_t complain,
+               vec<tree, va_gc>** flags /* = nullptr */)
 {
   tree rval;
   tree base2 = NULL_TREE;
@@ -4310,7 +4312,6 @@ build_vec_init (tree base, tree maxindex, tree init,
   tree stmt_expr;
   tree compound_stmt;
   int destroy_temps;
-  tree try_block = NULL_TREE;
   HOST_WIDE_INT num_initialized_elts = 0;
   bool is_global;
   tree obase = base;
@@ -4447,7 +4448,9 @@ build_vec_init (tree base, tree maxindex, tree init,
   current_stmt_tree ()->stmts_are_full_exprs_p = 0;
   rval = get_temp_regvar (ptype, base);
   base = get_temp_regvar (ptype, rval);
-  iterator = get_temp_regvar (ptrdiff_type_node, maxindex);
+  tree iterator_targ = get_target_expr (maxindex);
+  add_stmt (iterator_targ);
+  iterator = TARGET_EXPR_SLOT (iterator_targ);
 
   /* If initializing one array from another, initialize element by
      element.  We rely upon the below calls to do the argument
@@ -4470,7 +4473,37 @@ build_vec_init (tree base, tree maxindex, tree init,
   if (flag_exceptions && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)
       && from_array != 2)
     {
-      try_block = begin_try_block ();
+      tree e;
+      tree m = cp_build_binary_op (input_location,
+                                  MINUS_EXPR, maxindex, iterator,
+                                  complain);
+
+      /* Flatten multi-dimensional array since build_vec_delete only
+        expects one-dimensional array.  */
+      if (TREE_CODE (type) == ARRAY_TYPE)
+       m = cp_build_binary_op (input_location,
+                               MULT_EXPR, m,
+                               /* Avoid mixing signed and unsigned.  */
+                               convert (TREE_TYPE (m),
+                                        array_type_nelts_total (type)),
+                               complain);
+
+      e = build_vec_delete_1 (input_location, rval, m,
+                             inner_elt_type, sfk_complete_destructor,
+                             /*use_global_delete=*/0, complain);
+      if (e == error_mark_node)
+       errors = true;
+      TARGET_EXPR_CLEANUP (iterator_targ) = e;
+      CLEANUP_EH_ONLY (iterator_targ) = true;
+
+      /* Since we push this cleanup before doing any initialization, cleanups
+        for any temporaries in the initialization are naturally within our
+        cleanup region, so we don't want wrap_temporary_cleanups to do
+        anything for arrays.  But if the array is a subobject, we need to
+        tell split_nonconstant_init how to turn off this cleanup in favor of
+        the cleanup for the complete object.  */
+      if (flags)
+       vec_safe_push (*flags, build_tree_list (iterator, maxindex));
     }
 
   /* Should we try to create a constant initializer?  */
@@ -4520,11 +4553,10 @@ build_vec_init (tree base, tree maxindex, tree init,
 
          num_initialized_elts++;
 
-         current_stmt_tree ()->stmts_are_full_exprs_p = 1;
          if (digested)
            one_init = build2 (INIT_EXPR, type, baseref, elt);
          else if (TREE_CODE (elt) == VEC_INIT_EXPR)
-           one_init = expand_vec_init_expr (baseref, elt, complain);
+           one_init = expand_vec_init_expr (baseref, elt, complain, flags);
          else if (MAYBE_CLASS_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
            one_init = build_aggr_init (baseref, elt, 0, complain);
          else
@@ -4560,7 +4592,6 @@ build_vec_init (tree base, tree maxindex, tree init,
 
          if (one_init)
            finish_expr_stmt (one_init);
-         current_stmt_tree ()->stmts_are_full_exprs_p = 0;
 
          one_init = cp_build_unary_op (PREINCREMENT_EXPR, base, false,
                                        complain);
@@ -4782,6 +4813,17 @@ build_vec_init (tree base, tree maxindex, tree init,
            }
        }
 
+      /* [class.temporary]: "There are three contexts in which temporaries are
+        destroyed at a different point than the end of the full-
+        expression. The first context is when a default constructor is called
+        to initialize an element of an array with no corresponding
+        initializer. The second context is when a copy constructor is called
+        to copy an element of an array while the entire array is copied. In
+        either case, if the constructor has one or more default arguments, the
+        destruction of every temporary created in a default argument is
+        sequenced before the construction of the next array element, if any."
+
+        So, for this loop, statements are full-expressions.  */
       current_stmt_tree ()->stmts_are_full_exprs_p = 1;
       if (elt_init && !errors)
        elt_init = build2 (COMPOUND_EXPR, void_type_node, elt_init, decr);
@@ -4799,34 +4841,6 @@ build_vec_init (tree base, tree maxindex, tree init,
       finish_for_stmt (for_stmt);
     }
 
-  /* Make sure to cleanup any partially constructed elements.  */
-  if (flag_exceptions && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)
-      && from_array != 2)
-    {
-      tree e;
-      tree m = cp_build_binary_op (input_location,
-                                  MINUS_EXPR, maxindex, iterator,
-                                  complain);
-
-      /* Flatten multi-dimensional array since build_vec_delete only
-        expects one-dimensional array.  */
-      if (TREE_CODE (type) == ARRAY_TYPE)
-       m = cp_build_binary_op (input_location,
-                               MULT_EXPR, m,
-                               /* Avoid mixing signed and unsigned.  */
-                               convert (TREE_TYPE (m),
-                                        array_type_nelts_total (type)),
-                               complain);
-
-      finish_cleanup_try_block (try_block);
-      e = build_vec_delete_1 (input_location, rval, m,
-                             inner_elt_type, sfk_complete_destructor,
-                             /*use_global_delete=*/0, complain);
-      if (e == error_mark_node)
-       errors = true;
-      finish_cleanup (e, try_block);
-    }
-
   /* The value of the array initialization is the array itself, RVAL
      is a pointer to the first element.  */
   finish_stmt_expr_expr (rval, stmt_expr);
index 8bd1964..964e40e 100644 (file)
@@ -824,7 +824,8 @@ build_vec_init_expr (tree type, tree init, tsubst_flags_t complain)
    means VEC_INIT_EXPR_SLOT).  */
 
 tree
-expand_vec_init_expr (tree target, tree vec_init, tsubst_flags_t complain)
+expand_vec_init_expr (tree target, tree vec_init, tsubst_flags_t complain,
+                     vec<tree,va_gc> **flags)
 {
   iloc_sentinel ils = EXPR_LOCATION (vec_init);
 
@@ -834,7 +835,7 @@ expand_vec_init_expr (tree target, tree vec_init, tsubst_flags_t complain)
   int from_array = (init && TREE_CODE (TREE_TYPE (init)) == ARRAY_TYPE);
   return build_vec_init (target, NULL_TREE, init,
                         VEC_INIT_EXPR_VALUE_INIT (vec_init),
-                        from_array, complain);
+                        from_array, complain, flags);
 }
 
 /* Give a helpful diagnostic for a non-constexpr VEC_INIT_EXPR in a context
index 54b1d0d..7907c53 100644 (file)
@@ -486,7 +486,7 @@ maybe_push_temp_cleanup (tree sub, vec<tree,va_gc> **flags)
    generated statements.  */
 
 static bool
-split_nonconstant_init_1 (tree dest, tree init, bool nested, vec<tree,va_gc> **flags)
+split_nonconstant_init_1 (tree dest, tree init, vec<tree,va_gc> **flags)
 {
   unsigned HOST_WIDE_INT idx, tidx = HOST_WIDE_INT_M1U;
   tree field_index, value;
@@ -515,14 +515,10 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested, vec<tree,va_gc> **f
            }
 
          /* For an array, we only need/want a single cleanup region rather
-            than one per element.  */
+            than one per element.  build_vec_init will handle it.  */
          tree code = build_vec_init (dest, NULL_TREE, init, false, 1,
-                                     tf_warning_or_error);
+                                     tf_warning_or_error, flags);
          add_stmt (code);
-         if (nested)
-           /* Also clean up the whole array if something later in an enclosing
-              init-list throws.  */
-           maybe_push_temp_cleanup (dest, flags);
          return true;
        }
       /* FALLTHRU */
@@ -541,18 +537,17 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested, vec<tree,va_gc> **f
          if (!array_type_p)
            inner_type = TREE_TYPE (field_index);
 
+         tree sub;
+         if (array_type_p)
+           sub = build4 (ARRAY_REF, inner_type, dest, field_index,
+                         NULL_TREE, NULL_TREE);
+         else
+           sub = build3 (COMPONENT_REF, inner_type, dest, field_index,
+                         NULL_TREE);
+
          if (TREE_CODE (value) == CONSTRUCTOR)
            {
-             tree sub;
-
-             if (array_type_p)
-               sub = build4 (ARRAY_REF, inner_type, dest, field_index,
-                             NULL_TREE, NULL_TREE);
-             else
-               sub = build3 (COMPONENT_REF, inner_type, dest, field_index,
-                             NULL_TREE);
-
-             if (!split_nonconstant_init_1 (sub, value, true, flags)
+             if (!split_nonconstant_init_1 (sub, value, flags)
                      /* For flexible array member with initializer we
                         can't remove the initializer, because only the
                         initializer determines how many elements the
@@ -576,10 +571,20 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested, vec<tree,va_gc> **f
                  num_split_elts++;
                }
            }
+         else if (TREE_CODE (value) == VEC_INIT_EXPR)
+           {
+             add_stmt (expand_vec_init_expr (sub, value, tf_warning_or_error,
+                                             flags));
+
+             /* Mark element for removal.  */
+             CONSTRUCTOR_ELT (init, idx)->index = NULL_TREE;
+             if (idx < tidx)
+               tidx = idx;
+             num_split_elts++;
+           }
          else if (!initializer_constant_valid_p (value, inner_type))
            {
              tree code;
-             tree sub;
 
              /* Mark element for removal.  */
              CONSTRUCTOR_ELT (init, idx)->index = NULL_TREE;
@@ -603,13 +608,6 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested, vec<tree,va_gc> **f
                }
              else
                {
-                 if (array_type_p)
-                   sub = build4 (ARRAY_REF, inner_type, dest, field_index,
-                                 NULL_TREE, NULL_TREE);
-                 else
-                   sub = build3 (COMPONENT_REF, inner_type, dest, field_index,
-                                 NULL_TREE);
-
                  /* We may need to add a copy constructor call if
                     the field has [[no_unique_address]].  */
                  if (unsafe_return_slot_p (sub))
@@ -710,11 +708,14 @@ split_nonconstant_init (tree dest, tree init)
       init = cp_fully_fold_init (init);
       code = push_stmt_list ();
 
-      /* Collect flags for disabling subobject cleanups once the complete
-        object is fully constructed.  */
-      vec<tree, va_gc> *flags = make_tree_vector ();
+      /* If the complete object is an array, build_vec_init's cleanup is
+        enough.  Otherwise, collect flags for disabling subobject
+        cleanups once the complete object is fully constructed.  */
+      vec<tree, va_gc> *flags = nullptr;
+      if (TREE_CODE (TREE_TYPE (dest)) != ARRAY_TYPE)
+       flags = make_tree_vector ();
 
-      if (split_nonconstant_init_1 (dest, init, false, &flags))
+      if (split_nonconstant_init_1 (dest, init, &flags))
        init = NULL_TREE;
 
       for (tree f : flags)
@@ -722,6 +723,14 @@ split_nonconstant_init (tree dest, tree init)
          /* See maybe_push_temp_cleanup.  */
          tree d = f;
          tree i = boolean_false_node;
+         if (TREE_CODE (f) == TREE_LIST)
+           {
+             /* To disable a build_vec_init cleanup, set
+                iterator = maxindex.  */
+             d = TREE_PURPOSE (f);
+             i = TREE_VALUE (f);
+             ggc_free (f);
+           }
          add_stmt (build2 (MODIFY_EXPR, TREE_TYPE (d), d, i));
        }
       release_tree_vector (flags);
diff --git a/gcc/testsuite/g++.dg/init/aggr7-eh2.C b/gcc/testsuite/g++.dg/init/aggr7-eh2.C
new file mode 100644 (file)
index 0000000..0037b09
--- /dev/null
@@ -0,0 +1,98 @@
+// PR c++/50866, adjusted
+// { dg-do run }
+
+#if __cplusplus > 201100L
+#define THROWING noexcept(false)
+#else
+#define THROWING
+#endif
+
+extern "C" void abort ();
+
+#ifdef DEBUG
+  extern "C" int printf (const char *, ...);
+  #define dump(X,Y) printf(X,Y)
+#define abort() printf("wrong\n");
+
+#else
+  #define dump(X,Y)
+#endif
+
+int a, b;
+int d;
+struct A {
+  int n;
+  A() { n = ++a; dump("A%d\n",a); }
+  A(const A&);
+  ~A() THROWING {
+    dump("~A%d\n",n);
+    --a;
+    if (d == 1 ? a == 0 : (b == d && a == 1))
+      {
+       dump ("~A%d throwing\n", n);
+       throw (short)b;
+      }
+  }
+};
+int t;
+struct B {
+  int n;
+  B(const A& = A())
+  {
+    if (b == t)
+      {
+       dump ("B%d throwing\n", b+1);
+       throw b;
+      }
+    n = ++b;
+    dump("B%d\n",b);
+
+    /* The first B has an explicit initializer, so its A lives for the
+       full-expression.  The second B does not, so its A should be destroyed
+       before we construct the third B.  */
+    if (a != 2) abort ();
+  }
+  B(const char *, const A& = A())
+  {
+    if (b == t)
+      {
+       dump ("B%d throwing\n", b+1);
+       throw b;
+      }
+    n = ++b;
+    dump("B%d\n",b);
+    if (a != b) abort ();
+  }
+  B(const B&);
+  ~B()
+  {
+    dump("~B%d\n",n);
+    --b;
+  }
+};
+struct C {
+  B bs[3];
+};
+void f()
+{
+  a = b = 0;
+  try
+    {
+      C c = { "x" };
+      if (a != 0) abort ();
+      if (b != 3) abort ();
+    }
+  catch (int i) { }
+  catch (short s) { }
+  if (a != 0) abort ();
+  if (b != 0) abort ();
+  dump ("\n", 0);
+}
+
+int main()
+{
+  for (t = 0; t <= 3; ++t)
+    f();
+  for (d = 1; d <= 3; ++d)
+    f();
+}
diff --git a/gcc/testsuite/g++.dg/init/aggr7-eh3.C b/gcc/testsuite/g++.dg/init/aggr7-eh3.C
new file mode 100644 (file)
index 0000000..6ddabec
--- /dev/null
@@ -0,0 +1,98 @@
+// PR c++/50866, adjusted
+// { dg-do run }
+
+#if __cplusplus > 201100L
+#define THROWING noexcept(false)
+#else
+#define THROWING
+#endif
+
+extern "C" void abort ();
+
+#ifdef DEBUG
+  extern "C" int printf (const char *, ...);
+  #define dump(X,Y) printf(X,Y)
+#define abort() printf("wrong\n");
+
+#else
+  #define dump(X,Y)
+#endif
+
+int a, b;
+int d;
+struct A {
+  int n;
+  A() { n = ++a; dump("A%d\n",a); }
+  A(const A&);
+  ~A() THROWING {
+    dump("~A%d\n",n);
+    --a;
+    if (d == 1 ? a == 0 : (b == d && a == 1))
+      {
+       dump ("~A%d throwing\n", n);
+       throw (short)b;
+      }
+  }
+};
+int t;
+struct B {
+  int n;
+  B(const A& = A())
+  {
+    if (b == t)
+      {
+       dump ("B%d throwing\n", b+1);
+       throw b;
+      }
+    n = ++b;
+    dump("B%d\n",b);
+
+    /* The first B has an explicit initializer, so its A lives for the
+       full-expression.  The second B does not, so its A should be destroyed
+       before we construct the third B.  */
+    if (a != 2) abort ();
+  }
+  B(const char *, const A& = A())
+  {
+    if (b == t)
+      {
+       dump ("B%d throwing\n", b+1);
+       throw b;
+      }
+    n = ++b;
+    dump("B%d\n",b);
+    if (a != b) abort ();
+  }
+  B(const B&);
+  ~B()
+  {
+    dump("~B%d\n",n);
+    --b;
+  }
+};
+struct C {
+  B bs[3];
+};
+void f()
+{
+  a = b = 0;
+  try
+    {
+      B bs[3] = { "x" };
+      if (a != 0) abort ();
+      if (b != 3) abort ();
+    }
+  catch (int i) { }
+  catch (short s) { }
+  if (a != 0) abort ();
+  if (b != 0) abort ();
+  dump ("\n", 0);
+}
+
+int main()
+{
+  for (t = 0; t <= 3; ++t)
+    f();
+  for (d = 1; d <= 3; ++d)
+    f();
+}
index 3bb4800..f45a6e1 100644 (file)
@@ -1,5 +1,5 @@
 // PR c++/12253
-// Bug: We were failing to destroy the temporary A passed to the
+// We should not destroy the temporary A passed to the
 // constructor for b[0] before going on to construct b[1].
 
 // { dg-do run }
@@ -11,18 +11,21 @@ int r;
 
 struct A
 {
-  A() { printf ("A()\n"); if (c++) r = 1; }
+  A() { printf ("A()\n"); ++c; }
   A(const A&) { printf ("A(const A&)\n"); ++c; }
   ~A() { printf ("~A()\n"); --c; }
 };
  
 struct B
 {
-  B(int, const A& = A()) { printf ("B()\n"); }
+  B(int i, const A& = A()) {
+    printf ("B()\n");
+    if (c != i) r = 1;
+  }
 };
  
 int main()
 {
-  B b[] = { 0, 0 };
+  B b[] = { 1, 2 };
   return r;
 }