Insert default return_void at the end of coroutine body

Exception in coroutine is not correctly handled because the default
return_void call is now inserted before the finish suspend point,
rather than at the end of the original coroutine body.  This patch
fixes the issue by expanding code as following:
  co_await promise.initial_suspend();
  try {
    // The original coroutine body

    promise.return_void(); // The default return_void call.
  } catch (...) {
    promise.unhandled_exception();
  }
  final_suspend:
  // ...

gcc/cp/
    * coroutines.cc (build_actor_fn): Factor out code inserting the
    default return_void call to...
    (morph_fn_to_coro): ...here, also hoist local var declarations.

gcc/testsuite/
    * g++.dg/coroutines/torture/co-ret-15-default-return_void.C: New.

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index f32c27e..128880b 100644 (file)
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,9 @@
+2020-03-09  Bin Cheng  <bin.cheng@linux.alibaba.com>
+
+       * coroutines.cc (build_actor_fn): Factor out code inserting the
+       default return_void call to...
+       (morph_fn_to_coro): ...here, also hoist local var declarations.
+
 2020-03-08  Patrick Palka  <ppalka@redhat.com>
 
        PR c++/93729

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index bca4f1e..920575b 100644 (file)
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2161,21 +2161,6 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody,
   r = coro_build_expr_stmt (initial_await, loc);
   add_stmt (r);
 
-  /* Now we've built the promise etc, process fnbody for co_returns.
-     We want the call to return_void () below and it has no params so
-     we can create it once here.
-     Calls to return_value () will have to be checked and created as
-     required.  */
-
-  tree return_void = NULL_TREE;
-  tree rvm
-    = lookup_promise_method (orig, coro_return_void_identifier, loc,
-                            /*musthave=*/false);
-  if (rvm && rvm != error_mark_node)
-    return_void
-      = build_new_method_call (ap, rvm, NULL, NULL_TREE, LOOKUP_NORMAL, NULL,
-                              tf_warning_or_error);
-
   /* co_return branches to the final_suspend label, so declare that now.  */
   tree fs_label = create_named_label_with_ctx (loc, "final.suspend", actor);
 
@@ -2190,15 +2175,6 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody,
   /* Add in our function body with the co_returns rewritten to final form.  */
   add_stmt (fnbody);
 
-  /* [stmt.return.coroutine] (2.2 : 3) if p.return_void() is a valid
-     expression, flowing off the end of a coroutine is equivalent to
-     co_return; otherwise UB.
-     We just inject the call to p.return_void() here, and fall through to
-     the final_suspend: label (eliding the goto).  If the function body has
-     a co_return, then this statement will be unreachable and DCEd.  */
-  if (return_void != NULL_TREE)
-    add_stmt (return_void);
-
   /* Final suspend starts here.  */
   r = build_stmt (loc, LABEL_EXPR, fs_label);
   add_stmt (r);
@@ -3815,18 +3791,48 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
       BIND_EXPR_BLOCK (first) = replace_blk;
     }
 
+  /* actor's version of the promise.  */
+  tree actor_frame = build1_loc (fn_start, INDIRECT_REF, coro_frame_type,
+                                DECL_ARGUMENTS (actor));
+  tree ap_m = lookup_member (coro_frame_type, get_identifier ("__p"), 1, 0,
+                            tf_warning_or_error);
+  tree ap = build_class_member_access_expr (actor_frame, ap_m, NULL_TREE,
+                                           false, tf_warning_or_error);
+
+  /* Now we've built the promise etc, process fnbody for co_returns.
+     We want the call to return_void () below and it has no params so
+     we can create it once here.
+     Calls to return_value () will have to be checked and created as
+     required.  */
+
+  tree return_void = NULL_TREE;
+  tree rvm
+    = lookup_promise_method (orig, coro_return_void_identifier, fn_start,
+                            /*musthave=*/false);
+  if (rvm && rvm != error_mark_node)
+    return_void
+      = build_new_method_call (ap, rvm, NULL, NULL_TREE, LOOKUP_NORMAL, NULL,
+                              tf_warning_or_error);
+
+  /* [stmt.return.coroutine] (2.2 : 3) if p.return_void() is a valid
+     expression, flowing off the end of a coroutine is equivalent to
+     co_return; otherwise UB.
+     We just inject the call to p.return_void() here, and fall through to
+     the final_suspend: label (eliding the goto).  If the function body has
+     a co_return, then this statement will be unreachable and DCEd.  */
+  if (return_void != NULL_TREE)
+    {
+      tree append = push_stmt_list ();
+      add_stmt (fnbody);
+      add_stmt (return_void);
+      fnbody = pop_stmt_list(append);
+    }
+
   if (flag_exceptions)
     {
       tree ueh_meth
        = lookup_promise_method (orig, coro_unhandled_exception_identifier,
                                 fn_start, /*musthave=*/true);
-      /* actor's version of the promise.  */
-      tree actor_frame = build1_loc (fn_start, INDIRECT_REF, coro_frame_type,
-                                    DECL_ARGUMENTS (actor));
-      tree ap_m = lookup_member (coro_frame_type, get_identifier ("__p"), 1, 0,
-                                tf_warning_or_error);
-      tree ap = build_class_member_access_expr (actor_frame, ap_m, NULL_TREE,
-                                               false, tf_warning_or_error);
       /* Build promise.unhandled_exception();  */
       tree ueh
        = build_new_method_call (ap, ueh_meth, NULL, NULL_TREE, LOOKUP_NORMAL,

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index b72b9cb..cf7243d 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-03-09  Bin Cheng  <bin.cheng@linux.alibaba.com>
+
+       * g++.dg/coroutines/torture/co-ret-15-default-return_void.C: New.
+
 2020-03-09  Kewen Lin  <linkw@gcc.gnu.org>
 
        PR testsuite/94019

diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-ret-15-default-return_void.C b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-15-default-return_void.C
new file mode 100644 (file)
index 0000000..e600fea
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-15-default-return_void.C
@@ -0,0 +1,55 @@
+// { dg-do run }
+//
+// Check if default return_void is insert at correct position.
+#include <cassert>
+#include "../coro.h"
+
+class resumable {
+public:
+  class promise_type;
+  using coro_handle = std::coroutine_handle<promise_type>;
+  resumable(coro_handle handle) : handle_(handle) { assert(handle); }
+  resumable(resumable&) = delete;
+  resumable(resumable&&) = delete;
+  bool resume() {
+    if (!handle_.done())
+      handle_.resume();
+    return !handle_.done();
+  }
+  int recent_val();
+  ~resumable() { handle_.destroy(); }
+private:
+  coro_handle handle_;
+};
+
+class resumable::promise_type {
+public:
+  friend class resumable;
+  using coro_handle = std::coroutine_handle<promise_type>;
+  auto get_return_object() { return coro_handle::from_promise(*this); }
+  auto initial_suspend() { return std::suspend_always(); }
+  auto final_suspend() { return std::suspend_always(); }
+  void return_void() { value_ = -1; }
+  void unhandled_exception() {}
+private:
+  int value_ = 0;
+};
+
+int resumable::recent_val() {return handle_.promise().value_;}
+
+resumable foo(int n){
+  co_await std::suspend_always();
+  throw 1;
+}
+
+int bar (int n) {
+  resumable res = foo(n);
+  while(res.resume());
+  return res.recent_val();
+}
+
+int main() {
+  int res = bar(3);
+  assert(res == 0);
+  return 0;
+}