>>+ /* We must manage the cleanups ourselves, because the responsibility for
>>+ them changes after the initial suspend. However, any use of
>>+ cxx_maybe_build_cleanup () can set the throwing_cleanup flag. */
>>+ cp_function_chain->throwing_cleanup = false;
>Hmm...what if the gro cleanup throws after initializing the (different type)
>return value? That seems like a case that we need throwing_cleanup set for.
So I moved this to the position before the g_r_o is initialized
(since we only manage cleanups of the entities that come before that, although
that's a bit hard to see from the patch).
>>@@ -5245,8 +5195,11 @@ cp_coroutine_transform::build_ramp_function ()
>> tree not_iarc
>> = build1_loc (loc, TRUTH_NOT_EXPR, boolean_type_node, iarc_x);
>>+ tree do_cleanup = build2_loc (loc, TRUTH_AND_EXPR, boolean_type_node,
>>+ not_iarc, coro_before_return);
>As with the 14 patch, this should be reversed.
Yes, the same goof was C&P to several places.
OK for trunk now?
thanks
Iain
--- 8< ---
This addresses the clarification that, when the get_return_object is of a
different type from the ramp return, any necessary conversions should be
performed on the return expression (so that they typically occur after the
function body has started execution).
PR c++/119916
gcc/cp/ChangeLog:
* coroutines.cc
(cp_coroutine_transform::wrap_original_function_body): Do not
initialise initial_await_resume_called here...
(cp_coroutine_transform::build_ramp_function): ... but here.
When the coroutine is not void, initialize a GRO object from
promise.get_return_object(). Use this as the argument to the
return expression. Use a regular cleanup for the GRO, since
it is ramp-local.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/torture/special-termination-00-sync-completion.C:
Amend for CWG2563 expected behaviour.
* g++.dg/coroutines/torture/special-termination-01-self-destruct.C:
Likewise.
* g++.dg/coroutines/torture/pr119916.C: New test.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 125 ++++++------------
.../g++.dg/coroutines/torture/pr119916.C | 66 +++++++++
.../special-termination-00-sync-completion.C | 2 +-
.../special-termination-01-self-destruct.C | 2 +-
4 files changed, 108 insertions(+), 87 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/pr119916.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 743da068e35..6169a81cea5 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4451,7 +4451,7 @@ cp_coroutine_transform::wrap_original_function_body ()
tree i_a_r_c
= coro_build_artificial_var (loc, coro_frame_i_a_r_c_id,
boolean_type_node, orig_fn_decl,
- boolean_false_node);
+ NULL_TREE);
DECL_CHAIN (i_a_r_c) = var_list;
var_list = i_a_r_c;
add_decl_expr (i_a_r_c);
@@ -4867,7 +4867,6 @@ cp_coroutine_transform::build_ramp_function ()
add_decl_expr (coro_fp);
tree coro_promise_live = NULL_TREE;
- tree coro_gro_live = NULL_TREE;
if (flag_exceptions)
{
/* Signal that we need to clean up the promise object on exception. */
@@ -4876,13 +4875,6 @@ cp_coroutine_transform::build_ramp_function ()
boolean_type_node, orig_fn_decl,
boolean_false_node);
- /* When the get-return-object is in the RETURN slot, we need to arrange
- for cleanup on exception. */
- coro_gro_live
- = coro_build_and_push_artificial_var (loc, "_Coro_gro_live",
- boolean_type_node, orig_fn_decl,
- boolean_false_node);
-
/* To signal that we need to cleanup copied function args. */
if (DECL_ARGUMENTS (orig_fn_decl))
for (tree arg = DECL_ARGUMENTS (orig_fn_decl); arg != NULL;
@@ -4970,13 +4962,19 @@ cp_coroutine_transform::build_ramp_function ()
tree ramp_try_block = NULL_TREE;
tree ramp_try_stmts = NULL_TREE;
tree iarc_x = NULL_TREE;
+ tree coro_before_return = NULL_TREE;
if (flag_exceptions)
{
+ coro_before_return
+ = coro_build_and_push_artificial_var (loc, "_Coro_before_return",
+ boolean_type_node, orig_fn_decl,
+ boolean_true_node);
iarc_x
= coro_build_and_push_artificial_var_with_dve (loc,
coro_frame_i_a_r_c_id,
boolean_type_node,
- orig_fn_decl, NULL_TREE,
+ orig_fn_decl,
+ boolean_false_node,
deref_fp);
ramp_try_block = begin_try_block ();
ramp_try_stmts = begin_compound_stmt (BCS_TRY_BLOCK);
@@ -5136,90 +5134,53 @@ cp_coroutine_transform::build_ramp_function ()
(loc, coro_resume_index_id, short_unsigned_type_node, orig_fn_decl,
build_zero_cst (short_unsigned_type_node), deref_fp);
- if (flag_exceptions && iarc_x)
- {
- r = cp_build_init_expr (iarc_x, boolean_false_node);
- finish_expr_stmt (r);
- }
-
- /* Used for return objects in the RESULT slot. */
- tree ret_val_dtor = NULL_TREE;
- tree retval = NULL_TREE;
+ /* We must manage the cleanups ourselves, with the exception of the g_r_o,
+ because the responsibility for them changes after the initial suspend.
+ However, any use of cxx_maybe_build_cleanup () in preceding code can
+ set the throwing_cleanup flag. */
+ cp_function_chain->throwing_cleanup = false;
/* [dcl.fct.def.coroutine] / 7
The expression promise.get_return_object() is used to initialize the
glvalue result or prvalue result object of a call to a coroutine. */
- /* We must manage the cleanups ourselves, because the responsibility for
- them changes after the initial suspend. However, any use of
- cxx_maybe_build_cleanup () can set the throwing_cleanup flag. */
- cp_function_chain->throwing_cleanup = false;
+ tree coro_gro = NULL_TREE;
if (void_ramp_p)
/* We still want to call the method, even if the result is unused. */
- r = get_ro;
+ finish_expr_stmt (get_ro);
else
{
- /* The initial section of finish_return_expr (). */
- bool no_warning;
- bool dangling;
- /* Without a relevant location, bad conversions in check_return_expr
- result in unusable diagnostics, since there is not even a mention
- of the relevant function. Here we carry out the first part of
- finish_return_expr(). */
- input_location = fn_start;
- r = check_return_expr (get_ro, &no_warning, &dangling);
- input_location = UNKNOWN_LOCATION;
- gcc_checking_assert (!dangling);
- /* Check for bad things. */
- if (!r || r == error_mark_node)
- return false;
- if (!aggregate_value_p (fn_return_type, orig_fn_decl)
- && TREE_CODE (r) == INIT_EXPR)
- {
- /* If fn_return_type doesn't need to be returned in memory, normally
- gimplify_return_expr redirects the INIT_EXPR to a temporary. But
- r isn't wrapped in the RETURN_EXPR, so we need to do the
- redirection here as well. See PR118874. */
- tree temp = create_temporary_var (fn_return_type);
- add_decl_expr (temp);
- retval = copy_node (r);
- TREE_OPERAND (r, 0) = temp;
- TREE_OPERAND (retval, 1) = temp;
- }
- else
- retval = DECL_RESULT (orig_fn_decl);
- }
+ /* Per CWG2563, we keep the result of promise.get_return_object () in
+ a temp which is then used to intialize the return object, including
+ NVRO. */
- finish_expr_stmt (r);
-
- if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (fn_return_type))
- /* If some part of the initalization code (prior to the await_resume
- of the initial suspend expression), then we need to clean up the
- return value. */
- ret_val_dtor = cxx_maybe_build_cleanup (DECL_RESULT (orig_fn_decl),
- tf_warning_or_error);
+ /* Temporary var to hold the g_r_o across the function body. */
+ coro_gro
+ = coro_build_and_push_artificial_var (loc, "_Coro_gro", gro_type,
+ orig_fn_decl, NULL_TREE);
- /* If we have a live g.r.o in the return slot, then signal this for exception
- cleanup. */
- if (flag_exceptions && ret_val_dtor)
- {
- r = cp_build_init_expr (coro_gro_live, boolean_true_node);
+ r = cp_build_init_expr (coro_gro, get_ro);
finish_expr_stmt (r);
+ tree coro_gro_cleanup
+ = cxx_maybe_build_cleanup (coro_gro, tf_warning_or_error);
+ if (coro_gro_cleanup)
+ push_cleanup (coro_gro, coro_gro_cleanup, /*eh_only*/false);
}
/* Start the coroutine body. */
r = build_call_expr_loc (fn_start, resumer, 1, coro_fp);
finish_expr_stmt (r);
+ if (flag_exceptions)
+ {
+ r = cp_build_init_expr (coro_before_return, boolean_false_node);
+ finish_expr_stmt (r);
+ }
+
/* The ramp is done, we just need the return statement, which we build from
- the return object we constructed before we called the actor. */
+ the return object we constructed before we called the function body. */
- r = retval;
-
- /* The reminder of finish_return_expr (). */
- r = build_stmt (loc, RETURN_EXPR, r);
- r = maybe_cleanup_point_expr_void (r);
- r = add_stmt (r);
+ finish_return_stmt (void_ramp_p ? NULL_TREE : coro_gro);
if (flag_exceptions)
{
@@ -5228,16 +5189,6 @@ cp_coroutine_transform::build_ramp_function ()
tree handler = begin_handler ();
finish_handler_parms (NULL_TREE, handler); /* catch (...) */
- /* If we have a live G.R.O in the return slot, then run its DTOR. */
- if (ret_val_dtor && ret_val_dtor != error_mark_node)
- {
- tree gro_d_if = begin_if_stmt ();
- finish_if_stmt_cond (coro_gro_live, gro_d_if);
- finish_expr_stmt (ret_val_dtor);
- finish_then_clause (gro_d_if);
- finish_if_stmt (gro_d_if);
- }
-
/* Before initial resume is called, the responsibility for cleanup on
exception falls to the ramp. After that, the coroutine body code
should do the cleanup. This is signalled by the flag
@@ -5245,8 +5196,11 @@ cp_coroutine_transform::build_ramp_function ()
tree not_iarc
= build1_loc (loc, TRUTH_NOT_EXPR, boolean_type_node, iarc_x);
+ tree do_cleanup = build2_loc (loc, TRUTH_AND_EXPR, boolean_type_node,
+ coro_before_return, not_iarc);
tree cleanup_if = begin_if_stmt ();
- finish_if_stmt_cond (not_iarc, cleanup_if);
+ finish_if_stmt_cond (do_cleanup, cleanup_if);
+
/* If the promise is live, then run its dtor if that's available. */
if (promise_dtor && promise_dtor != error_mark_node)
{
@@ -5299,6 +5253,7 @@ cp_coroutine_transform::build_ramp_function ()
finish_handler (handler);
finish_handler_sequence (ramp_try_block);
}
+
finish_compound_stmt (ramp_fnbody);
return true;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/pr119916.C
b/gcc/testsuite/g++.dg/coroutines/torture/pr119916.C
new file mode 100644
index 00000000000..1976209a1f7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/pr119916.C
@@ -0,0 +1,66 @@
+// PR c++/119916
+// { dg-do run }
+// { dg-additional-options "-std=c++23" }
+
+namespace folly {
+struct OptionalPromise;
+struct Optional {
+ typedef OptionalPromise promise_type;
+ Optional() { hasValue = 0; }
+ Optional(int) { hasValue = 1; }
+ bool has_value() { return hasValue; }
+ int emptyState;
+ int value;
+ bool hasValue;
+};
+} // namespace folly
+namespace std {
+template <typename _Result, typename> struct coroutine_traits : _Result {};
+template <typename = void> struct coroutine_handle;
+template <> struct coroutine_handle<> {
+ void *address();
+};
+template <typename> struct coroutine_handle {
+ static coroutine_handle from_address(void *) {
+ coroutine_handle __self;
+ return __self;
+ }
+ coroutine_handle<> __trans_tmp_1;
+ operator coroutine_handle<>() { return __trans_tmp_1; }
+};
+struct suspend_never {
+ bool await_ready() noexcept { return true; }
+ void await_suspend(coroutine_handle<>) noexcept {}
+ void await_resume() noexcept {}
+};
+} // namespace std
+namespace folly {
+struct OptionalPromiseReturn;
+struct OptionalPromise {
+ Optional *value_;
+ OptionalPromiseReturn get_return_object();
+ std::suspend_never initial_suspend() { return {}; }
+ std::suspend_never final_suspend() noexcept { return {}; }
+ template <typename U> void return_value(U u) { *value_ = u; }
+ void unhandled_exception() {}
+};
+struct OptionalPromiseReturn {
+ Optional storage_;
+ Optional *&pointer_;
+ OptionalPromiseReturn(OptionalPromise &p) : pointer_{p.value_} {
+ pointer_ = &storage_;
+ }
+ operator Optional() { return storage_; }
+};
+OptionalPromiseReturn OptionalPromise::get_return_object() {
+ return *this;
+}
+} // namespace folly
+folly::Optional main_r0 = [] -> folly::Optional {
+ auto z(0);
+ co_return z;
+}();
+int main() {
+ if (!main_r0.has_value())
+ __builtin_abort();
+}
diff --git
a/gcc/testsuite/g++.dg/coroutines/torture/special-termination-00-sync-completion.C
b/gcc/testsuite/g++.dg/coroutines/torture/special-termination-00-sync-completion.C
index 21ce721d0ac..a24cc8a22c6 100644
---
a/gcc/testsuite/g++.dg/coroutines/torture/special-termination-00-sync-completion.C
+++
b/gcc/testsuite/g++.dg/coroutines/torture/special-termination-00-sync-completion.C
@@ -87,7 +87,7 @@ struct coro1 {
// The coro1 ramp return object will be constructed from this.
auto get_return_object () {
PRINT ("get_return_object: handle from promise");
- return handle_type::from_promise (*this);
+ return coro1 (handle_type::from_promise (*this));
}
auto initial_suspend () { return suspend_never_prt{}; }
diff --git
a/gcc/testsuite/g++.dg/coroutines/torture/special-termination-01-self-destruct.C
b/gcc/testsuite/g++.dg/coroutines/torture/special-termination-01-self-destruct.C
index 609b860ad02..e3327c2eaa5 100644
---
a/gcc/testsuite/g++.dg/coroutines/torture/special-termination-01-self-destruct.C
+++
b/gcc/testsuite/g++.dg/coroutines/torture/special-termination-01-self-destruct.C
@@ -90,7 +90,7 @@ struct coro1 {
// The coro1 ramp return object will be constructed from this.
auto get_return_object () {
PRINT ("get_return_object: handle from promise");
- return handle_type::from_promise (*this);
+ return coro1(handle_type::from_promise (*this));
}
auto initial_suspend () { return suspend_always_self_destr_prt{}; }
--
2.39.2 (Apple Git-143)
