From b9c91b7f3279e23aed965c05197acf3b6f439f8d Mon Sep 17 00:00:00 2001 From: Iain Sandoe Date: Tue, 28 Apr 2020 00:27:47 +0100 Subject: [PATCH] coroutines: Fix handling of non-class coroutine returns [PR94759] MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit From the standard: The header 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 has the following publicly accessible member: using promise_type = typename R::promise_type; this should not prevent more specialised cases and the following code should be accepted, but is currently rejected with: 'error: coroutine return type ‘void’ is not a class' This is because the check for non-class-ness of the return value was 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. The header is amended to implement the required functionality. gcc/cp/ChangeLog: 2020-04-28 Iain Sandoe 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-28 Iain Sandoe 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-28 Jonathan Wakely Iain Sandoe PR c++/94759 * include/std/coroutine: Implement handing for non- class coroutine return types. --- gcc/cp/ChangeLog | 9 ++ gcc/cp/coroutines.cc | 104 +++++++++--------- gcc/testsuite/ChangeLog | 13 ++- .../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/ChangeLog | 7 ++ libstdc++-v3/include/std/coroutine | 15 ++- 11 files changed, 151 insertions(+), 66 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index ae67a3e6c63..7a90c4c98f3 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,12 @@ +2020-04-28 Iain Sandoe + + 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. + 2020-04-27 Iain Sandoe PR c++/94701 diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 36229c77b4e..b4dc731ae86 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 */ - 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); @@ -3518,9 +3506,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)); @@ -3593,7 +3578,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); @@ -3778,7 +3762,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) tree ramp_bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL); add_stmt (ramp_bind); tree ramp_body = push_stmt_list (); - tree empty_list = build_empty_stmt (fn_start); tree coro_fp = build_lang_decl (VAR_DECL, get_identifier ("coro.frameptr"), coro_frame_ptr); @@ -3952,33 +3935,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. */ @@ -4176,17 +4153,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 @@ -4222,16 +4207,25 @@ 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 *args = make_tree_vector_single (gro); + /* construct the return value with a single GRO param, if it's not + void. */ + vec *args = NULL; + vec **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/ChangeLog b/gcc/testsuite/ChangeLog index b6e2f4f819f..fe3375a029c 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,4 +1,15 @@ -2020-04-21 Iain Sandoe +2020-04-28 Iain Sandoe + + 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. + +2020-04-27 Iain Sandoe PR c++/94701 * g++.dg/coroutines/torture/local-var-06-structured-binding.C: 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..4706deebf4e 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::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::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..252cb5e442c 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::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::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..89972b60945 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::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::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 +struct std::coroutine_traits { + struct promise_type { + promise_type (HandleRef h, T ...args) + { h = std::coroutine_handle::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/ChangeLog b/libstdc++-v3/ChangeLog index b3d001c02ab..f6de19da5de 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,10 @@ +2020-04-28 Jonathan Wakely + Iain Sandoe + + PR c++/94759 + * include/std/coroutine: Implement handing for non- + class coroutine return types. + 2020-04-24 Jonathan Wakely * include/experimental/executor (service_already_exists): Make default diff --git a/libstdc++-v3/include/std/coroutine b/libstdc++-v3/include/std/coroutine index 4fa1355c0ca..b40a3bcf9cc 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 - 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 + struct __coroutine_traits_impl {}; + + template + struct __coroutine_traits_impl<_Result, + __void_t> { - using promise_type = typename _Result::promise_type; + using promise_type = typename _Result::promise_type; }; + template + struct coroutine_traits : __coroutine_traits_impl<_Result> {}; + // 17.12.3 Class template coroutine_handle /// [coroutine.handle] template -- 2.30.2