coroutines: Fix handling of non-class coroutine returns [PR94759]

Message ID AB5483F3-4539-445C-80CC-680DB4278F65@sandoe.co.uk
State New
Headers show
Series
  • coroutines: Fix handling of non-class coroutine returns [PR94759]
Related show

Commit Message

Iain Sandoe April 25, 2020, 1:49 p.m.
Hi

From the standard:

The header <coroutine> defines the primary template coroutine_traits
such that if ArgTypes is a parameter pack of types and if the
qualified-id R::promise_type is valid and denotes a type, then
coroutine_traits<R,ArgTypes...> has the following publicly accessible
member:
     using promise_type = typename R::promise_type;

—

however, this should not prevent more specialised cases and  the code
in the PR (the testcase in the following patch) should be accepted, but is
currently rejected with:

'error: coroutine return type ‘void’ is not a class'

This is because I applied the check for non-class-ness of the return value
in the wrong place; it needs to be carried out in a SFINAE context.

The following patch removes the restriction in the traits template
instantiation and allows for the case that the ramp function could
return void.

Jonathan provided a draft of a change to the header to implement the
required functionality, which has been incorporated.

==

tested on x86_64-darwin,
OK for master if the testing passes regstrap on x86-64-linux?
thanks
Iain

gcc/cp/ChangeLog:

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

	PR c++/94759
	* coroutines.cc (coro_promise_type_found_p): Do not
	exclude non-classes here (this needs to be handled in the
	coroutine header).
	(morph_fn_to_coro):  Allow for the case where the coroutine
	returns void.

gcc/testsuite/ChangeLog:

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

	PR c++/94759
	* g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Adjust for
	updated error messages.
	* g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
	* g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
	* g++.dg/coroutines/coro-missing-promise.C: Likewise.
	* g++.dg/coroutines/pr93458-5-bad-coro-type.C: Liekwise.
	* g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C: New test.

libstdc++-v3/ChangeLog:

2020-04-25  Jonathan Wakely  <jwakely@redhat.com>
	    Iain Sandoe  <iain@sandoe.co.uk>

	PR c++/94759
	* include/std/coroutine: Implement handing for non-
	class coroutine return types.
---
 gcc/cp/coroutines.cc                          | 102 +++++++++---------
 .../coroutines/coro-bad-alloc-00-bad-op-new.C |   2 +-
 .../coroutines/coro-bad-alloc-01-bad-op-del.C |   2 +-
 .../coro-bad-alloc-02-no-op-new-nt.C          |   2 +-
 .../g++.dg/coroutines/coro-missing-promise.C  |   2 -
 .../coroutines/pr93458-5-bad-coro-type.C      |   4 +-
 .../torture/co-ret-17-void-ret-coro.C         |  57 ++++++++++
 libstdc++-v3/include/std/coroutine            |  15 ++-
 8 files changed, 122 insertions(+), 64 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C

-- 
2.24.1

Comments

Patrick Palka via Gcc-patches April 25, 2020, 3:13 p.m. | #1
On 25/04/20 14:49 +0100, Iain Sandoe wrote:
>Hi

>

>From the standard:

>

>The header <coroutine> defines the primary template coroutine_traits

>such that if ArgTypes is a parameter pack of types and if the

>qualified-id R::promise_type is valid and denotes a type, then

>coroutine_traits<R,ArgTypes...> has the following publicly accessible

>member:

>     using promise_type = typename R::promise_type;

>

>—

>

>however, this should not prevent more specialised cases and  the code

>in the PR (the testcase in the following patch) should be accepted, but is

>currently rejected with:

>

>'error: coroutine return type ‘void’ is not a class'

>

>This is because I applied the check for non-class-ness of the return value

>in the wrong place; it needs to be carried out in a SFINAE context.

>

>The following patch removes the restriction in the traits template

>instantiation and allows for the case that the ramp function could

>return void.

>

>Jonathan provided a draft of a change to the header to implement the

>required functionality, which has been incorporated.

>

>==

>

>tested on x86_64-darwin,

>OK for master if the testing passes regstrap on x86-64-linux?

>thanks

>Iain

>

>gcc/cp/ChangeLog:

>

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

>

>	PR c++/94759

>	* coroutines.cc (coro_promise_type_found_p): Do not

>	exclude non-classes here (this needs to be handled in the

>	coroutine header).

>	(morph_fn_to_coro):  Allow for the case where the coroutine

>	returns void.

>

>gcc/testsuite/ChangeLog:

>

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

>

>	PR c++/94759

>	* g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Adjust for

>	updated error messages.

>	* g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.

>	* g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.

>	* g++.dg/coroutines/coro-missing-promise.C: Likewise.

>	* g++.dg/coroutines/pr93458-5-bad-coro-type.C: Liekwise.

>	* g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C: New test.

>

>libstdc++-v3/ChangeLog:

>

>2020-04-25  Jonathan Wakely  <jwakely@redhat.com>

>	    Iain Sandoe  <iain@sandoe.co.uk>

>

>	PR c++/94759

>	* include/std/coroutine: Implement handing for non-

>	class coroutine return types.

>---

> gcc/cp/coroutines.cc                          | 102 +++++++++---------

> .../coroutines/coro-bad-alloc-00-bad-op-new.C |   2 +-

> .../coroutines/coro-bad-alloc-01-bad-op-del.C |   2 +-

> .../coro-bad-alloc-02-no-op-new-nt.C          |   2 +-

> .../g++.dg/coroutines/coro-missing-promise.C  |   2 -

> .../coroutines/pr93458-5-bad-coro-type.C      |   4 +-

> .../torture/co-ret-17-void-ret-coro.C         |  57 ++++++++++

> libstdc++-v3/include/std/coroutine            |  15 ++-

> 8 files changed, 122 insertions(+), 64 deletions(-)

> create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C

>

>diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

>index 4f254b7fd10..db6f16b3e21 100644

>--- a/gcc/cp/coroutines.cc

>+++ b/gcc/cp/coroutines.cc

>@@ -441,18 +441,6 @@ coro_promise_type_found_p (tree fndecl, location_t loc)

>     {

>       /* Get the coroutine traits template class instance for the function

> 	 signature we have - coroutine_traits <R, ...>  */

>-      tree return_type = TREE_TYPE (TREE_TYPE (fndecl));

>-      if (!CLASS_TYPE_P (return_type))

>-	{

>-	  /* It makes more sense to show the function header for this, even

>-	     though we will have encountered it when processing a keyword.

>-	     Only emit the error once, not for every keyword we encounter.  */

>-	  if (!coro_info->coro_ret_type_error_emitted)

>-	    error_at (DECL_SOURCE_LOCATION (fndecl), "coroutine return type"

>-		      " %qT is not a class", return_type);

>-	  coro_info->coro_ret_type_error_emitted = true;

>-	  return false;

>-	}

>

>       tree templ_class = instantiate_coro_traits (fndecl, loc);

>

>@@ -3500,9 +3488,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>   if (!coro_function_valid_p (orig))

>     return false;

>

>-  /* The ramp function does return a value.  */

>-  current_function_returns_value = 1;

>-

>   /* We can't validly get here with an empty statement list, since there's no

>      way for the FE to decide it's a coroutine in the absence of any code.  */

>   tree fnbody = pop_stmt_list (DECL_SAVED_TREE (orig));

>@@ -3575,7 +3560,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>      1. Types we already know.  */

>

>   tree fn_return_type = TREE_TYPE (TREE_TYPE (orig));

>-  gcc_assert (!VOID_TYPE_P (fn_return_type));

>   tree handle_type = get_coroutine_handle_type (orig);

>   tree promise_type = get_coroutine_promise_type (orig);

>

>@@ -3927,33 +3911,27 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

> 	 control to the caller of the coroutine and the return value is

> 	 obtained by a call to T::get_return_object_on_allocation_failure(),

> 	 where T is the promise type.  */

>-       tree cfra_label

>-	= create_named_label_with_ctx (fn_start, "coro.frame.active",

>-				       current_scope ());

>-      tree early_ret_list = NULL;

>-      /* init the retval using the user's func.  */

>-      r = build2 (INIT_EXPR, TREE_TYPE (DECL_RESULT (orig)), DECL_RESULT (orig),

>-		  grooaf);

>-      r = coro_build_cvt_void_expr_stmt (r, fn_start);

>-      append_to_statement_list (r, &early_ret_list);

>-      /* We know it's the correct type.  */

>-      r = DECL_RESULT (orig);

>-      r = build_stmt (fn_start, RETURN_EXPR, r);

>-      TREE_NO_WARNING (r) |= 1;

>-      r = maybe_cleanup_point_expr_void (r);

>-      append_to_statement_list (r, &early_ret_list);

>-

>-      tree goto_st = NULL;

>-      r = build1 (GOTO_EXPR, void_type_node, cfra_label);

>-      append_to_statement_list (r, &goto_st);

>-

>-      tree ckk = build1 (CONVERT_EXPR, coro_frame_ptr, integer_zero_node);

>-      tree ckz = build2 (EQ_EXPR, boolean_type_node, coro_fp, ckk);

>-      r = build3 (COND_EXPR, void_type_node, ckz, early_ret_list, empty_list);

>-      add_stmt (r);

>

>-      cfra_label = build_stmt (fn_start, LABEL_EXPR, cfra_label);

>-      add_stmt (cfra_label);

>+      gcc_checking_assert (same_type_p (fn_return_type, TREE_TYPE (grooaf)));

>+      tree if_stmt = begin_if_stmt ();

>+      tree cond = build1 (CONVERT_EXPR, coro_frame_ptr, integer_zero_node);

>+      cond = build2 (EQ_EXPR, boolean_type_node, coro_fp, cond);

>+      finish_if_stmt_cond (cond, if_stmt);

>+      if (VOID_TYPE_P (fn_return_type))

>+	{

>+	  /* Execute the get-return-object-on-alloc-fail call...  */

>+	  finish_expr_stmt (grooaf);

>+	  /* ... but discard the result, since we return void.  */

>+	  finish_return_stmt (NULL_TREE);

>+	}

>+      else

>+	{

>+	  /* Get the fallback return object.  */

>+	  r = build_cplus_new (fn_return_type, grooaf, tf_warning_or_error);

>+	  finish_return_stmt (r);

>+	}

>+      finish_then_clause (if_stmt);

>+      finish_if_stmt (if_stmt);

>     }

>

>   /* deref the frame pointer, to use in member access code.  */

>@@ -4151,17 +4129,25 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>     }

>

>   tree gro_context_body = push_stmt_list ();

>-  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;

>+  bool gro_is_void_p = VOID_TYPE_P (TREE_TYPE (get_ro));

>

>+  tree gro, gro_bind_vars = NULL_TREE;

>   /* 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);

>+  if (gro_is_void_p)

>+    finish_expr_stmt (get_ro);

>+  else

>+    {

>+      gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"),

>+			      TREE_TYPE (get_ro));

>+      DECL_CONTEXT (gro) = current_scope ();

>+      add_decl_expr (gro);

>+      gro_bind_vars = gro;

>+

>+      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);

>+    }

>

>   /* Initialize the resume_idx_name to 0, meaning "not started".  */

>   tree resume_idx_m

>@@ -4197,16 +4183,24 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>   /* The ramp is done, we just need the return value.  */

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

>+      /* construct the return value with a single GRO param, if it's not

>+	 void.  */

>+      vec<tree, va_gc> *args = NULL;

>+      vec<tree, va_gc> **arglist = NULL;

>+      if (!gro_is_void_p)

>+	{

>+	  args = make_tree_vector_single (gro);

>+	  arglist = &args;

>+	}

>       r = build_special_member_call (NULL_TREE,

>-				     complete_ctor_identifier, &args,

>+				     complete_ctor_identifier, arglist,

> 				     fn_return_type, LOOKUP_NORMAL,

> 				     tf_warning_or_error);

>       r = build_cplus_new (fn_return_type, r, tf_warning_or_error);

>     }

>-  else

>+  else if (!gro_is_void_p)

>     r = rvalue (gro); /* The GRO is the return value.  */

>+  else r = NULL_TREE;

>

>   finish_return_stmt (r);

>

>diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C

>index 06543a98824..0bcaff6d84a 100644

>--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C

>+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C

>@@ -6,7 +6,7 @@

> #include "coro1-allocators.h"

>

> struct coro1

>-f ()  /* { dg-error {'operator new' is provided by 'std::__n4861::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */

>+f ()  /* { dg-error {'operator new' is provided by 'std::__n4861::__coroutine_traits__impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */

> {

>   co_return;

> }

>diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C

>index 9a4ec34cdf3..385f99c3dfd 100644

>--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C

>+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C

>@@ -6,7 +6,7 @@

> #include "coro1-allocators.h"

>

> struct coro1

>-f ()  /* { dg-error {'operator delete' is provided by 'std::__n4861::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */

>+f ()  /* { dg-error {'operator delete' is provided by 'std::__n4861::__coroutine_traits__impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */

> {

>   co_return;

> }

>diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C

>index c0c0792d31d..faf2c55825e 100644

>--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C

>+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C

>@@ -9,7 +9,7 @@

> #include "coro1-allocators.h"

>

> struct coro1

>-f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::__n4861::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */

>+f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::__n4861::__coroutine_traits__impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */

> {

>   co_return;

> }

>diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C

>index 3fc21a4e2ca..e75f2002db0 100644

>--- a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C

>+++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C

>@@ -4,8 +4,6 @@

>

> // Diagnose completely missing promise.

>

>-// { dg-error {no type named 'promise_type' in 'struct NoPromiseHere'} "" { target *-*-* } 0 }

>-

> struct NoPromiseHere {

>   coro::coroutine_handle<> handle;

>   NoPromiseHere () : handle (nullptr) {}

>diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C b/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C

>index 8bb58cc0a78..765c04892ef 100644

>--- a/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C

>+++ b/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C

>@@ -5,8 +5,8 @@

> #include "coro.h"

>

> int

>-bad_coroutine (void) // { dg-error {coroutine return type 'int' is not a class} }

>+bad_coroutine (void)

> {

>-  co_yield 5;

>+  co_yield 5; // { dg-error {unable to find the promise type for this coroutine} }

>   co_return;

> }

>diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C

>new file mode 100644

>index 00000000000..3168ea272c1

>--- /dev/null

>+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C

>@@ -0,0 +1,57 @@

>+//  { dg-do run }

>+

>+// Test the ability to specialize the coroutine traits to include

>+// non-class type coroutine ramp return values.

>+

>+#include "../coro.h"

>+

>+template<typename R, typename HandleRef, typename ...T>

>+struct std::coroutine_traits<R, HandleRef, T...> {

>+    struct promise_type {

>+        promise_type (HandleRef h, T ...args)

>+        { h = std::coroutine_handle<promise_type>::from_promise (*this);

>+          PRINT ("Created Promise");

>+        }

>+

>+        void get_return_object() {}

>+

>+        auto initial_suspend() {

>+          return std::suspend_always{};

>+         }

>+        auto final_suspend() { return std::suspend_never{}; }

>+

>+        void return_void() {}

>+        void unhandled_exception() {}

>+    };

>+};

>+

>+void

>+my_coro (std::coroutine_handle<>& h)

>+{

>+  PRINT ("coro1: about to return");

>+  co_return;

>+}

>+

>+int main ()

>+{

>+  std::coroutine_handle<> h;

>+  my_coro (h);

>+

>+  if (h.done())

>+    {

>+      PRINT ("main: apparently was already done...");

>+      abort ();

>+    }

>+

>+  // initial suspend.

>+  h.resume ();

>+

>+  if (!h.done())

>+    {

>+      PRINT ("main: apparently wasn't done...");

>+      abort ();

>+    }

>+

>+  PRINT ("main: returning");

>+  return 0;

>+}

>diff --git a/libstdc++-v3/include/std/coroutine b/libstdc++-v3/include/std/coroutine

>index 4fa1355c0ca..89bcae8b09a 100644

>--- a/libstdc++-v3/include/std/coroutine

>+++ b/libstdc++-v3/include/std/coroutine

>@@ -63,12 +63,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

>   // 17.12.2 coroutine traits

>   /// [coroutine.traits]

>   /// [coroutine.traits.primary]

>-  template <typename _Result, typename...>

>-    struct coroutine_traits

>+  /// If _Result::promise_type is valid and denotes a type then the traits

>+  /// have a single publicly accessible member, otherwise they are empty.

>+  template <typename _Result, typename = void>

>+   struct __coroutine_traits__impl {};


Just one underscore before "impl" please. The libstdc++ part is fine
with that change.

>+

>+  template <typename _Result>

>+    struct __coroutine_traits__impl<_Result,

>+				    __void_t<typename _Result::promise_type>>

>     {

>-       using promise_type = typename _Result::promise_type;

>+      using promise_type = typename _Result::promise_type;

>     };

>

>+  template <typename _Result, typename...>

>+    struct coroutine_traits : __coroutine_traits__impl<_Result> {};

>+

>   // 17.12.3 Class template coroutine_handle

>   /// [coroutine.handle]

>   template <typename _Promise = void>

>-- 

>2.24.1

>
Nathan Sidwell April 27, 2020, 1:49 p.m. | #2
On 4/25/20 9:49 AM, Iain Sandoe wrote:
> Hi


> 

> tested on x86_64-darwin,

> OK for master if the testing passes regstrap on x86-64-linux?

> thanks

> Iain

> 

> gcc/cp/ChangeLog:

> 

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

> 

> 	PR c++/94759

> 	* coroutines.cc (coro_promise_type_found_p): Do not

> 	exclude non-classes here (this needs to be handled in the

> 	coroutine header).

> 	(morph_fn_to_coro):  Allow for the case where the coroutine

> 	returns void.


Ok, A nit ..

> @@ -4197,16 +4183,24 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>     /* The ramp is done, we just need the return value.  */

>     if (!same_type_p (TREE_TYPE (get_ro), fn_return_type))

>       {

> -      /* construct the return value with a single GRO param.  */


>         r = build_cplus_new (fn_return_type, r, tf_warning_or_error);

>       }

> -  else

> +  else if (!gro_is_void_p)

>       r = rvalue (gro); /* The GRO is the return value.  */

> +  else r = NULL_TREE;


^^ missing line break

I see Jonathan approved the library bit, with a nit too.

nathan

-- 
Nathan Sidwell

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 4f254b7fd10..db6f16b3e21 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -441,18 +441,6 @@  coro_promise_type_found_p (tree fndecl, location_t loc)
     {
       /* Get the coroutine traits template class instance for the function
 	 signature we have - coroutine_traits <R, ...>  */
-      tree return_type = TREE_TYPE (TREE_TYPE (fndecl));
-      if (!CLASS_TYPE_P (return_type))
-	{
-	  /* It makes more sense to show the function header for this, even
-	     though we will have encountered it when processing a keyword.
-	     Only emit the error once, not for every keyword we encounter.  */
-	  if (!coro_info->coro_ret_type_error_emitted)
-	    error_at (DECL_SOURCE_LOCATION (fndecl), "coroutine return type"
-		      " %qT is not a class", return_type);
-	  coro_info->coro_ret_type_error_emitted = true;
-	  return false;
-	}
 
       tree templ_class = instantiate_coro_traits (fndecl, loc);
 
@@ -3500,9 +3488,6 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   if (!coro_function_valid_p (orig))
     return false;
 
-  /* The ramp function does return a value.  */
-  current_function_returns_value = 1;
-
   /* We can't validly get here with an empty statement list, since there's no
      way for the FE to decide it's a coroutine in the absence of any code.  */
   tree fnbody = pop_stmt_list (DECL_SAVED_TREE (orig));
@@ -3575,7 +3560,6 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
      1. Types we already know.  */
 
   tree fn_return_type = TREE_TYPE (TREE_TYPE (orig));
-  gcc_assert (!VOID_TYPE_P (fn_return_type));
   tree handle_type = get_coroutine_handle_type (orig);
   tree promise_type = get_coroutine_promise_type (orig);
 
@@ -3927,33 +3911,27 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	 control to the caller of the coroutine and the return value is
 	 obtained by a call to T::get_return_object_on_allocation_failure(),
 	 where T is the promise type.  */
-       tree cfra_label
-	= create_named_label_with_ctx (fn_start, "coro.frame.active",
-				       current_scope ());
-      tree early_ret_list = NULL;
-      /* init the retval using the user's func.  */
-      r = build2 (INIT_EXPR, TREE_TYPE (DECL_RESULT (orig)), DECL_RESULT (orig),
-		  grooaf);
-      r = coro_build_cvt_void_expr_stmt (r, fn_start);
-      append_to_statement_list (r, &early_ret_list);
-      /* We know it's the correct type.  */
-      r = DECL_RESULT (orig);
-      r = build_stmt (fn_start, RETURN_EXPR, r);
-      TREE_NO_WARNING (r) |= 1;
-      r = maybe_cleanup_point_expr_void (r);
-      append_to_statement_list (r, &early_ret_list);
-
-      tree goto_st = NULL;
-      r = build1 (GOTO_EXPR, void_type_node, cfra_label);
-      append_to_statement_list (r, &goto_st);
-
-      tree ckk = build1 (CONVERT_EXPR, coro_frame_ptr, integer_zero_node);
-      tree ckz = build2 (EQ_EXPR, boolean_type_node, coro_fp, ckk);
-      r = build3 (COND_EXPR, void_type_node, ckz, early_ret_list, empty_list);
-      add_stmt (r);
 
-      cfra_label = build_stmt (fn_start, LABEL_EXPR, cfra_label);
-      add_stmt (cfra_label);
+      gcc_checking_assert (same_type_p (fn_return_type, TREE_TYPE (grooaf)));
+      tree if_stmt = begin_if_stmt ();
+      tree cond = build1 (CONVERT_EXPR, coro_frame_ptr, integer_zero_node);
+      cond = build2 (EQ_EXPR, boolean_type_node, coro_fp, cond);
+      finish_if_stmt_cond (cond, if_stmt);
+      if (VOID_TYPE_P (fn_return_type))
+	{
+	  /* Execute the get-return-object-on-alloc-fail call...  */
+	  finish_expr_stmt (grooaf);
+	  /* ... but discard the result, since we return void.  */
+	  finish_return_stmt (NULL_TREE);
+	}
+      else
+	{
+	  /* Get the fallback return object.  */
+	  r = build_cplus_new (fn_return_type, grooaf, tf_warning_or_error);
+	  finish_return_stmt (r);
+	}
+      finish_then_clause (if_stmt);
+      finish_if_stmt (if_stmt);
     }
 
   /* deref the frame pointer, to use in member access code.  */
@@ -4151,17 +4129,25 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
     }
 
   tree gro_context_body = push_stmt_list ();
-  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;
+  bool gro_is_void_p = VOID_TYPE_P (TREE_TYPE (get_ro));
 
+  tree gro, gro_bind_vars = NULL_TREE;
   /* 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);
+  if (gro_is_void_p)
+    finish_expr_stmt (get_ro);
+  else
+    {
+      gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"),
+			      TREE_TYPE (get_ro));
+      DECL_CONTEXT (gro) = current_scope ();
+      add_decl_expr (gro);
+      gro_bind_vars = gro;
+
+      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);
+    }
 
   /* Initialize the resume_idx_name to 0, meaning "not started".  */
   tree resume_idx_m
@@ -4197,16 +4183,24 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   /* The ramp is done, we just need the return value.  */
   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);
+      /* construct the return value with a single GRO param, if it's not
+	 void.  */
+      vec<tree, va_gc> *args = NULL;
+      vec<tree, va_gc> **arglist = NULL;
+      if (!gro_is_void_p)
+	{
+	  args = make_tree_vector_single (gro);
+	  arglist = &args;
+	}
       r = build_special_member_call (NULL_TREE,
-				     complete_ctor_identifier, &args,
+				     complete_ctor_identifier, arglist,
 				     fn_return_type, LOOKUP_NORMAL,
 				     tf_warning_or_error);
       r = build_cplus_new (fn_return_type, r, tf_warning_or_error);
     }
-  else
+  else if (!gro_is_void_p)
     r = rvalue (gro); /* The GRO is the return value.  */
+  else r = NULL_TREE;
 
   finish_return_stmt (r);
 
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
index 06543a98824..0bcaff6d84a 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
@@ -6,7 +6,7 @@ 
 #include "coro1-allocators.h"
 
 struct coro1
-f ()  /* { dg-error {'operator new' is provided by 'std::__n4861::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
+f ()  /* { dg-error {'operator new' is provided by 'std::__n4861::__coroutine_traits__impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
 {
   co_return;
 }
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
index 9a4ec34cdf3..385f99c3dfd 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
@@ -6,7 +6,7 @@ 
 #include "coro1-allocators.h"
 
 struct coro1
-f ()  /* { dg-error {'operator delete' is provided by 'std::__n4861::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
+f ()  /* { dg-error {'operator delete' is provided by 'std::__n4861::__coroutine_traits__impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
 {
   co_return;
 }
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
index c0c0792d31d..faf2c55825e 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
@@ -9,7 +9,7 @@ 
 #include "coro1-allocators.h"
 
 struct coro1
-f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::__n4861::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */
+f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::__n4861::__coroutine_traits__impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */
 {
   co_return;
 }
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C
index 3fc21a4e2ca..e75f2002db0 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C
@@ -4,8 +4,6 @@ 
 
 // Diagnose completely missing promise.
 
-// { dg-error {no type named 'promise_type' in 'struct NoPromiseHere'} "" { target *-*-* } 0 }
-
 struct NoPromiseHere {
   coro::coroutine_handle<> handle;
   NoPromiseHere () : handle (nullptr) {}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C b/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C
index 8bb58cc0a78..765c04892ef 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C
@@ -5,8 +5,8 @@ 
 #include "coro.h"
 
 int
-bad_coroutine (void) // { dg-error {coroutine return type 'int' is not a class} }
+bad_coroutine (void) 
 {
-  co_yield 5;
+  co_yield 5; // { dg-error {unable to find the promise type for this coroutine} }
   co_return;
 }
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C
new file mode 100644
index 00000000000..3168ea272c1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C
@@ -0,0 +1,57 @@ 
+//  { dg-do run }
+
+// Test the ability to specialize the coroutine traits to include
+// non-class type coroutine ramp return values.
+
+#include "../coro.h"
+
+template<typename R, typename HandleRef, typename ...T>
+struct std::coroutine_traits<R, HandleRef, T...> {
+    struct promise_type {
+        promise_type (HandleRef h, T ...args)
+        { h = std::coroutine_handle<promise_type>::from_promise (*this);
+          PRINT ("Created Promise");
+        }
+
+        void get_return_object() {}
+
+        auto initial_suspend() {
+          return std::suspend_always{};
+         }
+        auto final_suspend() { return std::suspend_never{}; }
+
+        void return_void() {}
+        void unhandled_exception() {}
+    };
+};
+
+void
+my_coro (std::coroutine_handle<>& h)
+{
+  PRINT ("coro1: about to return");
+  co_return;
+}
+
+int main ()
+{
+  std::coroutine_handle<> h;
+  my_coro (h);
+
+  if (h.done())
+    {
+      PRINT ("main: apparently was already done...");
+      abort ();
+    }
+
+  // initial suspend.
+  h.resume ();
+  
+  if (!h.done())
+    {
+      PRINT ("main: apparently wasn't done...");
+      abort ();
+    }
+
+  PRINT ("main: returning");
+  return 0;
+}
diff --git a/libstdc++-v3/include/std/coroutine b/libstdc++-v3/include/std/coroutine
index 4fa1355c0ca..89bcae8b09a 100644
--- a/libstdc++-v3/include/std/coroutine
+++ b/libstdc++-v3/include/std/coroutine
@@ -63,12 +63,21 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // 17.12.2 coroutine traits
   /// [coroutine.traits]
   /// [coroutine.traits.primary]
-  template <typename _Result, typename...>
-    struct coroutine_traits
+  /// If _Result::promise_type is valid and denotes a type then the traits
+  /// have a single publicly accessible member, otherwise they are empty.
+  template <typename _Result, typename = void>
+   struct __coroutine_traits__impl {};
+
+  template <typename _Result>
+    struct __coroutine_traits__impl<_Result,
+				    __void_t<typename _Result::promise_type>>
     {
-       using promise_type = typename _Result::promise_type;
+      using promise_type = typename _Result::promise_type;
     };
 
+  template <typename _Result, typename...>
+    struct coroutine_traits : __coroutine_traits__impl<_Result> {};
+
   // 17.12.3 Class template coroutine_handle
   /// [coroutine.handle]
   template <typename _Promise = void>