| Message ID | AB5483F3-4539-445C-80CC-680DB4278F65@sandoe.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | coroutines: Fix handling of non-class coroutine returns [PR94759] | expand |
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 >
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
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>
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