On 8/21/24 3:10 PM, Iain Sandoe wrote:
[dcl.fct.def.coroutine]/7 says:
The expression promise.get_return_object() is used to initialize the returned
reference or prvalue result object of a call to a coroutine. The call to
get_return_object is sequenced before the call to initial_suspend and is
invoked at most once.
The issue is about when any conversions are carried out if the type of
the g_r_o call is not the same as the ramp return. Currently, we have been
doing this by materialising the g_r_o return value and passing that to
finish_return_expr() which handles the necessary conversions and checks.
As the PR shows, this does not work as expected.
In the revised version we carry out the work of the conversions when
intialising the return slot (with the same facilities that are used by
finish_return_expr()). We do this before the call that initiates the
coroutine body, satisfying the requirements for one call before initial
suspend.
The return expression becomes a trivial 'return <retval>'.
This simplifes the ramp logic considerably, since we no longer need to
simplifies
keep track of the temporarily-materialised g_r_o value.
PR c++/115908
gcc/cp/ChangeLog:
* coroutines.cc
(cp_coroutine_transform::build_ramp_function): Rework the return
value initialisation to initialise the return slot always from
get_return_object, even if that implies carrying out conversions
to do so.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr115908.C: New test.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 153 +++++++--------------
gcc/testsuite/g++.dg/coroutines/pr115908.C | 75 ++++++++++
2 files changed, 121 insertions(+), 107 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr115908.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 64a08b49132..8738ce15533 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4641,6 +4641,8 @@ cp_coroutine_transform::build_ramp_function ()
tree promise_type = get_coroutine_promise_type (orig_fn_decl);
tree fn_return_type = TREE_TYPE (TREE_TYPE (orig_fn_decl));
bool void_ramp_p = VOID_TYPE_P (fn_return_type);
+ /* We know there was no return statement, that is intentional. */
+ suppress_warning (orig_fn_decl, OPT_Wreturn_type);
/* [dcl.fct.def.coroutine] / 10 (part1)
The unqualified-id get_return_object_on_allocation_failure is looked up
@@ -5009,162 +5011,99 @@ cp_coroutine_transform::build_ramp_function ()
promise_dtor = cxx_maybe_build_cleanup (p, tf_warning_or_error);
}
- /* Set up a new bind context for the GRO. */
- tree gro_context_bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
- /* Make and connect the scope blocks. */
- tree gro_block = make_node (BLOCK);
- BLOCK_SUPERCONTEXT (gro_block) = top_block;
- BLOCK_SUBBLOCKS (top_block) = gro_block;
- BIND_EXPR_BLOCK (gro_context_bind) = gro_block;
- add_stmt (gro_context_bind);
-
tree get_ro
= coro_build_promise_expression (orig_fn_decl, p,
coro_get_return_object_identifier,
fn_start, NULL, /*musthave=*/true);
- /* Without a return object we haven't got much clue what's going on. */
+ /* Without a return object we haven't got much clue what is going on. */
if (get_ro == error_mark_node)
{
- BIND_EXPR_BODY (ramp_bind) = pop_stmt_list (ramp_body);
/* Suppress warnings about the missing return value. */
- suppress_warning (orig_fn_decl, OPT_Wreturn_type);
valid_coroutine = false;
input_location = save_input_loc;
return;
}
- tree gro_context_body = push_stmt_list ();
- tree gro_type = TREE_TYPE (get_ro);
+ /* Initialize the resume_idx_var to 0, meaning "not started". */
+ tree resume_idx_m
+ = lookup_member (frame_type, coro_resume_index_id,
+ /*protect=*/1, /*want_type=*/0, tf_warning_or_error);
+ tree resume_idx
+ = build_class_member_access_expr (deref_fp, resume_idx_m, NULL_TREE, false,
+ tf_warning_or_error);
As in patch 4, I think the separate lookup_member is unnecessary.
OK with that tweak.
Jason
