[PATCH] coroutines: Update TREE_SIDE_EFFECTS on inserted bind exprs.

[Iain Sandoe]

Hi,

There are several places where we insert bind expressions while
making the coroutine AST transforms.  These should be marked as
having side-effects where relevant, which had been omitted.  This
leads to at least one failure in the cppcoros test suite, where a loop
body is dropped in gimplification because it is not marked.

tested so far on x86-64-darwin16,
OK if it passes on Linux too, for master, 10.2?
thanks
Iain

gcc/cp/ChangeLog:

2020-05-08  Iain Sandoe  <iain@sandoe.co.uk>

	PR c++/95003
	* coroutines.cc (build_actor_fn): Ensure that bind scopes
	are marked as having side-effects where necessary.
	(replace_statement_captures): Likewise.
	(morph_fn_to_coro): Likewise.

gcc/testsuite/ChangeLog:

2020-05-08  Iain Sandoe  <iain@sandoe.co.uk>

	PR c++/95003
	* g++.dg/coroutines/torture/pr95003.C: New test.
---
 gcc/cp/coroutines.cc                          | 15 ++++--
 .../g++.dg/coroutines/torture/pr95003.C       | 50 +++++++++++++++++++
 2 files changed, 60 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/pr95003.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 8f0f2d5960d..730e6fef82a 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1973,8 +1973,6 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody,
   current_stmt_tree ()->stmts_are_full_exprs_p = 1;
   tree stmt = begin_compound_stmt (BCS_FN_BODY);
 
-  /* ??? Can we dispense with the enclosing bind if the function body does
-     not start with a bind_expr? (i.e. there's no contained scopes).  */
   tree actor_bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
   tree top_block = make_node (BLOCK);
   BIND_EXPR_BLOCK (actor_bind) = top_block;
@@ -2425,8 +2423,8 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody,
 		       continue_label, continuation, 2};
   cp_walk_tree (&actor_body, await_statement_expander, &data, NULL);
 
-  actor_body = pop_stmt_list (actor_body);
-  BIND_EXPR_BODY (actor_bind) = actor_body;
+  BIND_EXPR_BODY (actor_bind) = pop_stmt_list (actor_body);
+  TREE_SIDE_EFFECTS (actor_bind) = true;
 
   finish_compound_stmt (stmt);
   DECL_SAVED_TREE (actor) = pop_stmt_list (actor_outer);
@@ -2891,6 +2889,7 @@ replace_statement_captures (tree *stmt, void *d)
 	}
     }
   BIND_EXPR_BLOCK (aw_bind) = b_block;
+  TREE_SIDE_EFFECTS (aw_bind) = TREE_SIDE_EFFECTS (BIND_EXPR_BODY (aw_bind));
   *stmt = aw_bind;
 }
 
@@ -3613,10 +3612,13 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	{
 	  tree tlist = NULL_TREE;
 	  append_to_statement_list_force (fnbody, &tlist);
+	  TREE_SIDE_EFFECTS (tlist) = TREE_SIDE_EFFECTS (fnbody);
 	  BIND_EXPR_BODY (update_body) = tlist;
 	}
       tree new_body_list = NULL_TREE;
-      append_to_statement_list_force (update_body, &new_body_list);
+      TREE_SIDE_EFFECTS (update_body) = true;
+      append_to_statement_list (update_body, &new_body_list);
+      TREE_SIDE_EFFECTS (new_body_list) = true;
       fnbody = new_body_list;
     }
 
@@ -4323,7 +4325,9 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   /* Finish up the ramp function.  */
   BIND_EXPR_VARS (gro_context_bind) = gro_bind_vars;
   BIND_EXPR_BODY (gro_context_bind) = pop_stmt_list (gro_context_body);
+  TREE_SIDE_EFFECTS (gro_context_bind) = true;
   BIND_EXPR_BODY (ramp_bind) = pop_stmt_list (ramp_body);
+  TREE_SIDE_EFFECTS (ramp_bind) = true;
 
   /* We know the "real" promise and have a frame layout with a slot for each
      suspend point, so we can build an actor function (which contains the
@@ -4442,6 +4446,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	  BLOCK_SUPERCONTEXT (replace_blk) = tcb_block;
 	  BLOCK_SUBBLOCKS (tcb_block) = replace_blk;
 	  BIND_EXPR_BLOCK (fnbody) = tcb_block;
+	  TREE_SIDE_EFFECTS (fnbody) = true;
 	}
     }
   else if (pedantic)
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/pr95003.C b/gcc/testsuite/g++.dg/coroutines/torture/pr95003.C
new file mode 100644
index 00000000000..eda785827ec
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/pr95003.C
@@ -0,0 +1,50 @@
+// { dg-do run }
+
+#include "../coro.h"
+#include "../coro1-ret-int-yield-int.h"
+
+// This tests that, when we insert bind scopes to contain variables that
+// have been promoted from compiler temporaries to extend their lifetimes
+// to a containing full expression, the inserted bind scopes have their
+// tree-side-effects set.
+
+struct Awaitable {
+  int v;
+  Awaitable (int _v) : v(_v) {}
+  bool await_ready() { return false; }
+  void await_suspend(std::coroutine_handle<coro1::promise_type>) {}
+  int await_resume() { return v; }
+  auto operator co_await() { return *this; }
+};
+
+coro1
+my_coro
+(int x)
+{
+  int sum = 0;
+  for (unsigned i = 0; i < 100; ++i) {
+    sum += co_await Awaitable{x+1};
+  }
+  co_return sum;
+}
+
+int main ()
+{
+  PRINT ("main: create coro1");
+  struct coro1 f_coro = my_coro (0);
+
+  PRINT ("main: OK -- looping");
+
+  do {
+    f_coro.handle.resume();
+  } while (!f_coro.handle.done());
+
+  int y = f_coro.handle.promise().get_value();
+  if (y != 100)
+    {
+      PRINTF ("main: y is wrong : %d, should be 100\n", y);
+      abort ();
+    }
+  puts ("main: done");
+  return 0;
+}
-- 
2.24.1