| Message ID | 2014547e103bd589380b26667c54c026c2f9c929.1724267239.git.iain@sandoe.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | c++, coroutines: Patch set for ramp function fixes. | expand |
On 8/21/24 3:10 PM, Iain Sandoe wrote: > In the fix for PR95822 (r11-7402) we set throwing_cleanup false in the top > level of the coroutine transform code. However, as the current PR shows, > that is not sufficient. > > Any use of cxx_maybe_build_cleanup() can reset the flag, which causes the > check_return_expr () logic to try to add a guard variable and set it. > > For the coroutine code, we need to handle the cleanups separately, since > the responsibility for them changes after the first resume point, which > we handle in the ramp exception processing. > > Fix this by forcing the "throwing_cleanup" flag false right before the > processing of the return expression. OK. > PR c++/102051 > > gcc/cp/ChangeLog: > > * coroutines.cc > (cp_coroutine_transform::build_ramp_function): Handle > "throwing_cleanup" here instead of ... > (cp_coroutine_transform::apply_transforms): ... here. > > gcc/testsuite/ChangeLog: > > * g++.dg/coroutines/pr102051.C: New test. > > Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> > --- > gcc/cp/coroutines.cc | 21 +++++++++------------ > gcc/testsuite/g++.dg/coroutines/pr102051.C | 16 ++++++++++++++++ > 2 files changed, 25 insertions(+), 12 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr102051.C > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index 8738ce15533..1039d2f8515 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -5042,6 +5042,10 @@ cp_coroutine_transform::build_ramp_function () > 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; > if (void_ramp_p) > { > gcc_checking_assert (VOID_TYPE_P (TREE_TYPE (get_ro))); > @@ -5050,6 +5054,7 @@ cp_coroutine_transform::build_ramp_function () > } > else > { > + /* The initial section of finish_return_expr (). */ > bool no_warning; > bool dangling; > r = check_return_expr (get_ro, &no_warning, &dangling); > @@ -5090,7 +5095,10 @@ cp_coroutine_transform::build_ramp_function () > the return object we constructed before we called the actor. */ > > r = void_ramp_p ? NULL_TREE : DECL_RESULT (orig_fn_decl); > - finish_return_stmt (r); > + /* The reminder of finish_return_expr (). */ > + r = build_stmt (loc, RETURN_EXPR, r); > + r = maybe_cleanup_point_expr_void (r); > + r = add_stmt (r); > > if (flag_exceptions) > { > @@ -5280,17 +5288,6 @@ cp_coroutine_transform::apply_transforms () > body_blocks = current_binding_level->blocks; > current_binding_level->blocks = NULL_TREE; > > - /* If the original function has a return value with a non-trivial DTOR > - and the body contains a var with a DTOR that might throw, the decl is > - marked "throwing_cleanup". > - We do not [in the ramp, which is synthesised here], use any body var > - types with DTORs that might throw. > - The original body is transformed into the actor function which only > - contains void returns, and is also wrapped in a try-catch block. > - So (a) the 'throwing_cleanup' is not correct for the ramp and (b) we do > - not need to transfer it to the actor which only contains void returns. */ > - cp_function_chain->throwing_cleanup = false; > - > /* Collect information on the original function params and their use in the > function body. */ > param_uses = analyze_fn_parms (orig_fn_decl); > diff --git a/gcc/testsuite/g++.dg/coroutines/pr102051.C b/gcc/testsuite/g++.dg/coroutines/pr102051.C > new file mode 100644 > index 00000000000..bba98b691cc > --- /dev/null > +++ b/gcc/testsuite/g++.dg/coroutines/pr102051.C > @@ -0,0 +1,16 @@ > +#include <coroutine> > + > +struct Foo { > + ~Foo() noexcept(false); // true succeeds > + struct promise_type { > + Foo get_return_object() { return {}; } > + std::suspend_never initial_suspend() { return {}; } > + void return_void() {} > + void unhandled_exception() {} > + std::suspend_always final_suspend() noexcept { return {}; } > + }; > +}; > + > +Foo bar() { > + co_return; > +}
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 8738ce15533..1039d2f8515 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -5042,6 +5042,10 @@ cp_coroutine_transform::build_ramp_function () 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; if (void_ramp_p) { gcc_checking_assert (VOID_TYPE_P (TREE_TYPE (get_ro))); @@ -5050,6 +5054,7 @@ cp_coroutine_transform::build_ramp_function () } else { + /* The initial section of finish_return_expr (). */ bool no_warning; bool dangling; r = check_return_expr (get_ro, &no_warning, &dangling); @@ -5090,7 +5095,10 @@ cp_coroutine_transform::build_ramp_function () the return object we constructed before we called the actor. */ r = void_ramp_p ? NULL_TREE : DECL_RESULT (orig_fn_decl); - finish_return_stmt (r); + /* The reminder of finish_return_expr (). */ + r = build_stmt (loc, RETURN_EXPR, r); + r = maybe_cleanup_point_expr_void (r); + r = add_stmt (r); if (flag_exceptions) { @@ -5280,17 +5288,6 @@ cp_coroutine_transform::apply_transforms () body_blocks = current_binding_level->blocks; current_binding_level->blocks = NULL_TREE; - /* If the original function has a return value with a non-trivial DTOR - and the body contains a var with a DTOR that might throw, the decl is - marked "throwing_cleanup". - We do not [in the ramp, which is synthesised here], use any body var - types with DTORs that might throw. - The original body is transformed into the actor function which only - contains void returns, and is also wrapped in a try-catch block. - So (a) the 'throwing_cleanup' is not correct for the ramp and (b) we do - not need to transfer it to the actor which only contains void returns. */ - cp_function_chain->throwing_cleanup = false; - /* Collect information on the original function params and their use in the function body. */ param_uses = analyze_fn_parms (orig_fn_decl); diff --git a/gcc/testsuite/g++.dg/coroutines/pr102051.C b/gcc/testsuite/g++.dg/coroutines/pr102051.C new file mode 100644 index 00000000000..bba98b691cc --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr102051.C @@ -0,0 +1,16 @@ +#include <coroutine> + +struct Foo { + ~Foo() noexcept(false); // true succeeds + struct promise_type { + Foo get_return_object() { return {}; } + std::suspend_never initial_suspend() { return {}; } + void return_void() {} + void unhandled_exception() {} + std::suspend_always final_suspend() noexcept { return {}; } + }; +}; + +Foo bar() { + co_return; +}
In the fix for PR95822 (r11-7402) we set throwing_cleanup false in the top level of the coroutine transform code. However, as the current PR shows, that is not sufficient. Any use of cxx_maybe_build_cleanup() can reset the flag, which causes the check_return_expr () logic to try to add a guard variable and set it. For the coroutine code, we need to handle the cleanups separately, since the responsibility for them changes after the first resume point, which we handle in the ramp exception processing. Fix this by forcing the "throwing_cleanup" flag false right before the processing of the return expression. PR c++/102051 gcc/cp/ChangeLog: * coroutines.cc (cp_coroutine_transform::build_ramp_function): Handle "throwing_cleanup" here instead of ... (cp_coroutine_transform::apply_transforms): ... here. gcc/testsuite/ChangeLog: * g++.dg/coroutines/pr102051.C: New test. Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> --- gcc/cp/coroutines.cc | 21 +++++++++------------ gcc/testsuite/g++.dg/coroutines/pr102051.C | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr102051.C