c++: change implementation of -frange-for-ext-temps [PR118574]

The implementation in r15-3840 used a novel technique of wrapping the entire
range-for loop in a CLEANUP_POINT_EXPR, which confused the coroutines
transformation.  Instead let's use the existing extend_ref_init_temps
mechanism.

This does not revert all of r15-3840, only the parts that change how
CLEANUP_POINT_EXPRs are applied to range-for declarations.

	PR c++/118574
	PR c++/107637

gcc/cp/ChangeLog:

	* call.cc (struct extend_temps_data): New.
	(extend_temps_r, extend_all_temps): New.
	(set_up_extended_ref_temp): Handle tree walk case.
	(extend_ref_init_temps): Cal extend_all_temps.
	* decl.cc (initialize_local_var): Revert ext-temps change.
	* parser.cc (cp_convert_range_for): Likewise.
	(cp_parser_omp_loop_nest): Likewise.
	* pt.cc (tsubst_stmt): Likewise.
	* semantics.cc (finish_for_stmt): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/coroutines/range-for1.C: New test.

Author: jicama

gcc/cp/call.cc

@@ -14154,6 +14154,20 @@ make_temporary_var_for_ref_to_temp (tree decl, tree type)
   return pushdecl (var);
 }
 
+/* Data for extend_temps_r, mostly matching the parameters of
+   extend_ref_init_temps.  */
+
+struct extend_temps_data
+{
+  tree decl;
+  tree init;
+  vec<tree, va_gc> **cleanups;
+  tree* cond_guard;
+  hash_set<tree> *pset;
+};
+
+static tree extend_temps_r (tree *, int *, void *);
+
 /* EXPR is the initializer for a variable DECL of reference or
    std::initializer_list type.  Create, push and return a new VAR_DECL
    for the initializer so that it will live as long as DECL.  Any
@@ -14162,7 +14176,8 @@ make_temporary_var_for_ref_to_temp (tree decl, tree type)
 
 static tree
 set_up_extended_ref_temp (tree decl, tree expr, vec<tree, va_gc> **cleanups,
-			  tree *initp, tree *cond_guard)
+			  tree *initp, tree *cond_guard,
+			  extend_temps_data *walk_data)
 {
   tree init;
   tree type;
@@ -14198,10 +14213,16 @@ set_up_extended_ref_temp (tree decl, tree expr, vec<tree, va_gc> **cleanups,
       suppress_warning (decl);
     }
 
-  /* Recursively extend temps in this initializer.  */
-  TARGET_EXPR_INITIAL (expr)
-    = extend_ref_init_temps (decl, TARGET_EXPR_INITIAL (expr), cleanups,
-			     cond_guard);
+  /* Recursively extend temps in this initializer.  The recursion needs to come
+     after creating the variable to conform to the mangling ABI, and before
+     maybe_constant_init because the extension might change its result.  */
+  if (walk_data)
+    cp_walk_tree (&TARGET_EXPR_INITIAL (expr), extend_temps_r,
+		  walk_data, walk_data->pset);
+  else
+    TARGET_EXPR_INITIAL (expr)
+      = extend_ref_init_temps (decl, TARGET_EXPR_INITIAL (expr), cleanups,
+			       cond_guard);
 
   /* Any reference temp has a non-trivial initializer.  */
   DECL_NONTRIVIALLY_INITIALIZED_P (var) = true;
@@ -14801,7 +14822,8 @@ extend_ref_init_temps_1 (tree decl, tree init, vec<tree, va_gc> **cleanups,
   if (TREE_CODE (*p) == TARGET_EXPR)
     {
       tree subinit = NULL_TREE;
-      *p = set_up_extended_ref_temp (decl, *p, cleanups, &subinit, cond_guard);
+      *p = set_up_extended_ref_temp (decl, *p, cleanups, &subinit,
+				     cond_guard, nullptr);
       recompute_tree_invariant_for_addr_expr (sub);
       if (init != sub)
 	init = fold_convert (TREE_TYPE (init), sub);
@@ -14811,6 +14833,81 @@ extend_ref_init_temps_1 (tree decl, tree init, vec<tree, va_gc> **cleanups,
   return init;
 }
 
+/* Tree walk function for extend_all_temps.  Generally parallel to
+   extend_ref_init_temps_1, but adapted for walk_tree.  */
+
+tree
+extend_temps_r (tree *tp, int *walk_subtrees, void *data)
+{
+  extend_temps_data *d = (extend_temps_data *)data;
+
+  if (TYPE_P (*tp) || TREE_CODE (*tp) == CLEANUP_POINT_EXPR)
+    {
+      *walk_subtrees = 0;
+      return NULL_TREE;
+    }
+
+  if (TREE_CODE (*tp) == COND_EXPR)
+    {
+      cp_walk_tree (&TREE_OPERAND (*tp, 0), extend_temps_r, d, d->pset);
+
+      auto walk_arm = [d](tree &op)
+      {
+	tree cur_cond_guard = NULL_TREE;
+	auto ov = make_temp_override (d->cond_guard, &cur_cond_guard);
+	cp_walk_tree (&op, extend_temps_r, d, d->pset);
+	if (cur_cond_guard)
+	  {
+	    tree set = build2 (MODIFY_EXPR, boolean_type_node,
+			       cur_cond_guard, boolean_true_node);
+	    op = add_stmt_to_compound (set, op);
+	  }
+      };
+      walk_arm (TREE_OPERAND (*tp, 1));
+      walk_arm (TREE_OPERAND (*tp, 2));
+
+      *walk_subtrees = 0;
+      return NULL_TREE;
+    }
+
+  if (TREE_CODE (*tp) == ADDR_EXPR
+      /* A discarded-value temporary.  */
+      || (TREE_CODE (*tp) == CONVERT_EXPR
+	  && VOID_TYPE_P (TREE_TYPE (*tp))))
+    {
+      tree *p;
+      for (p = &TREE_OPERAND (*tp, 0);
+	   TREE_CODE (*p) == COMPONENT_REF || TREE_CODE (*p) == ARRAY_REF; )
+	p = &TREE_OPERAND (*p, 0);
+      if (TREE_CODE (*p) == TARGET_EXPR)
+	{
+	  tree subinit = NULL_TREE;
+	  *p = set_up_extended_ref_temp (d->decl, *p, d->cleanups, &subinit,
+					 d->cond_guard, d);
+	  if (TREE_CODE (*tp) == ADDR_EXPR)
+	    recompute_tree_invariant_for_addr_expr (*tp);
+	  if (subinit)
+	    *tp = cp_build_compound_expr (subinit, *tp, tf_none);
+	}
+    }
+
+  /* TARGET_EXPRs that aren't handled by the above are implementation details
+     that shouldn't be ref-extended, like the build_vec_init iterator.  */
+
+  return NULL_TREE;
+}
+
+/* Extend all the temporaries in a for-range-initializer.  */
+
+static tree
+extend_all_temps (tree decl, tree init, vec<tree, va_gc> **cleanups)
+{
+  hash_set<tree> pset;
+  extend_temps_data d = { decl, init, cleanups, nullptr, &pset };
+  cp_walk_tree (&init, extend_temps_r, &d, &pset);
+  return init;
+}
+
 /* INIT is part of the initializer for DECL.  If there are any
    reference or initializer lists being initialized, extend their
    lifetime to match that of DECL.  */
@@ -14823,11 +14920,13 @@ extend_ref_init_temps (tree decl, tree init, vec<tree, va_gc> **cleanups,
   if (processing_template_decl)
     return init;
 
-  /* P2718R0 - ignore temporaries in C++23 for-range-initializer, those
-     have all extended lifetime.  */
+  /* P2718R0 - in C++23 for-range-initializer, extend all temps.  */
   if (DECL_NAME (decl) == for_range__identifier
       && flag_range_for_ext_temps)
-    return init;
+    {
+      gcc_checking_assert (!cond_guard);
+      return extend_all_temps (decl, init, cleanups);
+    }
 
   maybe_warn_dangling_reference (decl, init);
 

gcc/cp/decl.cc

@@ -8310,11 +8310,6 @@ initialize_local_var (tree decl, tree init, bool decomp)
 	     code emitted by cp_finish_decomp.  */
 	  if (decomp)
 	    current_stmt_tree ()->stmts_are_full_exprs_p = 0;
-	  /* P2718R0 - avoid CLEANUP_POINT_EXPR for range-for-initializer,
-	     temporaries from there should have lifetime extended.  */
-	  else if (DECL_NAME (decl) == for_range__identifier
-		   && flag_range_for_ext_temps)
-	    current_stmt_tree ()->stmts_are_full_exprs_p = 0;
 	  else
 	    current_stmt_tree ()->stmts_are_full_exprs_p = 1;
 	  finish_expr_stmt (init);

gcc/cp/parser.cc

@@ -14844,15 +14844,6 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr,
 	{
 	  range_temp = build_range_temp (range_expr);
 	  pushdecl (range_temp);
-	  if (flag_range_for_ext_temps)
-	    {
-	      /* P2718R0 - put the range_temp declaration and everything
-		 until end of the range for body into an extra STATEMENT_LIST
-		 which will have CLEANUP_POINT_EXPR around it, so that all
-		 temporaries are destroyed at the end of it.  */
-	      gcc_assert (FOR_INIT_STMT (statement) == NULL_TREE);
-	      FOR_INIT_STMT (statement) = push_stmt_list ();
-	    }
 	  cp_finish_decl (range_temp, range_expr,
 			  /*is_constant_init*/false, NULL_TREE,
 			  LOOKUP_ONLYCONVERTING);
@@ -46719,20 +46710,12 @@ cp_parser_omp_loop_nest (cp_parser *parser, bool *if_p)
 
   /* Pop and remember the init block.  */
   if (sl)
-    {
-      sl = pop_stmt_list (sl);
-      /* P2718R0 - Add CLEANUP_POINT_EXPR so that temporaries in
-	 for-range-initializer whose lifetime is extended are destructed
-	 here.  */
-      if (flag_range_for_ext_temps
-	  && is_range_for
-	  && !processing_template_decl)
-	sl = maybe_cleanup_point_expr_void (sl);
-      add_stmt (sl);
-    }
+    add_stmt (pop_stmt_list (sl));
+
   tree range_for_decl[3] = { NULL_TREE, NULL_TREE, NULL_TREE };
   if (is_range_for && !processing_template_decl)
     find_range_for_decls (range_for_decl);
+
   finish_compound_stmt (init_scope);
   init_block = pop_stmt_list (init_block);
   omp_for_parse_state->init_blockv[depth] = init_block;

gcc/cp/pt.cc

@@ -19478,18 +19478,6 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	RECUR (OMP_FOR_PRE_BODY (t));
 	pre_body = pop_stmt_list (pre_body);
 
-	tree sl = NULL_TREE;
-	if (flag_range_for_ext_temps
-	    && OMP_FOR_INIT (t) != NULL_TREE
-	    && !processing_template_decl)
-	  for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (t)); i++)
-	    if (TREE_VEC_ELT (OMP_FOR_INIT (t), i)
-		&& TREE_VEC_ELT (OMP_FOR_COND (t), i) == global_namespace)
-	      {
-		sl = push_stmt_list ();
-		break;
-	      }
-
 	if (OMP_FOR_INIT (t) != NULL_TREE)
 	  for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (t)); i++)
 	    {
@@ -19545,16 +19533,6 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    add_stmt (t);
 	  }
 
-	if (sl)
-	  {
-	    /* P2718R0 - Add CLEANUP_POINT_EXPR so that temporaries in
-	       for-range-initializer whose lifetime is extended are destructed
-	       here.  */
-	    sl = pop_stmt_list (sl);
-	    sl = maybe_cleanup_point_expr_void (sl);
-	    add_stmt (sl);
-	  }
-
 	add_stmt (finish_omp_for_block (finish_omp_structured_block (stmt),
 					t));
 	pop_omp_privatization_clauses (r);

gcc/cp/semantics.cc

@@ -1736,19 +1736,6 @@ finish_for_stmt (tree for_stmt)
   tree range_for_decl[3] = { NULL_TREE, NULL_TREE, NULL_TREE };
   find_range_for_decls (range_for_decl);
 
-  /* P2718R0 - Add CLEANUP_POINT_EXPR so that temporaries in
-     for-range-initializer whose lifetime is extended are destructed
-     here.  */
-  if (flag_range_for_ext_temps
-      && range_for_decl[0]
-      && FOR_INIT_STMT (for_stmt))
-    {
-      tree stmt = pop_stmt_list (FOR_INIT_STMT (for_stmt));
-      FOR_INIT_STMT (for_stmt) = NULL_TREE;
-      stmt = maybe_cleanup_point_expr_void (stmt);
-      add_stmt (stmt);
-    }
-
   add_stmt (do_poplevel (scope));
 
   /* If we're being called from build_vec_init, don't mess with the names of

gcc/testsuite/g++.dg/coroutines/range-for1.C

@@ -0,0 +1,69 @@
+// PR c++/118574
+// { dg-do run { target c++20 } }
+// { dg-additional-options -frange-for-ext-temps }
+
+#include <coroutine>
+
+struct A {
+  const char **a = nullptr;
+  int n = 0;
+  void push_back (const char *x) { if (!a) a = new const char *[2]; a[n++] = x; }
+  const char **begin () const { return a; }
+  const char **end () const { return a + n; }
+  ~A () { delete[] a; }
+};
+
+struct B {
+  long ns;
+  bool await_ready () const noexcept { return false; }
+  void await_suspend (std::coroutine_handle<> h) const noexcept {
+    volatile int v = 0;
+    while (v < ns)
+      v = v + 1;
+    h.resume ();
+  }
+  void await_resume () const noexcept {}
+};
+
+struct C {
+  struct promise_type {
+    const char *value;
+    std::suspend_never initial_suspend () { return {}; }
+    std::suspend_always final_suspend () noexcept { return {}; }
+    void return_value (const char *v) { value = v; }
+    void unhandled_exception () { __builtin_abort (); }
+    C get_return_object () { return C{this}; }
+  };
+  promise_type *p;
+  explicit C (promise_type *p) : p(p) {}
+  const char *get () { return p->value; }
+};
+
+A
+foo ()
+{
+  A a;
+  a.push_back ("foo");
+  a.push_back ("bar");
+  return a;
+}
+
+C
+bar ()
+{
+  A ret;
+  for (const auto &item : foo ())
+    {
+      co_await B{200000};
+      ret.push_back (item);
+    }
+  co_return "foobar";
+}
+
+int
+main ()
+{
+  auto task = bar ();
+  if (__builtin_strcmp (task.get (), "foobar"))
+    __builtin_abort ();
+}