coroutines: Fix handling of ramp return value [PR94661]

Coroutine ramp functions have synthesised return values (the
user-authored function body cannot have an explicit 'return').
The current implementation attempts to optimise by building
the return in-place, in the manner of C++17 code. Clearly,
that was too ambitious and the fix builds a target expr for
the constructed version and passes that to finish_return_stmt.

This also means that we now get the same error messages for
implicit use of deleted CTORs etc.

gcc/cp/ChangeLog:

2020-04-21 Iain Sandoe <iain@sandoe.co.uk>

	PR c++/94661
	* coroutines.cc (morph_fn_to_coro): Simplify return
	value computation.

gcc/testsuite/ChangeLog:

	2020-04-21 Iain Sandoe <iain@sandoe.co.uk>

	PR c++/94661
	* g++.dg/coroutines/ramp-return-a.C: New test.
	* g++.dg/coroutines/ramp-return-b.C: New test.
	* g++.dg/coroutines/ramp-return-c.C: New test.

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 53daab16c1542db43f8a8841350f8da9b7ff1d02..5b2bff8c561b2d6c42b3749c84212420fb5ac8f6 100644 (file)
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,9 @@
+2020-04-21 Iain Sandoe <iain@sandoe.co.uk>
+
+       PR c++/94661
+       * coroutines.cc (morph_fn_to_coro): Simplify return
+       value computation.
+
 2020-04-17  Marek Polacek  <polacek@redhat.com>
 
        PR c++/94592

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index ceb8daa4e460160f90341664a56f58fb2aa03269..30676eba6c23f75042aa48070bbedb8c62db030f 100644 (file)
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3726,23 +3726,14 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
     }
 
   tree gro_context_body = push_stmt_list ();
-  tree gro, gro_bind_vars;
-  if (same_type_p (TREE_TYPE (get_ro), fn_return_type))
-    {
-      gro = DECL_RESULT (orig);
-      gro_bind_vars = NULL_TREE; /* We don't need a separate var.  */
-    }
-  else
-    {
-      gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"),
-                            TREE_TYPE (TREE_OPERAND (get_ro, 0)));
-      DECL_CONTEXT (gro) = current_scope ();
-      r = build_stmt (fn_start, DECL_EXPR, gro);
-      add_stmt (r);
-      gro_bind_vars = gro; /* We need a temporary var.  */
-    }
-
-  /* Initialize our actual var.  */
+  tree gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"),
+                             TREE_TYPE (get_ro));
+  DECL_CONTEXT (gro) = current_scope ();
+  add_decl_expr (gro);
+  tree gro_bind_vars = gro;
+
+  /* We have to sequence the call to get_return_object before initial
+     suspend.  */
   r = build2_loc (fn_start, INIT_EXPR, TREE_TYPE (gro), gro, get_ro);
   r = coro_build_cvt_void_expr_stmt (r, fn_start);
   add_stmt (r);
@@ -3779,30 +3770,22 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
      logically doing things related to the end of the function.  */
 
   /* The ramp is done, we just need the return value.  */
-  if (!same_type_p (TREE_TYPE (gro), fn_return_type))
+  if (!same_type_p (TREE_TYPE (get_ro), fn_return_type))
     {
       /* construct the return value with a single GRO param.  */
       vec<tree, va_gc> *args = make_tree_vector_single (gro);
-      r = build_special_member_call (DECL_RESULT (orig),
+      r = build_special_member_call (NULL_TREE,
                                     complete_ctor_identifier, &args,
                                     fn_return_type, LOOKUP_NORMAL,
                                     tf_warning_or_error);
-      r = coro_build_cvt_void_expr_stmt (r, input_location);
-      add_stmt (r);
-      release_tree_vector (args);
+      r = build_cplus_new (fn_return_type, r, tf_warning_or_error);
     }
-  /* Else the GRO is the return and we already built it in place.  */
+  else
+    r = rvalue (gro); /* The GRO is the return value.  */
 
-  bool no_warning;
-  r = check_return_expr (DECL_RESULT (orig), &no_warning);
-  if (error_operand_p (r) && warn_return_type)
-    /* Suppress -Wreturn-type for the ramp.  */
-    TREE_NO_WARNING (orig) = true;
+  finish_return_stmt (r);
 
-  r = build_stmt (input_location, RETURN_EXPR, DECL_RESULT (orig));
-  TREE_NO_WARNING (r) |= no_warning;
-  r = maybe_cleanup_point_expr_void (r);
-  add_stmt (r);
+  /* 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);
   BIND_EXPR_BODY (ramp_bind) = pop_stmt_list (ramp_body);

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index e733cae0e918f0de94f504e91562b636e2bc7708..89178740200958d0647d9c44750da05de9752ebb 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2020-04-21 Iain Sandoe <iain@sandoe.co.uk>
+
+       PR c++/94661
+       * g++.dg/coroutines/ramp-return-a.C: New test.
+       * g++.dg/coroutines/ramp-return-b.C: New test.
+       * g++.dg/coroutines/ramp-return-c.C: New test.
+
 2020-04-17  Marek Polacek  <polacek@redhat.com>
 
        PR c++/94592

diff --git a/gcc/testsuite/g++.dg/coroutines/ramp-return-a.C b/gcc/testsuite/g++.dg/coroutines/ramp-return-a.C
new file mode 100644 (file)
index 0000000..c6e445e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/ramp-return-a.C
@@ -0,0 +1,24 @@
+//  { dg-additional-options "-std=c++14" }
+
+#include "ramp-return.h"
+
+task<int>
+foo ()
+{
+ std::coroutine_handle<promise<int>> _handle;
+ return task<int> (_handle);
+}
+
+// This ICEd for the PR.
+
+task<int>
+bar ()
+{
+  co_return 0;
+}
+
+task<std::vector<int>>
+baz ()
+{
+  co_return std::vector<int>();
+}

diff --git a/gcc/testsuite/g++.dg/coroutines/ramp-return-b.C b/gcc/testsuite/g++.dg/coroutines/ramp-return-b.C
new file mode 100644 (file)
index 0000000..d0e5d1f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/ramp-return-b.C
@@ -0,0 +1,22 @@
+//  { dg-options "-fcoroutines -std=c++14" }
+#define DELETE_COPY_CTOR 1
+#include "ramp-return.h"
+
+task<int>
+foo ()
+{
+ std::coroutine_handle<promise<int>> _handle;
+ return task<int> (_handle);  // { dg-error {use of deleted function 'task<T>::task\(const task<T>&\) \[with T = int\]'} }
+}
+
+task<int>
+bar ()
+{
+  co_return 0;
+} // { dg-error {use of deleted function 'task<T>::task\(const task<T>&\) \[with T = int\]'} }
+
+task<std::vector<int>>
+baz ()
+{
+  co_return std::vector<int>();
+} // { dg-error {use of deleted function 'task<T>::task\(const task<T>&\) \[with T = std::vector<int>\]'} }

diff --git a/gcc/testsuite/g++.dg/coroutines/ramp-return-c.C b/gcc/testsuite/g++.dg/coroutines/ramp-return-c.C
new file mode 100644 (file)
index 0000000..e030ca1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/ramp-return-c.C
@@ -0,0 +1,22 @@
+//  { dg-additional-options "-std=c++17" }
+#define DELETE_COPY_CTOR 1
+#include "ramp-return.h"
+
+task<int>
+foo ()
+{
+ std::coroutine_handle<promise<int>> _handle;
+ return task<int> (_handle); 
+}
+
+task<int>
+bar ()
+{
+  co_return 0;
+}
+
+task<std::vector<int>>
+baz ()
+{
+  co_return std::vector<int>();
+}

diff --git a/gcc/testsuite/g++.dg/coroutines/ramp-return.h b/gcc/testsuite/g++.dg/coroutines/ramp-return.h
new file mode 100644 (file)
index 0000000..f41a07d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/ramp-return.h
@@ -0,0 +1,64 @@
+#include "coro.h"
+
+#include <exception>
+#include <vector>
+
+template <typename T>
+struct promise {
+  T _value;
+  coro::coroutine_handle<> _continuation = nullptr;
+
+  struct final_awaitable {
+    bool _has_continuation;
+    final_awaitable(bool has_continuation)
+        : _has_continuation(has_continuation) {}
+
+    bool await_ready() const noexcept { return !_has_continuation; }
+
+    template <typename Promise>
+    coro::coroutine_handle<>
+    await_suspend(coro::coroutine_handle<Promise> coro) noexcept {
+      return coro.promise()._continuation;
+    }
+
+    void await_resume() noexcept {}
+  };
+
+  auto get_return_object() noexcept {
+    return coro::coroutine_handle<promise>::from_promise(*this);
+  }
+
+  auto initial_suspend() noexcept { return coro::suspend_always(); }
+
+  auto final_suspend() noexcept {
+    return final_awaitable(_continuation != nullptr);
+  }
+
+  void return_value(T value) { _value = value; }
+
+  void unhandled_exception() { std::terminate(); }
+};
+
+template <typename T> 
+struct task {
+  using promise_type = promise<T>;
+  std::coroutine_handle<promise<T>> _handle;
+
+  task (coro::coroutine_handle<promise<T>> handle) : _handle(handle) {}
+#if DELETE_COPY_CTOR
+ task (const task &) = delete; // no copying
+#endif
+#if DELETE_MOVE_CTOR
+ task(task&& t) noexcept
+    : _handle(t._handle) { t._handle = nullptr; }
+#endif
+  bool await_ready() noexcept { return _handle.done(); }
+
+  std::coroutine_handle<>
+  await_suspend(std::coroutine_handle<> handle) noexcept {
+    _handle.promise()._continuation = handle;
+    return _handle;
+  }
+
+  T await_resume() noexcept { return _handle.promise()._value; }
+};