[pushed,01/11] c++: don't preevaluate new-initializer

Message ID 20220107002156.2992278-1-jason@redhat.com
State Committed
Commit ad64a821970ef5547d77ece22d302b0e599ae0d2
Headers
Series [pushed,01/11] c++: don't preevaluate new-initializer |

Commit Message

Jason Merrill Jan. 7, 2022, 12:21 a.m. UTC
  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


base-commit: 11ce8d04f29417f2541d9b9bbfb54b3b26d7a90d
  

Patch

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()