From cd08718d57d1552fa2dbca96809e4915559685e7 Mon Sep 17 00:00:00 2001 From: Iain Sandoe Date: Tue, 21 Apr 2020 10:35:13 +0100 Subject: [PATCH] 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 PR c++/94661 * coroutines.cc (morph_fn_to_coro): Simplify return value computation. gcc/testsuite/ChangeLog: 2020-04-21 Iain Sandoe 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. --- gcc/cp/ChangeLog | 6 +++ gcc/cp/coroutines.cc | 47 ++++++------------ gcc/testsuite/ChangeLog | 7 +++ gcc/testsuite/g++.dg/coroutines/ramp-return-a.C | 24 ++++++++++ gcc/testsuite/g++.dg/coroutines/ramp-return-b.C | 22 +++++++++ gcc/testsuite/g++.dg/coroutines/ramp-return-c.C | 22 +++++++++ gcc/testsuite/g++.dg/coroutines/ramp-return.h | 64 +++++++++++++++++++++++++ 7 files changed, 160 insertions(+), 32 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/ramp-return-a.C create mode 100644 gcc/testsuite/g++.dg/coroutines/ramp-return-b.C create mode 100644 gcc/testsuite/g++.dg/coroutines/ramp-return-c.C create mode 100644 gcc/testsuite/g++.dg/coroutines/ramp-return.h diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 53daab1..5b2bff8 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,9 @@ +2020-04-21 Iain Sandoe + + PR c++/94661 + * coroutines.cc (morph_fn_to_coro): Simplify return + value computation. + 2020-04-17 Marek Polacek PR c++/94592 diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index ceb8daa..30676eb 100644 --- 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 *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 e733cae..8917874 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2020-04-21 Iain Sandoe + + 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 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 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 +foo () +{ + std::coroutine_handle> _handle; + return task (_handle); +} + +// This ICEd for the PR. + +task +bar () +{ + co_return 0; +} + +task> +baz () +{ + co_return std::vector(); +} 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 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 +foo () +{ + std::coroutine_handle> _handle; + return task (_handle); // { dg-error {use of deleted function 'task::task\(const task&\) \[with T = int\]'} } +} + +task +bar () +{ + co_return 0; +} // { dg-error {use of deleted function 'task::task\(const task&\) \[with T = int\]'} } + +task> +baz () +{ + co_return std::vector(); +} // { dg-error {use of deleted function 'task::task\(const task&\) \[with T = std::vector\]'} } 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 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 +foo () +{ + std::coroutine_handle> _handle; + return task (_handle); +} + +task +bar () +{ + co_return 0; +} + +task> +baz () +{ + co_return std::vector(); +} diff --git a/gcc/testsuite/g++.dg/coroutines/ramp-return.h b/gcc/testsuite/g++.dg/coroutines/ramp-return.h new file mode 100644 index 0000000..f41a07d --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/ramp-return.h @@ -0,0 +1,64 @@ +#include "coro.h" + +#include +#include + +template +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 + coro::coroutine_handle<> + await_suspend(coro::coroutine_handle coro) noexcept { + return coro.promise()._continuation; + } + + void await_resume() noexcept {} + }; + + auto get_return_object() noexcept { + return coro::coroutine_handle::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 +struct task { + using promise_type = promise; + std::coroutine_handle> _handle; + + task (coro::coroutine_handle> 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; } +}; -- 2.7.4