public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [pushed 01/11] c++: don't preevaluate new-initializer
@ 2022-01-07  0:21 Jason Merrill
  2022-01-07  0:21 ` [pushed 02/11] c++: loop over array elts w/o explicit init [PR92385] Jason Merrill
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

Here begins a series of EH cleanup bugfixes.

The preevaluation code was causing trouble with my fix for PR94041, and now
I see that it's actually wrong since P0145 was adopted for C++17, mandating
order of evaluation for many expressions that were previously unspecified.
I don't see a need to preserve the preevaluation code for older standard
modes.

gcc/cp/ChangeLog:

	* init.c (build_new_1): Remove preevaluation code.

gcc/testsuite/ChangeLog:

	* g++.old-deja/g++.martin/new1.C: Don't expect preeval.
---
 gcc/cp/init.c                                | 38 +++++---------------
 gcc/testsuite/g++.dg/tree-ssa/stabilize1.C   | 13 -------
 gcc/testsuite/g++.old-deja/g++.martin/new1.C | 18 +++++-----
 3 files changed, 17 insertions(+), 52 deletions(-)
 delete mode 100644 gcc/testsuite/g++.dg/tree-ssa/stabilize1.C

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 9d616f3f5e9..2cab4b4cdce 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3047,7 +3047,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
      address of the first array element.  This node is a VAR_DECL, and
      is therefore reusable.  */
   tree data_addr;
-  tree init_preeval_expr = NULL_TREE;
   tree orig_type = type;
 
   if (nelts)
@@ -3561,7 +3560,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
      placement delete.  */
   if (is_initialized)
     {
-      bool stable;
       bool explicit_value_init_p = false;
 
       if (*init != NULL && (*init)->is_empty ())
@@ -3587,7 +3585,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 						   init, elt_type,
 						   LOOKUP_NORMAL,
 						   complain);
-	  stable = stabilize_init (init_expr, &init_preeval_expr);
 	}
       else if (array_p)
 	{
@@ -3633,11 +3630,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 			      explicit_value_init_p,
 			      /*from_array=*/0,
                               complain);
-
-	  /* An array initialization is stable because the initialization
-	     of each element is a full-expression, so the temporaries don't
-	     leak out.  */
-	  stable = true;
 	}
       else
 	{
@@ -3694,8 +3686,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 	      = replace_placeholders (TREE_OPERAND (init_expr, 1),
 				      TREE_OPERAND (init_expr, 0),
 				      &had_placeholder);
-	  stable = (!had_placeholder
-		    && stabilize_init (init_expr, &init_preeval_expr));
 	}
 
       if (init_expr == error_mark_node)
@@ -3726,20 +3716,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 		      alloc_fn,
 		      complain));
 
-	  if (!cleanup)
-	    /* We're done.  */;
-	  else if (stable)
-	    /* This is much simpler if we were able to preevaluate all of
-	       the arguments to the constructor call.  */
-	    {
-	      /* CLEANUP is compiler-generated, so no diagnostics.  */
-	      suppress_warning (cleanup);
-	      init_expr = build2 (TRY_CATCH_EXPR, void_type_node,
-				  init_expr, cleanup);
-	      /* Likewise, this try-catch is compiler-generated.  */
-	      suppress_warning (init_expr);
-	    }
-	  else
+	  if (cleanup && !processing_template_decl)
 	    /* Ack!  First we allocate the memory.  Then we set our sentry
 	       variable to true, and expand a cleanup that deletes the
 	       memory if sentry is true.  Then we run the constructor, and
@@ -3747,9 +3724,13 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 
 	       We need to do this because we allocate the space first, so
 	       if there are any temporaries with cleanups in the
-	       constructor args and we weren't able to preevaluate them, we
-	       need this EH region to extend until end of full-expression
-	       to preserve nesting.  */
+	       constructor args, we need this EH region to extend until
+	       end of full-expression to preserve nesting.
+
+	       We used to try to evaluate the args first to avoid this, but
+	       since C++17 [expr.new] says that "The invocation of the
+	       allocation function is sequenced before the evaluations of
+	       expressions in the new-initializer."  */
 	    {
 	      tree end, sentry, begin;
 
@@ -3810,9 +3791,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
       rval = build2 (COMPOUND_EXPR, TREE_TYPE (rval), alloc_expr, rval);
     }
 
-  if (init_preeval_expr)
-    rval = build2 (COMPOUND_EXPR, TREE_TYPE (rval), init_preeval_expr, rval);
-
   /* A new-expression is never an lvalue.  */
   gcc_assert (!obvalue_p (rval));
 
diff --git a/gcc/testsuite/g++.dg/tree-ssa/stabilize1.C b/gcc/testsuite/g++.dg/tree-ssa/stabilize1.C
deleted file mode 100644
index 5eb0bf8d525..00000000000
--- a/gcc/testsuite/g++.dg/tree-ssa/stabilize1.C
+++ /dev/null
@@ -1,13 +0,0 @@
-// PR c++/53356
-// { dg-options "-fdump-tree-gimple" }
-// { dg-final { scan-tree-dump-not "= 0" "gimple" } }
-
-class A {};
-
-struct B {
-    operator const A &() const;
-};
-
-A* cause_ICE() {
-    return new A(B());
-}
diff --git a/gcc/testsuite/g++.old-deja/g++.martin/new1.C b/gcc/testsuite/g++.old-deja/g++.martin/new1.C
index 502c4f42ad0..18eb88d7c79 100644
--- a/gcc/testsuite/g++.old-deja/g++.martin/new1.C
+++ b/gcc/testsuite/g++.old-deja/g++.martin/new1.C
@@ -1,5 +1,5 @@
 // { dg-do run  }
-//Lifetime of temporaries: 
+//Lifetime of temporaries:
 //egcs 2.92 performs cleanup for temporaries inside new expressions
 //after the new is complete, not at the end of the full expression.
 
@@ -71,8 +71,8 @@ void test1()
     func(new B(A(10).addr()));
   }catch(int){
   }
-  CHECK(ctor_done==1);
-  CHECK(new_done==2);
+  CHECK(new_done==1);
+  CHECK(ctor_done==2);
   CHECK(func_done==3);
   CHECK(dtor_done==4);
   CHECK(delete_done==0);
@@ -86,10 +86,10 @@ void test2()
     func(new B(A(10).addr()));
   }catch(int){
   }
-  CHECK(ctor_done==1);
-  CHECK(new_done==2);
+  CHECK(new_done==1);
+  CHECK(ctor_done==0);
   CHECK(func_done==0);
-  CHECK(dtor_done==3);
+  CHECK(dtor_done==0);
   CHECK(delete_done==0);
 }
 
@@ -101,11 +101,11 @@ void test3()
     func(new B(A(10).addr()));
   }catch(int){
   }
-  CHECK(new_done==0);
-  CHECK(ctor_done==1);
+  CHECK(new_done==1);
+  CHECK(ctor_done==2);
   CHECK(func_done==0);
   CHECK(dtor_done==0);
-  CHECK(delete_done==0);
+  CHECK(delete_done==3);
 }
 
 int main()

base-commit: 11ce8d04f29417f2541d9b9bbfb54b3b26d7a90d
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 02/11] c++: loop over array elts w/o explicit init [PR92385]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 03/11] c++: temporary lifetime with aggregate init [PR94041] Jason Merrill
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

The PR complains that initializing a large array with {} takes a long time
to compile; this was because digest_init would turn {} into a long
CONSTRUCTOR with an initializer for each element, instead of more sensibly
generating a loop.  The standard doesn't specify this implementation, but it
does allow for it by specifying that a temporary created "when a default
constructor is called to initialize an element of an array with no
corresponding initializer" is destroyed "before the construction of the next
array element, if any." rather than living until the end of the complete
object initialization as usual.

This change is also needed before the PR94041 fix extends the lifetime of
temporaries from elements with explicit initializers.

To implement this, I change digest_init so that in cases where
initialization of trailing array elements isn't constant, we return a
VEC_INIT_EXPR instead of a bare CONSTRUCTOR; when it is encountered later,
we call build_vec_init to generate the actual initialization code.

	PR c++/92385

gcc/cp/ChangeLog:

	* typeck2.c (PICFLAG_VEC_INIT): New.
	(process_init_constructor_array): Set it.
	(process_init_constructor): Handle it.
	(split_nonconstant_init_1): Handle VEC_INIT_EXPR.
	* init.c (build_vec_init): Likewise.
	* cp-gimplify.c (cp_gimplify_expr): Factor out...
	* tree.c (expand_vec_init_expr): ...this function.
	(build_vec_init_elt): Handle BRACE_ENCLOSED_INITIALIZER_P.
	(build_vec_init_expr): Likewise.
	* constexpr.c (cxx_eval_vec_init): Likewise.
	(reduced_constant_expression_p): Check arrays before C++20.
	* cp-tree.h (expand_vec_init_expr): Declare.

gcc/testsuite/ChangeLog:

	* g++.dg/init/array61.C: New test.
---
 gcc/cp/cp-tree.h                    |  1 +
 gcc/cp/constexpr.c                  | 35 ++++++++++++++++++++---
 gcc/cp/cp-gimplify.c                | 13 ++-------
 gcc/cp/init.c                       | 11 +++++++-
 gcc/cp/tree.c                       | 44 ++++++++++++++++++++---------
 gcc/cp/typeck2.c                    | 23 +++++++++++++--
 gcc/testsuite/g++.dg/init/array61.C | 16 +++++++++++
 7 files changed, 112 insertions(+), 31 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/init/array61.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 10ca8098a85..0a3697f2f98 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7779,6 +7779,7 @@ 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 void diagnose_non_constexpr_vec_init	(tree);
 extern tree hash_tree_cons			(tree, tree, tree);
 extern tree hash_tree_chain			(tree, tree);
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 72be45c9e87..4a4b347c31d 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3020,8 +3020,7 @@ reduced_constant_expression_p (tree t)
 	  if (TREE_CODE (TREE_TYPE (t)) == VECTOR_TYPE)
 	    /* An initialized vector would have a VECTOR_CST.  */
 	    return false;
-	  else if (cxx_dialect >= cxx20
-		   && TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE)
+	  else if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE)
 	    {
 	      /* There must be a valid constant initializer at every array
 		 index.  */
@@ -4955,8 +4954,36 @@ cxx_eval_vec_init (const constexpr_ctx *ctx, tree t,
 {
   tree atype = TREE_TYPE (t);
   tree init = VEC_INIT_EXPR_INIT (t);
-  tree r = cxx_eval_vec_init_1 (ctx, atype, init,
-				VEC_INIT_EXPR_VALUE_INIT (t),
+  bool value_init = VEC_INIT_EXPR_VALUE_INIT (t);
+  if (!init || !BRACE_ENCLOSED_INITIALIZER_P (init))
+    ;
+  else if (CONSTRUCTOR_NELTS (init) == 0)
+    {
+      /* Handle {} as value-init.  */
+      init = NULL_TREE;
+      value_init = true;
+    }
+  else
+    {
+      /* This is a more complicated case, like needing to loop over trailing
+	 elements; call build_vec_init and evaluate the result.  */
+      tsubst_flags_t complain = ctx->quiet ? tf_none : tf_warning_or_error;
+      constexpr_ctx new_ctx = *ctx;
+      if (!ctx->object)
+	{
+	  /* We want to have an initialization target for an VEC_INIT_EXPR.
+	     If we don't already have one in CTX, use the VEC_INIT_EXPR_SLOT.  */
+	  new_ctx.object = VEC_INIT_EXPR_SLOT (t);
+	  tree ctor = new_ctx.ctor = build_constructor (atype, NULL);
+	  CONSTRUCTOR_NO_CLEARING (ctor) = true;
+	  ctx->global->values.put (new_ctx.object, ctor);
+	  ctx = &new_ctx;
+	}
+      init = expand_vec_init_expr (ctx->object, t, complain);
+      return cxx_eval_constant_expression (ctx, init, lval, non_constant_p,
+					   overflow_p);
+    }
+  tree r = cxx_eval_vec_init_1 (ctx, atype, init, value_init,
 				lval, non_constant_p, overflow_p);
   if (*non_constant_p)
     return t;
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 27e78ee3d24..e4f24428629 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -462,21 +462,14 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 
     case VEC_INIT_EXPR:
       {
-	location_t loc = input_location;
-	tree init = VEC_INIT_EXPR_INIT (*expr_p);
-	int from_array = (init && TREE_CODE (TREE_TYPE (init)) == ARRAY_TYPE);
-	gcc_assert (EXPR_HAS_LOCATION (*expr_p));
-	input_location = EXPR_LOCATION (*expr_p);
-	*expr_p = build_vec_init (VEC_INIT_EXPR_SLOT (*expr_p), NULL_TREE,
-				  init, VEC_INIT_EXPR_VALUE_INIT (*expr_p),
-				  from_array,
-				  tf_warning_or_error);
+	*expr_p = expand_vec_init_expr (NULL_TREE, *expr_p,
+					tf_warning_or_error);
+
 	hash_set<tree> pset;
 	cp_walk_tree (expr_p, cp_fold_r, &pset, NULL);
 	cp_genericize_tree (expr_p, false);
 	copy_if_shared (expr_p);
 	ret = GS_OK;
-	input_location = loc;
       }
       break;
 
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 2cab4b4cdce..2a7dfe2b505 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4337,12 +4337,19 @@ build_vec_init (tree base, tree maxindex, tree init,
       && from_array != 2)
     init = TARGET_EXPR_INITIAL (init);
 
+  if (init && TREE_CODE (init) == VEC_INIT_EXPR)
+    {
+      gcc_checking_assert (false);
+      init = VEC_INIT_EXPR_INIT (init);
+    }
+
   bool direct_init = false;
   if (from_array && init && BRACE_ENCLOSED_INITIALIZER_P (init)
       && CONSTRUCTOR_NELTS (init) == 1)
     {
       tree elt = CONSTRUCTOR_ELT (init, 0)->value;
-      if (TREE_CODE (TREE_TYPE (elt)) == ARRAY_TYPE)
+      if (TREE_CODE (TREE_TYPE (elt)) == ARRAY_TYPE
+	  && TREE_CODE (elt) != VEC_INIT_EXPR)
 	{
 	  direct_init = DIRECT_LIST_INIT_P (init);
 	  init = elt;
@@ -4516,6 +4523,8 @@ build_vec_init (tree base, tree maxindex, tree init,
 	  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);
 	  else if (MAYBE_CLASS_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
 	    one_init = build_aggr_init (baseref, elt, 0, complain);
 	  else
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 61998019312..8bd1964e867 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -756,13 +756,11 @@ build_vec_init_elt (tree type, tree init, tsubst_flags_t complain)
   else if (init == void_type_node)
     return build_value_init (inner_type, complain);
 
-  gcc_assert (init == NULL_TREE
-	      || (same_type_ignoring_top_level_qualifiers_p
-		  (type, TREE_TYPE (init))));
-
   releasing_vec argvec;
-  if (init)
+  if (init && !BRACE_ENCLOSED_INITIALIZER_P (init))
     {
+      gcc_assert (same_type_ignoring_top_level_qualifiers_p
+		  (type, TREE_TYPE (init)));
       tree init_type = strip_array_types (TREE_TYPE (init));
       tree dummy = build_dummy_object (init_type);
       if (!lvalue_p (init))
@@ -788,25 +786,28 @@ build_vec_init_elt (tree type, tree init, tsubst_flags_t complain)
 tree
 build_vec_init_expr (tree type, tree init, tsubst_flags_t complain)
 {
-  tree slot;
-  bool value_init = false;
-  tree elt_init;
-  if (init && TREE_CODE (init) == CONSTRUCTOR)
+  if (init && TREE_CODE (init) == VEC_INIT_EXPR)
     {
-      gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (init));
-      /* We built any needed constructor calls in digest_init.  */
-      elt_init = init;
+      gcc_checking_assert (false);
+      return init;
     }
+
+  tree elt_init;
+  if (init && TREE_CODE (init) == CONSTRUCTOR
+      && !BRACE_ENCLOSED_INITIALIZER_P (init))
+    /* We built any needed constructor calls in digest_init.  */
+    elt_init = init;
   else
     elt_init = build_vec_init_elt (type, init, complain);
 
+  bool value_init = false;
   if (init == void_type_node)
     {
       value_init = true;
       init = NULL_TREE;
     }
 
-  slot = build_local_temp (type);
+  tree slot = build_local_temp (type);
   init = build2 (VEC_INIT_EXPR, type, slot, init);
   TREE_SIDE_EFFECTS (init) = true;
   SET_EXPR_LOCATION (init, input_location);
@@ -819,6 +820,23 @@ build_vec_init_expr (tree type, tree init, tsubst_flags_t complain)
   return init;
 }
 
+/* Call build_vec_init to expand VEC_INIT into TARGET (for which NULL_TREE
+   means VEC_INIT_EXPR_SLOT).  */
+
+tree
+expand_vec_init_expr (tree target, tree vec_init, tsubst_flags_t complain)
+{
+  iloc_sentinel ils = EXPR_LOCATION (vec_init);
+
+  if (!target)
+    target = VEC_INIT_EXPR_SLOT (vec_init);
+  tree init = VEC_INIT_EXPR_INIT (vec_init);
+  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);
+}
+
 /* Give a helpful diagnostic for a non-constexpr VEC_INIT_EXPR in a context
    that requires a constant expression.  */
 
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 84cef54e112..3d4d35e13c6 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1317,6 +1317,7 @@ digest_nsdmi_init (tree decl, tree init, tsubst_flags_t complain)
 #define PICFLAG_NOT_ALL_CONSTANT 2
 #define PICFLAG_NOT_ALL_SIMPLE 4
 #define PICFLAG_SIDE_EFFECTS 8
+#define PICFLAG_VEC_INIT 16
 
 /* Given an initializer INIT, return the flag (PICFLAG_*) which better
    describe it.  */
@@ -1460,10 +1461,19 @@ process_init_constructor_array (tree type, tree init, int nested, int flags,
 
 	if (next)
 	  {
-	    picflags |= picflag_from_initializer (next);
-	    if (len > i+1
+	    if (next != error_mark_node
+		&& ! seen_error () // Improves error-recovery on anew5.C.
 		&& (initializer_constant_valid_p (next, TREE_TYPE (next))
-		    == null_pointer_node))
+		    != null_pointer_node))
+	      {
+		/* Use VEC_INIT_EXPR for non-constant initialization of
+		   trailing elements with no explicit initializers.  */
+		picflags |= PICFLAG_VEC_INIT;
+		break;
+	      }
+
+	    picflags |= picflag_from_initializer (next);
+	    if (len > i+1)
 	      {
 		tree range = build2 (RANGE_EXPR, size_type_node,
 				     build_int_cst (size_type_node, i),
@@ -1858,6 +1868,13 @@ process_init_constructor (tree type, tree init, int nested, int flags,
       if (!(picflags & PICFLAG_NOT_ALL_SIMPLE))
 	TREE_STATIC (init) = 1;
     }
+  if (picflags & PICFLAG_VEC_INIT)
+    {
+      /* Defer default-initialization of array elements with no corresponding
+	 initializer-clause until later so we can use a loop.  */
+      TREE_TYPE (init) = init_list_type_node;
+      init = build_vec_init_expr (type, init, complain);
+    }
   return init;
 }
 \f
diff --git a/gcc/testsuite/g++.dg/init/array61.C b/gcc/testsuite/g++.dg/init/array61.C
new file mode 100644
index 00000000000..eaf535c2546
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/array61.C
@@ -0,0 +1,16 @@
+// PR c++/92385
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -fdump-tree-gimple }
+// { dg-final { scan-tree-dump-times "item::item" 1 "gimple" } }
+
+struct item {
+  int i;
+  item();
+};
+
+struct item_array {
+  item a[10];
+  item_array();
+};
+
+item_array::item_array() : a{} {}
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 03/11] c++: temporary lifetime with aggregate init [PR94041]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
  2022-01-07  0:21 ` [pushed 02/11] c++: loop over array elts w/o explicit init [PR92385] Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 04/11] c++: temporary lifetime with array aggr " Jason Merrill
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

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.
---
 gcc/cp/decl.c                            | 12 ++---
 gcc/cp/typeck2.c                         | 55 +++++++++++++++++----
 gcc/testsuite/g++.dg/cpp0x/initlist122.C | 12 ++++-
 gcc/testsuite/g++.dg/init/aggr7-eh.C     | 62 ++++++++++++++++++++++++
 4 files changed, 121 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/init/aggr7-eh.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0b71c00f5ab..9f759ceb1c8 100644
--- 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 3d4d35e13c6..54b1d0da129 100644
--- 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 002bc1eaf3c..81953a44e12 100644
--- 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
index 00000000000..db45e15d9af
--- /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();
+}
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 04/11] c++: temporary lifetime with array aggr init [PR94041]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
  2022-01-07  0:21 ` [pushed 02/11] c++: loop over array elts w/o explicit init [PR92385] Jason Merrill
  2022-01-07  0:21 ` [pushed 03/11] c++: temporary lifetime with aggregate init [PR94041] Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 05/11] c++: EH and partially constructed aggr temp [PR66139] Jason Merrill
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

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                      |  6 +-
 gcc/cp/decl.c                         |  3 +-
 gcc/cp/init.c                         | 84 +++++++++++++----------
 gcc/cp/tree.c                         |  5 +-
 gcc/cp/typeck2.c                      | 67 ++++++++++--------
 gcc/testsuite/g++.dg/init/aggr7-eh2.C | 98 +++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/init/aggr7-eh3.C | 98 +++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/init/array12.C   | 11 +--
 8 files changed, 298 insertions(+), 74 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/init/aggr7-eh2.C
 create mode 100644 gcc/testsuite/g++.dg/init/aggr7-eh3.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 0a3697f2f98..c75ecaf8c8e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -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);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 9f759ceb1c8..b16a4f9ed34 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -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);
 
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 2a7dfe2b505..7c7b8104026 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -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);
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 8bd1964e867..964e40ea11a 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -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
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 54b1d0da129..7907c53996d 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -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
index 00000000000..0037b092394
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/aggr7-eh2.C
@@ -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
index 00000000000..6ddabec58b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/aggr7-eh3.C
@@ -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();
+}
diff --git a/gcc/testsuite/g++.dg/init/array12.C b/gcc/testsuite/g++.dg/init/array12.C
index 3bb48002967..f45a6e141b7 100644
--- a/gcc/testsuite/g++.dg/init/array12.C
+++ b/gcc/testsuite/g++.dg/init/array12.C
@@ -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;
 }
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 05/11] c++: EH and partially constructed aggr temp [PR66139]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
                   ` (2 preceding siblings ...)
  2022-01-07  0:21 ` [pushed 04/11] c++: temporary lifetime with array aggr " Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 06/11] c++: don't cleanup the last aggregate elt Jason Merrill
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

Now that PR94041 is fixed, I can return to addressing PR66139, missing
cleanups for partially constructed aggregate temporaries.  My previous
approach of calling split_nonconstant_init in cp_gimplify_init_expr broke
because gimplification is too late to be introducing destructor calls.  So
instead I now call it under cp_fold_function, just before cp_genericize;
doing it from cp_genericize itself ran into trouble with the rewriting of
invisible references.

So now the prediction in cp_gimplify_expr that cp_gimplify_init_expr
might need to replace references to TARGET_EXPR_SLOT within
TARGET_EXPR_INITIAL has come to pass.  constexpr.c already had the simple
search-and-replace tree walk I needed, but it needed to be fixed to actually
replace all occurrences instead of just the first one.

Handling of VEC_INIT_EXPR at gimplify time had similar issues that we worked
around with build_vec_init_elt, so I'm moving that to cp_fold_function as
well.  But it seems that build_vec_init_elt is still useful for setting the
VEC_INIT_EXPR_IS_CONSTEXPR flag, so I'm leaving it alone.

This also fixes 52320, because build_aggr_init of each X from build_vec_init
builds an INIT_EXPR rather than call split_nonconstant_init at that point,
and now that INIT_EXPR gets split later.

	PR c++/66139
	PR c++/52320

gcc/cp/ChangeLog:

	* constexpr.c (replace_decl): Rename from replace_result_decl.
	* cp-tree.h (replace_decl): Declare it.
	* cp-gimplify.c (cp_gimplify_init_expr): Call it.
	(cp_gimplify_expr): Don't handle VEC_INIT_EXPR.
	(cp_genericize_init, cp_genericize_init_expr)
	(cp_genericize_target_expr): New.
	(cp_fold_r): Call them.
	* tree.c (build_array_copy): Add a TARGET_EXPR.
	* typeck2.c (digest_init_r): Look through a TARGET_EXPR.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/initlist116.C: New test.
	* g++.dg/cpp0x/initlist117.C: New test.
	* g++.dg/cpp0x/lambda/lambda-eh.C: New test.
	* g++.dg/eh/aggregate1.C: New test.
---
 gcc/cp/cp-tree.h                              |  1 +
 gcc/cp/constexpr.c                            | 37 ++++----
 gcc/cp/cp-gimplify.c                          | 93 +++++++++++++++----
 gcc/cp/tree.c                                 | 15 +--
 gcc/cp/typeck2.c                              |  3 +
 gcc/testsuite/g++.dg/cpp0x/initlist116.C      | 29 ++++++
 gcc/testsuite/g++.dg/cpp0x/initlist117.C      | 40 ++++++++
 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C | 34 +++++++
 gcc/testsuite/g++.dg/eh/aggregate1.C          | 56 +++++++++++
 9 files changed, 265 insertions(+), 43 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist116.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist117.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C
 create mode 100644 gcc/testsuite/g++.dg/eh/aggregate1.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c75ecaf8c8e..56e6d661537 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8385,6 +8385,7 @@ extern tree fold_sizeof_expr			(tree);
 extern void clear_cv_and_fold_caches		(void);
 extern tree unshare_constructor			(tree CXX_MEM_STAT_INFO);
 extern bool decl_implicit_constexpr_p		(tree);
+extern bool replace_decl			(tree *, tree, tree);
 
 /* An RAII sentinel used to restrict constexpr evaluation so that it
    doesn't do anything that causes extra DECL_UID generation.  */
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 4a4b347c31d..41594c782fc 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2251,24 +2251,26 @@ cxx_eval_dynamic_cast_fn (const constexpr_ctx *ctx, tree call,
   return cp_build_addr_expr (obj, complain);
 }
 
-/* Data structure used by replace_result_decl and replace_result_decl_r.  */
+/* Data structure used by replace_decl and replace_decl_r.  */
 
-struct replace_result_decl_data
+struct replace_decl_data
 {
-  /* The RESULT_DECL we want to replace.  */
+  /* The _DECL we want to replace.  */
   tree decl;
   /* The replacement for DECL.  */
   tree replacement;
+  /* Trees we've visited.  */
+  hash_set<tree> *pset;
   /* Whether we've performed any replacements.  */
   bool changed;
 };
 
-/* Helper function for replace_result_decl, called through cp_walk_tree.  */
+/* Helper function for replace_decl, called through cp_walk_tree.  */
 
 static tree
-replace_result_decl_r (tree *tp, int *walk_subtrees, void *data)
+replace_decl_r (tree *tp, int *walk_subtrees, void *data)
 {
-  replace_result_decl_data *d = (replace_result_decl_data *) data;
+  replace_decl_data *d = (replace_decl_data *) data;
 
   if (*tp == d->decl)
     {
@@ -2276,24 +2278,25 @@ replace_result_decl_r (tree *tp, int *walk_subtrees, void *data)
       d->changed = true;
       *walk_subtrees = 0;
     }
-  else if (TYPE_P (*tp))
+  else if (TYPE_P (*tp)
+	   || d->pset->add (*tp))
     *walk_subtrees = 0;
 
   return NULL_TREE;
 }
 
-/* Replace every occurrence of DECL, a RESULT_DECL, with (an unshared copy of)
-   REPLACEMENT within the reduced constant expression *TP.  Returns true iff a
+/* Replace every occurrence of DECL with (an unshared copy of)
+   REPLACEMENT within the expression *TP.  Returns true iff a
    replacement was performed.  */
 
-static bool
-replace_result_decl (tree *tp, tree decl, tree replacement)
+bool
+replace_decl (tree *tp, tree decl, tree replacement)
 {
-  gcc_checking_assert (TREE_CODE (decl) == RESULT_DECL
-		       && (same_type_ignoring_top_level_qualifiers_p
-			   (TREE_TYPE (decl), TREE_TYPE (replacement))));
-  replace_result_decl_data data = { decl, replacement, false };
-  cp_walk_tree_without_duplicates (tp, replace_result_decl_r, &data);
+  gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p
+		       (TREE_TYPE (decl), TREE_TYPE (replacement)));
+  hash_set<tree> pset;
+  replace_decl_data data = { decl, replacement, &pset, false };
+  cp_walk_tree (tp, replace_decl_r, &data, NULL);
   return data.changed;
 }
 
@@ -2962,7 +2965,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	    if (!*non_constant_p && ctx->object
 		&& CLASS_TYPE_P (TREE_TYPE (res))
 		&& !is_empty_class (TREE_TYPE (res)))
-	      if (replace_result_decl (&result, res, ctx->object))
+	      if (replace_decl (&result, res, ctx->object))
 		cacheable = false;
 	}
       else
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index e4f24428629..114fa78b353 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -239,12 +239,21 @@ cp_gimplify_init_expr (tree *expr_p)
   tree to = TREE_OPERAND (*expr_p, 0);
   tree t;
 
-  /* What about code that pulls out the temp and uses it elsewhere?  I
-     think that such code never uses the TARGET_EXPR as an initializer.  If
-     I'm wrong, we'll abort because the temp won't have any RTL.  In that
-     case, I guess we'll need to replace references somehow.  */
-  if (TREE_CODE (from) == TARGET_EXPR && TARGET_EXPR_INITIAL (from))
-    from = TARGET_EXPR_INITIAL (from);
+  if (TREE_CODE (from) == TARGET_EXPR)
+    if (tree init = TARGET_EXPR_INITIAL (from))
+      {
+	if (VOID_TYPE_P (TREE_TYPE (init))
+	    && TREE_CODE (init) != AGGR_INIT_EXPR)
+	  {
+	    /* If this was changed by cp_genericize_target_expr, we need to
+	       walk into it to replace uses of the slot.  */
+	    replace_decl (&init, TARGET_EXPR_SLOT (from), to);
+	    *expr_p = init;
+	    return;
+	  }
+	else
+	  from = init;
+      }
 
   /* Look through any COMPOUND_EXPRs, since build_compound_expr pushes them
      inside the TARGET_EXPR.  */
@@ -460,19 +469,6 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
       ret = GS_OK;
       break;
 
-    case VEC_INIT_EXPR:
-      {
-	*expr_p = expand_vec_init_expr (NULL_TREE, *expr_p,
-					tf_warning_or_error);
-
-	hash_set<tree> pset;
-	cp_walk_tree (expr_p, cp_fold_r, &pset, NULL);
-	cp_genericize_tree (expr_p, false);
-	copy_if_shared (expr_p);
-	ret = GS_OK;
-      }
-      break;
-
     case THROW_EXPR:
       /* FIXME communicate throw type to back end, probably by moving
 	 THROW_EXPR into ../tree.def.  */
@@ -868,6 +864,57 @@ omp_cxx_notice_variable (struct cp_genericize_omp_taskreg *omp_ctx, tree decl)
     }
 }
 
+/* If we might need to clean up a partially constructed object, break down the
+   CONSTRUCTOR with split_nonconstant_init.  Also expand VEC_INIT_EXPR at this
+   point.  If initializing TO with FROM is non-trivial, overwrite *REPLACE with
+   the result.  */
+
+static void
+cp_genericize_init (tree *replace, tree from, tree to)
+{
+  if (TREE_CODE (from) == VEC_INIT_EXPR)
+    {
+      tree init = expand_vec_init_expr (to, from, tf_warning_or_error);
+
+      /* Make cp_gimplify_init_expr call replace_decl.  */
+      *replace = fold_convert (void_type_node, init);
+    }
+  else if (flag_exceptions
+	   && TREE_CODE (from) == CONSTRUCTOR
+	   && TREE_SIDE_EFFECTS (from)
+	   && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (from)))
+    {
+      to = cp_stabilize_reference (to);
+      replace_placeholders (from, to);
+      *replace = split_nonconstant_init (to, from);
+    }
+}
+
+/* For an INIT_EXPR, replace the INIT_EXPR itself.  */
+
+static void
+cp_genericize_init_expr (tree *stmt_p)
+{
+  tree to = TREE_OPERAND (*stmt_p, 0);
+  tree from = TREE_OPERAND (*stmt_p, 1);
+  if (SIMPLE_TARGET_EXPR_P (from)
+      /* Return gets confused if we clobber its INIT_EXPR this soon.  */
+      && TREE_CODE (to) != RESULT_DECL)
+    from = TARGET_EXPR_INITIAL (from);
+  cp_genericize_init (stmt_p, from, to);
+}
+
+/* For a TARGET_EXPR, change the TARGET_EXPR_INITIAL.  We will need to use
+   replace_decl later when we know what we're initializing.  */
+
+static void
+cp_genericize_target_expr (tree *stmt_p)
+{
+  cp_genericize_init (&TARGET_EXPR_INITIAL (*stmt_p),
+		      TARGET_EXPR_INITIAL (*stmt_p),
+		      TARGET_EXPR_SLOT (*stmt_p));
+}
+
 /* Genericization context.  */
 
 struct cp_genericize_data
@@ -1007,6 +1054,14 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data)
 	}
       break;
 
+    case INIT_EXPR:
+      cp_genericize_init_expr (stmt_p);
+      break;
+
+    case TARGET_EXPR:
+      cp_genericize_target_expr (stmt_p);
+      break;
+
     default:
       break;
     }
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 964e40ea11a..4c1135ba386 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -737,12 +737,12 @@ build_cplus_new (tree type, tree init, tsubst_flags_t complain)
    intialization as a proxy for the full array initialization to get things
    marked as used and any appropriate diagnostics.
 
-   Since we're deferring building the actual constructor calls until
-   gimplification time, we need to build one now and throw it away so
-   that the relevant constructor gets mark_used before cgraph decides
-   what functions are needed.  Here we assume that init is either
-   NULL_TREE, void_type_node (indicating value-initialization), or
-   another array to copy.  */
+   This used to be necessary because we were deferring building the actual
+   constructor calls until gimplification time; now we only do it to set
+   VEC_INIT_EXPR_IS_CONSTEXPR.
+
+   We assume that init is either NULL_TREE, void_type_node (indicating
+   value-initialization), or another array to copy.  */
 
 static tree
 build_vec_init_elt (tree type, tree init, tsubst_flags_t complain)
@@ -858,7 +858,8 @@ diagnose_non_constexpr_vec_init (tree expr)
 tree
 build_array_copy (tree init)
 {
-  return build_vec_init_expr (TREE_TYPE (init), init, tf_warning_or_error);
+  return get_target_expr (build_vec_init_expr
+			  (TREE_TYPE (init), init, tf_warning_or_error));
 }
 
 /* Build a TARGET_EXPR using INIT to initialize a new temporary of the
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 7907c53996d..7e7fc7f9f48 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1282,6 +1282,9 @@ digest_init_r (tree type, tree init, int nested, int flags,
 	}
     }
 
+  if (SIMPLE_TARGET_EXPR_P (stripped_init))
+    stripped_init = TARGET_EXPR_INITIAL (stripped_init);
+
   if (BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
       && !TYPE_NON_AGGREGATE_CLASS (type))
     return process_init_constructor (type, stripped_init, nested, flags,
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist116.C b/gcc/testsuite/g++.dg/cpp0x/initlist116.C
new file mode 100644
index 00000000000..90dd8d70d63
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist116.C
@@ -0,0 +1,29 @@
+// PR c++/66139
+// { dg-do run { target c++11 } }
+
+int constructed = 0;
+
+class lock_guard_ext{
+public:
+  lock_guard_ext() { ++constructed; }
+  ~lock_guard_ext() { --constructed; }
+};
+ 
+struct Access {
+  lock_guard_ext lock;
+  int value;
+};
+ 
+int t() {
+  throw 0;
+}
+
+Access foo1() {
+  return { {}, t() };
+}
+ 
+int main () {
+  try { foo1(); } catch (int) {}
+  if (constructed != 0)
+    __builtin_abort();
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist117.C b/gcc/testsuite/g++.dg/cpp0x/initlist117.C
new file mode 100644
index 00000000000..415a5de2dd1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist117.C
@@ -0,0 +1,40 @@
+// PR c++/66139
+// { dg-do run { target c++11 } }
+
+#include <initializer_list>
+
+int c, d;
+
+struct a
+{
+  a (int i) { if (i) throw i; c++; }
+  ~a () { d++; }
+};
+
+void check (void (*f) ())
+{
+  try
+  {
+    c = d = 0;
+    f ();
+  }
+  catch (int)
+  {
+    if (c != 1 || d != 1)
+      __builtin_abort ();
+    return;
+  }
+  __builtin_abort ();
+}
+
+int main ()
+{
+  struct s { a x, y; };
+  check ([] { s t { 0, 1 }; });
+  check ([] { s { 0, 1 }; });
+  check ([] { a t[2] { 0, 1 }; });
+  using array = a[2];
+  check ([] { array { 0, 1 }; });
+  check ([] { std::initializer_list <a> t { 0, 1 }; });
+  check ([] { std::initializer_list <a> { 0, 1 }; });
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C
new file mode 100644
index 00000000000..4d1f4f3edfc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C
@@ -0,0 +1,34 @@
+// Test that we properly clean up if we get an exception in the middle of
+// constructing the closure object.
+
+// { dg-do run }
+// { dg-require-effective-target c++11 }
+
+struct A
+{
+  A() {}
+  A(const A&) { throw 1; }
+};
+
+int bs;
+struct B
+{
+  B() { ++bs; }
+  B(const B&) { ++bs; }
+  ~B() { --bs; }
+};
+
+int main()
+{
+  {
+    B b1, b2;
+    A a;
+
+    try
+      {
+	[b1, a, b2]{ };
+      }
+    catch(...) {}
+  }
+  return bs;
+}
diff --git a/gcc/testsuite/g++.dg/eh/aggregate1.C b/gcc/testsuite/g++.dg/eh/aggregate1.C
new file mode 100644
index 00000000000..68d0ed74c9a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/aggregate1.C
@@ -0,0 +1,56 @@
+// PR c++/52320
+// { dg-do run }
+
+#if DEBUG
+extern "C" int printf (const char *, ...);
+#define FUNCTION_NAME __PRETTY_FUNCTION__
+#define TRACE_FUNCTION printf ("%p->%s\n", this, FUNCTION_NAME);
+#else
+#define TRACE_FUNCTION 
+#endif
+int c,d;
+#define TRACE_CTOR TRACE_FUNCTION ++c
+#define TRACE_DTOR TRACE_FUNCTION ++d
+
+int throw_at = 0;
+
+struct A {
+  A() { int i = c+1; if (i == throw_at) throw i; TRACE_CTOR; }
+  A(int i) { if (i == throw_at) throw i; TRACE_CTOR; }
+  A(const A&) { throw 10; }
+  A &operator=(const A&) { throw 11; return *this; }
+  ~A() { TRACE_DTOR; }
+};
+
+int fails;
+
+void try_idx (int i)
+{
+#if DEBUG
+  printf ("trying %d\n", i);
+#endif
+  throw_at = i;
+  c = d = 0;
+  int t = 10;
+  try {
+    struct X {
+      A e1[2], e2;
+    }
+    x2[3] = { { 1, 2, 3 }, { 4, 5, 6 } };
+  } catch (int x) { t = x; }
+  if (t != i || c != d || c != i-1)
+    {
+#if DEBUG
+      printf ("%d FAIL\n", i);
+#endif
+      ++fails;
+    }
+}
+
+int main()
+{
+  for (int i = 1; i <= 10; ++i)
+    try_idx (i);
+
+  return fails;
+}
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 06/11] c++: don't cleanup the last aggregate elt
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
                   ` (3 preceding siblings ...)
  2022-01-07  0:21 ` [pushed 05/11] c++: EH and partially constructed aggr temp [PR66139] Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 07/11] c++: keep destroying array after one dtor throws [PR66451] Jason Merrill
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

Now that we're building cleanups for aggregate elements more often, it seems
worth optimizing by avoiding building one for the last element; once it is
initialized, the complete object is fully initialized, the element cleanups
end in favor of the complete object cleanup, and so a cleanup for the last
element would guard nothing at all.

gcc/cp/ChangeLog:

	* typeck2.c (split_nonconstant_init_1): Don't cleanup the last elt.
	(split_nonconstant_init): Adjust.

gcc/testsuite/ChangeLog:

	* g++.dg/tree-ssa/aggregate1.C: New test.
---
 gcc/cp/typeck2.c                           | 14 +++++++++-----
 gcc/testsuite/g++.dg/tree-ssa/aggregate1.C | 19 +++++++++++++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/aggregate1.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 7e7fc7f9f48..f439dd54866 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -486,7 +486,8 @@ maybe_push_temp_cleanup (tree sub, vec<tree,va_gc> **flags)
    generated statements.  */
 
 static bool
-split_nonconstant_init_1 (tree dest, tree init, vec<tree,va_gc> **flags)
+split_nonconstant_init_1 (tree dest, tree init, bool last,
+			  vec<tree,va_gc> **flags)
 {
   unsigned HOST_WIDE_INT idx, tidx = HOST_WIDE_INT_M1U;
   tree field_index, value;
@@ -545,9 +546,11 @@ split_nonconstant_init_1 (tree dest, tree init, vec<tree,va_gc> **flags)
 	    sub = build3 (COMPONENT_REF, inner_type, dest, field_index,
 			  NULL_TREE);
 
+	  bool elt_last = last && idx == CONSTRUCTOR_NELTS (init) - 1;
+
 	  if (TREE_CODE (value) == CONSTRUCTOR)
 	    {
-	      if (!split_nonconstant_init_1 (sub, value, flags)
+	      if (!split_nonconstant_init_1 (sub, value, elt_last, flags)
 		      /* For flexible array member with initializer we
 			 can't remove the initializer, because only the
 			 initializer determines how many elements the
@@ -558,7 +561,7 @@ split_nonconstant_init_1 (tree dest, tree init, vec<tree,va_gc> **flags)
 		      && TREE_CODE (TREE_TYPE (value)) == ARRAY_TYPE
 		      && COMPLETE_TYPE_P (TREE_TYPE (value))
 		      && !integer_zerop (TYPE_SIZE (TREE_TYPE (value)))
-		      && idx == CONSTRUCTOR_NELTS (init) - 1
+		      && elt_last
 		      && TYPE_HAS_TRIVIAL_DESTRUCTOR
 				(strip_array_types (inner_type))))
 		complete_p = false;
@@ -634,7 +637,8 @@ split_nonconstant_init_1 (tree dest, tree init, vec<tree,va_gc> **flags)
 		    }
 		  code = build_stmt (input_location, EXPR_STMT, code);
 		  add_stmt (code);
-		  maybe_push_temp_cleanup (sub, flags);
+		  if (!elt_last)
+		    maybe_push_temp_cleanup (sub, flags);
 		}
 
 	      num_split_elts++;
@@ -715,7 +719,7 @@ split_nonconstant_init (tree dest, tree init)
       if (TREE_CODE (TREE_TYPE (dest)) != ARRAY_TYPE)
 	flags = make_tree_vector ();
 
-      if (split_nonconstant_init_1 (dest, init, &flags))
+      if (split_nonconstant_init_1 (dest, init, true, &flags))
 	init = NULL_TREE;
 
       for (tree f : flags)
diff --git a/gcc/testsuite/g++.dg/tree-ssa/aggregate1.C b/gcc/testsuite/g++.dg/tree-ssa/aggregate1.C
new file mode 100644
index 00000000000..3f30a73b4e0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/aggregate1.C
@@ -0,0 +1,19 @@
+// Test that we don't bother building a cleanup for the last aggregate element.
+// { dg-additional-options -fdump-tree-gimple }
+// { dg-final { scan-tree-dump-not {A::~A \(&b\.a} "gimple" } }
+
+struct A
+{
+  A(int);
+  ~A();
+};
+
+struct B
+{
+  A a;
+};
+
+int main()
+{
+  B b = { 1 };
+}
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 07/11] c++: keep destroying array after one dtor throws [PR66451]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
                   ` (4 preceding siblings ...)
  2022-01-07  0:21 ` [pushed 06/11] c++: don't cleanup the last aggregate elt Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 08/11] c++: clean up ref-extended temp on throwing dtor [PR53868] Jason Merrill
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

When we're cleaning up an array, if one destructor throws, we should still
try to clean up the rest of the array.  We can use TRY_CATCH_EXPR for this,
instead of a TARGET_EXPR like my other recent patches, because a destructor
call can't involve any temporaries that need to live longer.

I thought about only doing this when we call build_vec_delete_1 from
build_vec_init, but it seems appropriate for delete-expressions as well;
we've said that the array's lifetime is over, it makes sense to keep trying
to destroy it.  The standard isn't clear, but clang seems to agree with me.

	PR c++/66451

gcc/cp/ChangeLog:

	* init.c (build_vec_delete_1): Handle throwing dtor.
	(build_vec_init): Tell it we're in a cleanup already.

gcc/testsuite/ChangeLog:

	* g++.dg/eh/array3.C: New test.
---
 gcc/cp/init.c                        | 18 +++++++++++--
 gcc/testsuite/g++.dg/eh/array1.C     |  8 +++++-
 gcc/testsuite/g++.dg/eh/array3.C     | 40 ++++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/eh/delete1.C    |  2 +-
 gcc/testsuite/g++.dg/ipa/devirt-40.C | 10 ++++++-
 gcc/testsuite/g++.dg/warn/pr83054.C  |  9 ++++++-
 6 files changed, 81 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/eh/array3.C

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 7c7b8104026..df63e618394 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4006,7 +4006,8 @@ build_new (location_t loc, vec<tree, va_gc> **placement, tree type,
 static tree
 build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
 		    special_function_kind auto_delete_vec,
-		    int use_global_delete, tsubst_flags_t complain)
+		    int use_global_delete, tsubst_flags_t complain,
+		    bool in_cleanup = false)
 {
   tree virtual_size;
   tree ptype = build_pointer_type (type = complete_type (type));
@@ -4109,6 +4110,18 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
   body = build_compound_expr (loc, body, tmp);
 
   loop = build1 (LOOP_EXPR, void_type_node, body);
+
+  /* If one destructor throws, keep trying to clean up the rest, unless we're
+     already in a build_vec_init cleanup.  */
+  if (flag_exceptions && !in_cleanup && !expr_noexcept_p (tmp, tf_none))
+    {
+      loop = build2 (TRY_CATCH_EXPR, void_type_node, loop,
+		     unshare_expr (loop));
+      /* Tell honor_protect_cleanup_actions to discard this on the
+	 exceptional path.  */
+      TRY_CATCH_IS_CLEANUP (loop) = true;
+    }
+
   loop = build_compound_expr (loc, tbase_init, loop);
 
  no_destructor:
@@ -4490,7 +4503,8 @@ build_vec_init (tree base, tree maxindex, tree init,
 
       e = build_vec_delete_1 (input_location, rval, m,
 			      inner_elt_type, sfk_complete_destructor,
-			      /*use_global_delete=*/0, complain);
+			      /*use_global_delete=*/0, complain,
+			      /*in_cleanup*/true);
       if (e == error_mark_node)
 	errors = true;
       TARGET_EXPR_CLEANUP (iterator_targ) = e;
diff --git a/gcc/testsuite/g++.dg/eh/array1.C b/gcc/testsuite/g++.dg/eh/array1.C
index 30b035cfc52..79d62ad5058 100644
--- a/gcc/testsuite/g++.dg/eh/array1.C
+++ b/gcc/testsuite/g++.dg/eh/array1.C
@@ -2,10 +2,16 @@
 // rather than one for each element.
 // { dg-options "-fdump-tree-gimple" }
 
+#if __cplusplus < 201100L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept
+#endif
+
 struct A
 {
   A();
-  ~A();
+  ~A() NOTHROW;
 };
 
 void f()
diff --git a/gcc/testsuite/g++.dg/eh/array3.C b/gcc/testsuite/g++.dg/eh/array3.C
new file mode 100644
index 00000000000..547541b5dc3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/array3.C
@@ -0,0 +1,40 @@
+// PR c++/66451
+// { dg-do run }
+
+#if __cplusplus > 201100L
+#define THROWING noexcept(false)
+#else
+#define THROWING
+#endif
+
+extern "C" void abort();
+
+int c;
+struct A
+{
+  int a;
+
+  A(int new_a) : a(new_a) { ++c; }
+  A(const A&); // not defined
+  ~A() THROWING
+  {
+    --c;
+    if(a==4)
+      throw a;
+  }
+};
+
+struct B
+{
+  A a[2];
+  ~B() { }
+};
+
+int sink;
+int main()
+{
+  try {
+    B b = {3,4};
+  } catch(...) { }
+  if (c != 0) abort();
+}
diff --git a/gcc/testsuite/g++.dg/eh/delete1.C b/gcc/testsuite/g++.dg/eh/delete1.C
index 1727a74ff36..92ed646ef2c 100644
--- a/gcc/testsuite/g++.dg/eh/delete1.C
+++ b/gcc/testsuite/g++.dg/eh/delete1.C
@@ -69,7 +69,7 @@ int ary ()
 {
   deleted = 0;
 
-  Baz *p = new Baz[5];
+  Baz *p = new Baz[1];
   try { delete[] p; }
   catch (...) { return deleted != 1;}
   return 1;
diff --git a/gcc/testsuite/g++.dg/ipa/devirt-40.C b/gcc/testsuite/g++.dg/ipa/devirt-40.C
index 32e0d22c0e7..31fe1504cb8 100644
--- a/gcc/testsuite/g++.dg/ipa/devirt-40.C
+++ b/gcc/testsuite/g++.dg/ipa/devirt-40.C
@@ -1,4 +1,12 @@
 /* { dg-options "-O2 -fdump-tree-fre3-details"  } */
+
+// A throwing dtor in C++98 mode changes the results.
+#if __cplusplus < 201100L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept
+#endif
+
 typedef enum
 {
 } UErrorCode;
@@ -6,7 +14,7 @@ class UnicodeString
 {
 public:
   UnicodeString ();
-  virtual ~UnicodeString ();
+  virtual ~UnicodeString () NOTHROW;
 };
 class A
 {
diff --git a/gcc/testsuite/g++.dg/warn/pr83054.C b/gcc/testsuite/g++.dg/warn/pr83054.C
index 506c9609b90..5285f94acee 100644
--- a/gcc/testsuite/g++.dg/warn/pr83054.C
+++ b/gcc/testsuite/g++.dg/warn/pr83054.C
@@ -2,6 +2,13 @@
 // { dg-options "-O3 -Wsuggest-final-types" }
 // { dg-do compile }
 
+// A throwing dtor in C++98 mode changes the warning.
+#if __cplusplus < 201100L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept
+#endif
+
 extern "C" int printf (const char *, ...);
 struct foo // { dg-warning "final would enable devirtualization of 5 calls" }
 {
@@ -12,7 +19,7 @@ struct foo // { dg-warning "final would enable devirtualization of 5 calls" }
     x = count++;
     printf("this %d = %x\n", x, (void *)this);
   }
-  virtual ~foo () {
+  virtual ~foo () NOTHROW {
     printf("this %d = %x\n", x, (void *)this);
     --count;
   }
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 08/11] c++: clean up ref-extended temp on throwing dtor [PR53868]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
                   ` (5 preceding siblings ...)
  2022-01-07  0:21 ` [pushed 07/11] c++: keep destroying array after one dtor throws [PR66451] Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 09/11] c++: destroy retval on throwing cleanup in try [PR33799] Jason Merrill
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

We have wrap_temporary_cleanups to handle the EH region nesting problems
between cleanups for complete variables and cleanups for temporaries used in
their construction, but we weren't calling it for temporaries extended from
binding to a reference.

We still don't want this for array cleanups (since my PR94041 fix), so I
move that exception from initialize_local_var to wrap_temporary_cleanups.

	PR c++/53868

gcc/cp/ChangeLog:

	* decl.c (cp_finish_decl): Use wrap_temporary_cleanups for
	cleanups from set_up_extended_ref_temp.
	(wrap_temporary_cleanups): Ignore array cleanups.
	(initialize_local_var): Don't check for array here.
	* cp-tree.h (BIND_EXPR_VEC_DTOR): New.
	* init.c (build_vec_delete_1): Set it.

gcc/testsuite/ChangeLog:

	* g++.dg/eh/ref-temp1.C: New test.
	* g++.dg/eh/ref-temp2.C: New test.
---
 gcc/cp/cp-tree.h                    |  5 +++
 gcc/cp/decl.c                       | 25 +++++++++++---
 gcc/cp/init.c                       |  1 +
 gcc/testsuite/g++.dg/eh/ref-temp1.C | 51 +++++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/eh/ref-temp2.C | 15 +++++++++
 5 files changed, 93 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/eh/ref-temp1.C
 create mode 100644 gcc/testsuite/g++.dg/eh/ref-temp2.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 56e6d661537..e204182da97 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -465,6 +465,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
       OVL_USING_P (in OVERLOAD)
       IMPLICIT_CONV_EXPR_NONTYPE_ARG (in IMPLICIT_CONV_EXPR)
       BASELINK_FUNCTIONS_MAYBE_INCOMPLETE_P (in BASELINK)
+      BIND_EXPR_VEC_DTOR (in BIND_EXPR)
    2: IDENTIFIER_KIND_BIT_2 (in IDENTIFIER_NODE)
       ICS_THIS_FLAG (in _CONV)
       DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (in VAR_DECL)
@@ -712,6 +713,10 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
 #define BIND_EXPR_TRY_BLOCK(NODE) \
   TREE_LANG_FLAG_0 (BIND_EXPR_CHECK (NODE))
 
+/* This BIND_EXPR is from build_vec_delete_1.  */
+#define BIND_EXPR_VEC_DTOR(NODE) \
+  TREE_LANG_FLAG_1 (BIND_EXPR_CHECK (NODE))
+
 /* Used to mark the block around the member initializers and cleanups.  */
 #define BIND_EXPR_BODY_BLOCK(NODE) \
   TREE_LANG_FLAG_3 (BIND_EXPR_CHECK (NODE))
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index b16a4f9ed34..5fe341e0b75 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7451,11 +7451,24 @@ wrap_cleanups_r (tree *stmt_p, int *walk_subtrees, void *data)
    they are run on the normal path, but not if they are run on the
    exceptional path.  We implement this by telling
    honor_protect_cleanup_actions to strip the variable cleanup from the
-   exceptional path.  */
+   exceptional path.
+
+   Another approach could be to make the variable cleanup region enclose
+   initialization, but depend on a flag to indicate that the variable is
+   initialized; that's effectively what we do for arrays.  But the current
+   approach works fine for non-arrays, and has no code overhead in the usual
+   case where the temporary destructors are noexcept.  */
 
 static void
 wrap_temporary_cleanups (tree init, tree guard)
 {
+  if (TREE_CODE (guard) == BIND_EXPR)
+    {
+      /* An array cleanup region already encloses any temporary cleanups,
+	 don't wrap it around them again.  */
+      gcc_checking_assert (BIND_EXPR_VEC_DTOR (guard));
+      return;
+    }
   cp_walk_tree_without_duplicates (&init, wrap_cleanups_r, (void *)guard);
 }
 
@@ -7518,8 +7531,8 @@ 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.  But arrays are handled in build_vec_init.  */
-	  if (cleanup && TREE_CODE (type) != ARRAY_TYPE)
+	     its destructor.  */
+	  if (cleanup)
 	    wrap_temporary_cleanups (init, cleanup);
 
 	  gcc_assert (building_stmt_list_p ());
@@ -8367,7 +8380,11 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
   if (cleanups)
     {
       for (tree t : *cleanups)
-	push_cleanup (decl, t, false);
+	{
+	  push_cleanup (decl, t, false);
+	  /* As in initialize_local_var.  */
+	  wrap_temporary_cleanups (init, t);
+	}
       release_tree_vector (cleanups);
     }
 
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index df63e618394..bfe4ad464bf 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4092,6 +4092,7 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
   tbase_init = build_stmt (loc, DECL_EXPR, tbase);
   controller = build3 (BIND_EXPR, void_type_node, tbase, NULL_TREE, NULL_TREE);
   TREE_SIDE_EFFECTS (controller) = 1;
+  BIND_EXPR_VEC_DTOR (controller) = true;
 
   body = build1 (EXIT_EXPR, void_type_node,
 		 build2 (EQ_EXPR, boolean_type_node, tbase,
diff --git a/gcc/testsuite/g++.dg/eh/ref-temp1.C b/gcc/testsuite/g++.dg/eh/ref-temp1.C
new file mode 100644
index 00000000000..2df1a4937b7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/ref-temp1.C
@@ -0,0 +1,51 @@
+// PR c++/53868
+// { dg-do run { target c++11 } }
+
+#if __cplusplus > 201100L
+#define THROWING noexcept(false)
+#else
+#define THROWING
+#endif
+
+extern "C" int printf(const char *, ...);
+extern "C" void abort();
+
+struct SubobjectInA {
+   SubobjectInA();
+   ~SubobjectInA();
+};
+
+int a;
+struct A : SubobjectInA {
+   A() = delete;
+   A(const A &) = delete;
+  A(A &&) = delete;
+   A(int);
+   ~A();
+};
+
+#ifdef DEBUG
+#define TRACE_FUNC( ... ) \
+{   printf("%s\n", __PRETTY_FUNCTION__); __VA_ARGS__   }
+#else
+#define TRACE_FUNC( ... ) \
+{   __VA_ARGS__   }
+#endif
+
+struct Q {
+   Q() : q(0)  TRACE_FUNC()
+   ~Q() THROWING;
+   int q;
+};
+
+int main() {
+   try { const A &a = Q().q; }
+   catch (...) { if (!a) return 0; }
+   abort();
+}
+
+SubobjectInA::SubobjectInA()  TRACE_FUNC()
+SubobjectInA::~SubobjectInA()  TRACE_FUNC()
+A::A(int)  TRACE_FUNC(++a;)
+A::~A()  TRACE_FUNC(--a;)
+Q::~Q() THROWING TRACE_FUNC( throw 0; )
diff --git a/gcc/testsuite/g++.dg/eh/ref-temp2.C b/gcc/testsuite/g++.dg/eh/ref-temp2.C
new file mode 100644
index 00000000000..0c718962d49
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/ref-temp2.C
@@ -0,0 +1,15 @@
+// { dg-do run { target c++11 } }
+
+struct B { B() {} ~B() noexcept(false) { throw 42; } };
+int a;
+struct A { A() { ++a; }; A(B) { ++a; } ~A() { --a; } };
+
+using Arr = A[3];
+
+int main()
+{
+  try {
+    auto&& ref = Arr{B()};
+  } catch (...) { }
+  return a;
+}
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 09/11] c++: destroy retval on throwing cleanup in try [PR33799]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
                   ` (6 preceding siblings ...)
  2022-01-07  0:21 ` [pushed 08/11] c++: clean up ref-extended temp on throwing dtor [PR53868] Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 10/11] c++: nested catch in ctor fn-try-block [PR61611] Jason Merrill
  2022-01-07  0:21 ` [pushed 11/11] c++: when delegating constructor throws [PR103711] Jason Merrill
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

My earlier attempt to fix this bug didn't handle the case where both the
return and the throwing cleanup are within a try-block that catches and
discards the exception.  Fixed by adding the retval cleanup to any
try-blocks as well as the function body.  PR102191 pointed out that we also
weren't handling templates properly, so I moved the call out of the parser.

	PR c++/33799
	PR c++/102191

gcc/cp/ChangeLog:

	* except.c (maybe_splice_retval_cleanup): Check
	current_binding_level.
	* semantics.c (do_poplevel): Call it here.
	* parser.c (cp_parser_compound_statement): Not here.

gcc/testsuite/ChangeLog:

	* g++.dg/eh/return1.C: Add temporary in try block case.
	* g++.dg/cpp2a/constexpr-dtor11.C: New test.
---
 gcc/cp/except.c                               | 29 ++++++----
 gcc/cp/parser.c                               |  3 -
 gcc/cp/semantics.c                            |  2 +
 gcc/testsuite/g++.dg/cpp2a/constexpr-dtor11.C | 12 ++++
 gcc/testsuite/g++.dg/eh/return1.C             | 56 +++++++++++++++++--
 5 files changed, 84 insertions(+), 18 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-dtor11.C

diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index 5c7eeff9035..bcd9f84431b 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -1294,26 +1294,35 @@ maybe_set_retval_sentinel ()
 		 current_retval_sentinel, boolean_true_node);
 }
 
-/* COMPOUND_STMT is the STATEMENT_LIST for the current function body.  If
-   current_retval_sentinel was set in this function, wrap the body in a
-   CLEANUP_STMT to destroy the return value on throw.  */
+/* COMPOUND_STMT is the STATEMENT_LIST for some block.  If COMPOUND_STMT is the
+   current function body or a try block, and current_retval_sentinel was set in
+   this function, wrap the block in a CLEANUP_STMT to destroy the return value
+   on throw.  */
 
 void
 maybe_splice_retval_cleanup (tree compound_stmt)
 {
-  /* If need_retval_cleanup set current_retval_sentinel, wrap the function body
-     in a CLEANUP_STMT to handle destroying the return value.  */
-  if (!DECL_CONSTRUCTOR_P (current_function_decl)
+  /* If we need a cleanup for the return value, add it in at the same level as
+     pushdecl_outermost_localscope.  And also in try blocks.  */
+  bool function_body
+    = (current_binding_level->level_chain
+       && current_binding_level->level_chain->kind == sk_function_parms);
+
+  if ((function_body || current_binding_level->kind == sk_try)
+      && !DECL_CONSTRUCTOR_P (current_function_decl)
       && !DECL_DESTRUCTOR_P (current_function_decl)
       && current_retval_sentinel)
     {
       location_t loc = DECL_SOURCE_LOCATION (current_function_decl);
-
-      /* Add a DECL_EXPR for current_retval_sentinel.  */
       tree_stmt_iterator iter = tsi_start (compound_stmt);
       tree retval = DECL_RESULT (current_function_decl);
-      tree decl_expr = build_stmt (loc, DECL_EXPR, current_retval_sentinel);
-      tsi_link_before (&iter, decl_expr, TSI_SAME_STMT);
+
+      if (function_body)
+	{
+	  /* Add a DECL_EXPR for current_retval_sentinel.  */
+	  tree decl_expr = build_stmt (loc, DECL_EXPR, current_retval_sentinel);
+	  tsi_link_before (&iter, decl_expr, TSI_SAME_STMT);
+	}
 
       /* Skip past other decls, they can't contain a return.  */
       while (TREE_CODE (tsi_stmt (iter)) == DECL_EXPR)
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6b91a0ce491..f40e707d47c 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -12751,9 +12751,6 @@ cp_parser_compound_statement (cp_parser *parser, tree in_statement_expr,
   /* Parse an (optional) statement-seq.  */
   cp_parser_statement_seq_opt (parser, in_statement_expr);
 
-  if (function_body)
-    maybe_splice_retval_cleanup (compound_stmt);
-
   /* Consume the `}'.  */
   braces.require_close (parser);
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 58f45e9c3e3..645654768e3 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -624,6 +624,8 @@ do_poplevel (tree stmt_list)
 {
   tree block = NULL;
 
+  maybe_splice_retval_cleanup (stmt_list);
+
   if (stmts_are_full_exprs_p ())
     block = poplevel (kept_level_p (), 1, 0);
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor11.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor11.C
new file mode 100644
index 00000000000..e371f89a10e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor11.C
@@ -0,0 +1,12 @@
+// PR c++/102191
+// { dg-do compile { target c++20 } }
+
+struct X {
+  struct A {
+    constexpr ~A() noexcept(false) { }
+  };
+
+  constexpr A operator()(auto...) { return {}; }
+};
+
+void f() { []() consteval { X{}(); }(); }
diff --git a/gcc/testsuite/g++.dg/eh/return1.C b/gcc/testsuite/g++.dg/eh/return1.C
index 5ef2f1dee85..ac2225405da 100644
--- a/gcc/testsuite/g++.dg/eh/return1.C
+++ b/gcc/testsuite/g++.dg/eh/return1.C
@@ -11,13 +11,16 @@ int c, d;
 #define THROWS
 #endif
 
+extern "C" int printf (const char *, ...);
+#define DEBUG // printf ("%p %s\n", this, __PRETTY_FUNCTION__)
+
 struct X
 {
-  X(bool throws) : throws_(throws) { ++c; }
-  X(const X& x) : throws_(x.throws_) { ++c; }
+  X(bool throws) : throws_(throws) { ++c; DEBUG; }
+  X(const X& x); // not defined
   ~X() THROWS
   {
-    ++d;
+    ++d; DEBUG;
     if (throws_) { throw 1; }
   }
 private:
@@ -42,6 +45,40 @@ void h()
 #endif
 }
 
+X i()
+{
+  try {
+    X x(true);
+    return X(false);
+  } catch(...) {}
+  return X(false);
+}
+
+X j()
+{
+  try {
+    return X(true),X(false);
+  } catch(...) {}
+  return X(false);
+}
+
+template <class T>
+T k()
+{
+  try {
+    return T(true),T(false);
+  } catch (...) {}
+  return T(true),T(false);
+}
+
+X l() try { return X(true),X(false); }
+  catch (...) { return X(true),X(false); }
+
+template <class T>
+T m()
+  try { return T(true),T(false); }
+  catch (...) { return T(true),T(false); }
+
 int main()
 {
   try { f(); }
@@ -53,6 +90,15 @@ int main()
   try { h(); }
   catch (...) {}
 
-  if (c != d)
-    throw;
+  try { i(); }
+  catch (...) {}
+
+  try { j(); } catch (...) {}
+
+  try { k<X>(); } catch (...) {}
+
+  try { l(); } catch (...) {}
+  try { m<X>(); } catch (...) {}
+
+  return c - d;
 }
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 10/11] c++: nested catch in ctor fn-try-block [PR61611]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
                   ` (7 preceding siblings ...)
  2022-01-07  0:21 ` [pushed 09/11] c++: destroy retval on throwing cleanup in try [PR33799] Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  2022-01-07  0:21 ` [pushed 11/11] c++: when delegating constructor throws [PR103711] Jason Merrill
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

Being in_function_try_handler isn't enough to satisfy the condition of
reaching the end of such a handler; in this case, we're reaching the end of
a handler within that handler, so we don't want the special semantics.

	PR c++/61611

gcc/cp/ChangeLog:

	* except.c (in_nested_catch): New.
	(expand_end_catch_block): Check it.

gcc/testsuite/ChangeLog:

	* g++.dg/eh/ctor-fntry1.C: New test.
---
 gcc/cp/except.c                       | 20 +++++++++++++++++++-
 gcc/testsuite/g++.dg/eh/ctor-fntry1.C | 23 +++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/eh/ctor-fntry1.C

diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index bcd9f84431b..9b746be231a 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -448,6 +448,23 @@ expand_start_catch_block (tree decl)
   return type;
 }
 
+/* True if we are in a catch block within a catch block.  Assumes that we are
+   in function scope.  */
+
+static bool
+in_nested_catch (void)
+{
+  int catches = 0;
+
+  /* Scan through the template parameter scopes.  */
+  for (cp_binding_level *b = current_binding_level;
+       b->kind != sk_function_parms;
+       b = b->level_chain)
+    if (b->kind == sk_catch
+	&& ++catches == 2)
+      return true;
+  return false;
+}
 
 /* Call this to end a catch block.  Its responsible for emitting the
    code to handle jumping back to the correct place, and for emitting
@@ -463,7 +480,8 @@ expand_end_catch_block (void)
      a handler of the function-try-block of a constructor or destructor.  */
   if (in_function_try_handler
       && (DECL_CONSTRUCTOR_P (current_function_decl)
-	  || DECL_DESTRUCTOR_P (current_function_decl)))
+	  || DECL_DESTRUCTOR_P (current_function_decl))
+      && !in_nested_catch ())
     {
       tree rethrow = build_throw (input_location, NULL_TREE);
       /* Disable all warnings for the generated rethrow statement.  */
diff --git a/gcc/testsuite/g++.dg/eh/ctor-fntry1.C b/gcc/testsuite/g++.dg/eh/ctor-fntry1.C
new file mode 100644
index 00000000000..0c783bb45ee
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/ctor-fntry1.C
@@ -0,0 +1,23 @@
+// PR c++/61611
+// { dg-do run }
+
+struct A { };
+struct B { };
+
+struct Test
+{
+  Test()
+  try { throw A(); }
+  catch(const A&)
+    {
+      try { throw B(); }
+      catch(const B&) { }
+    }
+};
+
+int
+main()
+{
+  try { Test x; }
+  catch(const A&) { }
+}
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed 11/11] c++: when delegating constructor throws [PR103711]
  2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
                   ` (8 preceding siblings ...)
  2022-01-07  0:21 ` [pushed 10/11] c++: nested catch in ctor fn-try-block [PR61611] Jason Merrill
@ 2022-01-07  0:21 ` Jason Merrill
  9 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-01-07  0:21 UTC (permalink / raw)
  To: gcc-patches

We were always calling the complete destructor if the target constructor
throws, even if we were calling the base constructor.

	PR c++/103711

gcc/cp/ChangeLog:

	* init.c (perform_target_ctor): Select destructor by in_chrg.

gcc/testsuite/ChangeLog:

	* g++.dg/eh/delegating1.C: New test.
---
 gcc/cp/init.c                         | 10 ++++++++++
 gcc/testsuite/g++.dg/eh/delegating1.C | 28 +++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/eh/delegating1.C

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index bfe4ad464bf..c932699ffa6 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -545,6 +545,16 @@ perform_target_ctor (tree init)
 				|LOOKUP_NONVIRTUAL
 				|LOOKUP_DESTRUCTOR,
 				0, tf_warning_or_error);
+      if (DECL_HAS_IN_CHARGE_PARM_P (current_function_decl))
+	{
+	  tree base = build_delete (input_location,
+				    type, decl, sfk_base_destructor,
+				    LOOKUP_NORMAL
+				    |LOOKUP_NONVIRTUAL
+				    |LOOKUP_DESTRUCTOR,
+				    0, tf_warning_or_error);
+	  expr = build_if_in_charge (expr, base);
+	}
       if (expr != error_mark_node
 	  && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
 	finish_eh_cleanup (expr);
diff --git a/gcc/testsuite/g++.dg/eh/delegating1.C b/gcc/testsuite/g++.dg/eh/delegating1.C
new file mode 100644
index 00000000000..c33374a3b6b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/delegating1.C
@@ -0,0 +1,28 @@
+// PR c++/103711
+// { dg-do run { target c++11 } }
+
+int constructions = 0;
+int destructions = 0;
+
+struct A
+{
+  A() { constructions++; }
+  virtual ~A() { destructions++; }
+};
+
+struct B : public virtual A
+{
+  B(int) { }
+  B() : B(1) { throw -1; }
+  virtual ~B() = default;
+};
+
+struct C : public B { };
+
+int main() {
+  try {
+    C c;
+  }
+  catch (int) {}
+  return (constructions - destructions);
+}
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-01-07  0:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-07  0:21 [pushed 01/11] c++: don't preevaluate new-initializer Jason Merrill
2022-01-07  0:21 ` [pushed 02/11] c++: loop over array elts w/o explicit init [PR92385] Jason Merrill
2022-01-07  0:21 ` [pushed 03/11] c++: temporary lifetime with aggregate init [PR94041] Jason Merrill
2022-01-07  0:21 ` [pushed 04/11] c++: temporary lifetime with array aggr " Jason Merrill
2022-01-07  0:21 ` [pushed 05/11] c++: EH and partially constructed aggr temp [PR66139] Jason Merrill
2022-01-07  0:21 ` [pushed 06/11] c++: don't cleanup the last aggregate elt Jason Merrill
2022-01-07  0:21 ` [pushed 07/11] c++: keep destroying array after one dtor throws [PR66451] Jason Merrill
2022-01-07  0:21 ` [pushed 08/11] c++: clean up ref-extended temp on throwing dtor [PR53868] Jason Merrill
2022-01-07  0:21 ` [pushed 09/11] c++: destroy retval on throwing cleanup in try [PR33799] Jason Merrill
2022-01-07  0:21 ` [pushed 10/11] c++: nested catch in ctor fn-try-block [PR61611] Jason Merrill
2022-01-07  0:21 ` [pushed 11/11] c++: when delegating constructor throws [PR103711] Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).