Message ID | D289C888-DBF7-4B7D-9934-5936B570410A@sandoe.co.uk |
---|---|
State | New |
Headers | show |
Series | coroutines: Add a cleanup expression for g-r-o when needed [PR95477]. | expand |
On 6/8/20 5:17 AM, Iain Sandoe wrote: > Hi > > The PR reports that we fail to destroy the object initially created from > the get-return-object call. Fixed by adding a cleanup when the DTOR is > non-trivial. In addition, to meet the specific wording that the call to > get_return_object creates the glvalue for the return, we must construct > that in-place in the return object to avoid a second copy/move CTOR. > > tested on x86_64,powerpc64-linux, x86_64-darwin > OK for master? > 10.2? > thanks > Iain > > gcc/cp/ChangeLog: > > PR c++/95477 > * coroutines.cc (morph_fn_to_coro): Apply a cleanup to > the get return object when the DTOR is non-trivial. > > gcc/testsuite/ChangeLog: > > * g++.dg/coroutines/pr95477.C: New test. > --- > gcc/cp/coroutines.cc | 61 +++++++++++++++++++---- > gcc/testsuite/g++.dg/coroutines/pr95477.C | 37 ++++++++++++++ > 2 files changed, 89 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95477.C > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index f2d7853477d..5a78bec1c9a 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -4284,12 +4284,34 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > > tree gro = NULL_TREE; > tree gro_bind_vars = NULL_TREE; > + tree gro_cleanup_stmt = NULL_TREE; > /* We have to sequence the call to get_return_object before initial > suspend. */ > if (gro_is_void_p) > - finish_expr_stmt (get_ro); > + r = get_ro; > + else if (same_type_p (gro_type, fn_return_type)) > + { > + /* [dcl.fct.def.coroutine] / 7 > + The expression promise.get_return_object() is used to initialize the > + glvalue result or... (see below) > + Construct the return result directly. */ > + if (TYPE_NEEDS_CONSTRUCTING (gro_type)) > + { > + vec<tree, va_gc> *arg = make_tree_vector_single (get_ro); > + r = build_special_member_call (DECL_RESULT (orig), > + complete_ctor_identifier, > + &arg, gro_type, LOOKUP_NORMAL, > + tf_warning_or_error); > + release_tree_vector (arg); > + } > + else > + r = build2_loc (fn_start, INIT_EXPR, gro_type, > + DECL_RESULT (orig), get_ro); > + } > else > { > + /* ... or ... Construct an object that will be used as the single > + param to the CTOR for the return object. */ > gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"), gro_type); > DECL_CONTEXT (gro) = current_scope (); > DECL_ARTIFICIAL (gro) = true; > @@ -4306,8 +4328,21 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > } > else > r = build2_loc (fn_start, INIT_EXPR, gro_type, gro, get_ro); > - finish_expr_stmt (r); > + /* The constructed object might require a cleanup. */ > + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type)) > + { > + tree cleanup > + = build_special_member_call (gro, complete_dtor_identifier, > + NULL, gro_type, LOOKUP_NORMAL, > + tf_warning_or_error); > + gro_cleanup_stmt = build_stmt (input_location, CLEANUP_STMT, NULL, > + cleanup, gro); > + } > } > + finish_expr_stmt (r); > + > + if (gro_cleanup_stmt) > + CLEANUP_BODY (gro_cleanup_stmt) = push_stmt_list (); > > /* Initialize the resume_idx_name to 0, meaning "not started". */ > tree resume_idx_m > @@ -4349,14 +4384,15 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > promise was constructed. We now supply a reference to that var, > either as the return value (if it's the same type) or to the CTOR > for an object of the return type. */ > - if (gro_is_void_p) > - r = NULL_TREE; > - else > - r = rvalue (gro); > > - if (!same_type_p (gro_type, fn_return_type)) > + if (same_type_p (gro_type, fn_return_type)) > + r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig); > + else > { > - /* The return object is , even if the gro is void. */ > + /* If we have void gro and a non-class return type, then pick a > + defensive initialisation value. */ > + r = gro_is_void_p ? integer_zero_node : rvalue (gro); > + /* The return object is constructed, even if the gro is void. */ Would error_mark_node work here? I presume we've already diagnosed the problem, so will result in no cascade of errors. > if (CLASS_TYPE_P (fn_return_type)) > { > vec<tree, va_gc> *args = NULL; > @@ -4374,12 +4410,19 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > if (args) > release_tree_vector (args); > } > - else /* ??? suppose we have non-class return and void gro? */ > + else > r = build1_loc (input_location, CONVERT_EXPR, fn_return_type, r); > } > > finish_return_stmt (r); > > + if (gro_cleanup_stmt) > + { > + CLEANUP_BODY (gro_cleanup_stmt) > + = pop_stmt_list (CLEANUP_BODY (gro_cleanup_stmt)); > + add_stmt (gro_cleanup_stmt); > + } > + > /* 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); > diff --git a/gcc/testsuite/g++.dg/coroutines/pr95477.C b/gcc/testsuite/g++.dg/coroutines/pr95477.C > new file mode 100644 > index 00000000000..7050aee0078 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/coroutines/pr95477.C > @@ -0,0 +1,37 @@ > +// { dg-do run } > + > +#include "coro.h" > + > +struct simple { > + static inline int alive = 0; > + simple() { ++alive; } > + simple(simple&&) { ++alive; } > + ~simple() { --alive; } > + > + struct promise_type { > + simple get_return_object() { return simple{}; } > + void return_void() {} > + void unhandled_exception() {} > + auto initial_suspend() noexcept { return coro::suspend_never{}; } > + auto final_suspend() noexcept { return coro::suspend_never{}; } > + }; > +}; > + > +simple > +f() > +{ > + co_return; > +} > + > +int main() { > + { > + f(); > + } > + > + if (simple::alive != 0) > + { > + PRINTF ("something wrong with dtors: %d\n", simple::alive); > + abort (); > + } > + return 0; > +} >
Nathan Sidwell <nathan@acm.org> wrote: > On 6/8/20 5:17 AM, Iain Sandoe wrote: >> Hi >> The PR reports that we fail to destroy the object initially created from >> the get-return-object call. Fixed by adding a cleanup when the DTOR is >> non-trivial. In addition, to meet the specific wording that the call to >> get_return_object creates the glvalue for the return, we must construct >> that in-place in the return object to avoid a second copy/move CTOR. >> tested on x86_64,powerpc64-linux, x86_64-darwin >> OK for master? >> 10.2? >> thanks >> Iain >> gcc/cp/ChangeLog: >> PR c++/95477 >> * coroutines.cc (morph_fn_to_coro): Apply a cleanup to >> the get return object when the DTOR is non-trivial. >> gcc/testsuite/ChangeLog: >> * g++.dg/coroutines/pr95477.C: New test. >> --- >> gcc/cp/coroutines.cc | 61 +++++++++++++++++++---- >> gcc/testsuite/g++.dg/coroutines/pr95477.C | 37 ++++++++++++++ >> 2 files changed, 89 insertions(+), 9 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95477.C >> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc >> index f2d7853477d..5a78bec1c9a 100644 >> --- a/gcc/cp/coroutines.cc >> +++ b/gcc/cp/coroutines.cc >> @@ -4284,12 +4284,34 @@ morph_fn_to_coro (tree orig, tree *resumer, tree >> *destroyer) >> tree gro = NULL_TREE; >> tree gro_bind_vars = NULL_TREE; >> + tree gro_cleanup_stmt = NULL_TREE; >> /* We have to sequence the call to get_return_object before initial >> suspend. */ >> if (gro_is_void_p) >> - finish_expr_stmt (get_ro); >> + r = get_ro; >> + else if (same_type_p (gro_type, fn_return_type)) >> + { >> + /* [dcl.fct.def.coroutine] / 7 >> + The expression promise.get_return_object() is used to initialize the >> + glvalue result or... (see below) >> + Construct the return result directly. */ >> + if (TYPE_NEEDS_CONSTRUCTING (gro_type)) >> + { >> + vec<tree, va_gc> *arg = make_tree_vector_single (get_ro); >> + r = build_special_member_call (DECL_RESULT (orig), >> + complete_ctor_identifier, >> + &arg, gro_type, LOOKUP_NORMAL, >> + tf_warning_or_error); >> + release_tree_vector (arg); >> + } >> + else >> + r = build2_loc (fn_start, INIT_EXPR, gro_type, >> + DECL_RESULT (orig), get_ro); >> + } >> else >> { >> + /* ... or ... Construct an object that will be used as the single >> + param to the CTOR for the return object. */ >> gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"), gro_type); >> DECL_CONTEXT (gro) = current_scope (); >> DECL_ARTIFICIAL (gro) = true; >> @@ -4306,8 +4328,21 @@ morph_fn_to_coro (tree orig, tree *resumer, tree >> *destroyer) >> } >> else >> r = build2_loc (fn_start, INIT_EXPR, gro_type, gro, get_ro); >> - finish_expr_stmt (r); >> + /* The constructed object might require a cleanup. */ >> + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type)) >> + { >> + tree cleanup >> + = build_special_member_call (gro, complete_dtor_identifier, >> + NULL, gro_type, LOOKUP_NORMAL, >> + tf_warning_or_error); >> + gro_cleanup_stmt = build_stmt (input_location, CLEANUP_STMT, NULL, >> + cleanup, gro); >> + } >> } >> + finish_expr_stmt (r); >> + >> + if (gro_cleanup_stmt) >> + CLEANUP_BODY (gro_cleanup_stmt) = push_stmt_list (); >> /* Initialize the resume_idx_name to 0, meaning "not started". */ >> tree resume_idx_m >> @@ -4349,14 +4384,15 @@ morph_fn_to_coro (tree orig, tree *resumer, tree >> *destroyer) >> promise was constructed. We now supply a reference to that var, >> either as the return value (if it's the same type) or to the CTOR >> for an object of the return type. */ >> - if (gro_is_void_p) >> - r = NULL_TREE; >> - else >> - r = rvalue (gro); >> - if (!same_type_p (gro_type, fn_return_type)) >> + if (same_type_p (gro_type, fn_return_type)) >> + r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig); >> + else >> { >> - /* The return object is , even if the gro is void. */ >> + /* If we have void gro and a non-class return type, then pick a >> + defensive initialisation value. */ >> + r = gro_is_void_p ? integer_zero_node : rvalue (gro); >> + /* The return object is constructed, even if the gro is void. */ > > Would error_mark_node work here? I presume we've already diagnosed the > problem, so will result in no cascade of errors. We have not diagnosed the problem - it’s valid to have a void g-r-o and a non-void function return. I am not sure the circumstance has been identified specifically where this (valid) permutation is found together with a specialization of the traits that has a non-class return type. (I spotted this as a corner-case while working on the code). Perhaps I should construct a test-case and see how the other compilers handle it. Do you want me to do that as part of this fix, or can it be a follow-on? thanks Iain > >> if (CLASS_TYPE_P (fn_return_type)) >> { >> vec<tree, va_gc> *args = NULL; >> @@ -4374,12 +4410,19 @@ morph_fn_to_coro (tree orig, tree *resumer, tree >> *destroyer) >> if (args) >> release_tree_vector (args); >> } >> - else /* ??? suppose we have non-class return and void gro? */ >> + else >> r = build1_loc (input_location, CONVERT_EXPR, fn_return_type, r); >> } >> finish_return_stmt (r); >> + if (gro_cleanup_stmt) >> + { >> + CLEANUP_BODY (gro_cleanup_stmt) >> + = pop_stmt_list (CLEANUP_BODY (gro_cleanup_stmt)); >> + add_stmt (gro_cleanup_stmt); >> + } >> + >> /* 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); >> diff --git a/gcc/testsuite/g++.dg/coroutines/pr95477.C >> b/gcc/testsuite/g++.dg/coroutines/pr95477.C >> new file mode 100644 >> index 00000000000..7050aee0078 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/coroutines/pr95477.C >> @@ -0,0 +1,37 @@ >> +// { dg-do run } >> + >> +#include "coro.h" >> + >> +struct simple { >> + static inline int alive = 0; >> + simple() { ++alive; } >> + simple(simple&&) { ++alive; } >> + ~simple() { --alive; } >> + >> + struct promise_type { >> + simple get_return_object() { return simple{}; } >> + void return_void() {} >> + void unhandled_exception() {} >> + auto initial_suspend() noexcept { return coro::suspend_never{}; } >> + auto final_suspend() noexcept { return coro::suspend_never{}; } >> + }; >> +}; >> + >> +simple >> +f() >> +{ >> + co_return; >> +} >> + >> +int main() { >> + { >> + f(); >> + } >> + >> + if (simple::alive != 0) >> + { >> + PRINTF ("something wrong with dtors: %d\n", simple::alive); >> + abort (); >> + } >> + return 0; >> +} > > > -- > Nathan Sidwell
Iain Sandoe <iain@sandoe.co.uk> wrote: > Nathan Sidwell <nathan@acm.org> wrote: > >> On 6/8/20 5:17 AM, Iain Sandoe wrote: >>> + r = gro_is_void_p ? integer_zero_node : rvalue (gro); >>> + /* The return object is constructed, even if the gro is void. */ >> >> Would error_mark_node work here? I presume we've already diagnosed the problem, so will result in no cascade of errors. > > We have not diagnosed the problem - it’s valid to have a void g-r-o and a non-void function > return. > > I am not sure the circumstance has been identified specifically where this (valid) permutation > is found together with a specialization of the traits that has a non-class return type. (I spotted > this as a corner-case while working on the code). > > Perhaps I should construct a test-case and see how the other compilers handle it. I did this anyway … To answer your original question, we need to diagnose this specifically. clang does so, I’ve now matched the diagnostic for GCC. we can then propagate an error_mark_node onwards. OK now? Iain === The PR reports that we fail to destroy the object initially created from the get-return-object call. Fixed by adding a cleanup when the DTOR is non-trivial. In addition, to meet the specific wording that the call to get_return_object creates the glvalue for the return, we must construct that in-place in the return object to avoid a second copy/move CTOR. gcc/cp/ChangeLog: PR c++/95477 * coroutines.cc (morph_fn_to_coro): Apply a cleanup to the get return object when the DTOR is non-trivial. gcc/testsuite/ChangeLog: PR c++/95477 * g++.dg/coroutines/pr95477.C: New test. * g++.dg/coroutines/void-gro-non-class-coro.C: New test. --- gcc/cp/coroutines.cc | 69 ++++++++++++++++--- gcc/testsuite/g++.dg/coroutines/pr95477.C | 37 ++++++++++ .../coroutines/void-gro-non-class-coro.C | 59 ++++++++++++++++ 3 files changed, 155 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95477.C create mode 100644 gcc/testsuite/g++.dg/coroutines/void-gro-non-class-coro.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 11fca9954ac..d4f582e91e2 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -4280,12 +4280,34 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) tree gro = NULL_TREE; tree gro_bind_vars = NULL_TREE; + tree gro_cleanup_stmt = NULL_TREE; /* We have to sequence the call to get_return_object before initial suspend. */ if (gro_is_void_p) - finish_expr_stmt (get_ro); + r = get_ro; + else if (same_type_p (gro_type, fn_return_type)) + { + /* [dcl.fct.def.coroutine] / 7 + The expression promise.get_return_object() is used to initialize the + glvalue result or... (see below) + Construct the return result directly. */ + if (TYPE_NEEDS_CONSTRUCTING (gro_type)) + { + vec<tree, va_gc> *arg = make_tree_vector_single (get_ro); + r = build_special_member_call (DECL_RESULT (orig), + complete_ctor_identifier, + &arg, gro_type, LOOKUP_NORMAL, + tf_warning_or_error); + release_tree_vector (arg); + } + else + r = build2_loc (fn_start, INIT_EXPR, gro_type, + DECL_RESULT (orig), get_ro); + } else { + /* ... or ... Construct an object that will be used as the single + param to the CTOR for the return object. */ gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"), gro_type); DECL_CONTEXT (gro) = current_scope (); DECL_ARTIFICIAL (gro) = true; @@ -4302,8 +4324,21 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) } else r = build2_loc (fn_start, INIT_EXPR, gro_type, gro, get_ro); - finish_expr_stmt (r); + /* The constructed object might require a cleanup. */ + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type)) + { + tree cleanup + = build_special_member_call (gro, complete_dtor_identifier, + NULL, gro_type, LOOKUP_NORMAL, + tf_warning_or_error); + gro_cleanup_stmt = build_stmt (input_location, CLEANUP_STMT, NULL, + cleanup, gro); + } } + finish_expr_stmt (r); + + if (gro_cleanup_stmt) + CLEANUP_BODY (gro_cleanup_stmt) = push_stmt_list (); /* Initialize the resume_idx_name to 0, meaning "not started". */ tree resume_idx_m @@ -4345,16 +4380,15 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) promise was constructed. We now supply a reference to that var, either as the return value (if it's the same type) or to the CTOR for an object of the return type. */ - if (gro_is_void_p) - r = NULL_TREE; - else - r = rvalue (gro); - if (!same_type_p (gro_type, fn_return_type)) + if (same_type_p (gro_type, fn_return_type)) + r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig); + else { - /* The return object is , even if the gro is void. */ if (CLASS_TYPE_P (fn_return_type)) { + /* For class type return objects, we can attempt to construct, + even if the gro is void. */ vec<tree, va_gc> *args = NULL; vec<tree, va_gc> **arglist = NULL; if (!gro_is_void_p) @@ -4370,12 +4404,27 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) if (args) release_tree_vector (args); } - else /* ??? suppose we have non-class return and void gro? */ - r = build1_loc (input_location, CONVERT_EXPR, fn_return_type, r); + else if (gro_is_void_p) + { + /* We can't initialize a non-class return value from void. */ + error_at (input_location, "cannot initialize a return object of type" + " %qT with an rvalue of type %<void%>", fn_return_type); + r = error_mark_node; + } + else + r = build1_loc (input_location, CONVERT_EXPR, + fn_return_type, rvalue (gro)); } finish_return_stmt (r); + if (gro_cleanup_stmt) + { + CLEANUP_BODY (gro_cleanup_stmt) + = pop_stmt_list (CLEANUP_BODY (gro_cleanup_stmt)); + add_stmt (gro_cleanup_stmt); + } + /* 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); diff --git a/gcc/testsuite/g++.dg/coroutines/pr95477.C b/gcc/testsuite/g++.dg/coroutines/pr95477.C new file mode 100644 index 00000000000..7050aee0078 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr95477.C @@ -0,0 +1,37 @@ +// { dg-do run } + +#include "coro.h" + +struct simple { + static inline int alive = 0; + simple() { ++alive; } + simple(simple&&) { ++alive; } + ~simple() { --alive; } + + struct promise_type { + simple get_return_object() { return simple{}; } + void return_void() {} + void unhandled_exception() {} + auto initial_suspend() noexcept { return coro::suspend_never{}; } + auto final_suspend() noexcept { return coro::suspend_never{}; } + }; +}; + +simple +f() +{ + co_return; +} + +int main() { + { + f(); + } + + if (simple::alive != 0) + { + PRINTF ("something wrong with dtors: %d\n", simple::alive); + abort (); + } + return 0; +} diff --git a/gcc/testsuite/g++.dg/coroutines/void-gro-non-class-coro.C b/gcc/testsuite/g++.dg/coroutines/void-gro-non-class-coro.C new file mode 100644 index 00000000000..d9e53fe4c76 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/void-gro-non-class-coro.C @@ -0,0 +1,59 @@ +// Test handling of the case where we have a void g-r-o and a non-void +// and non-class-type ramp return. + +#include "coro.h" + +int g_promise = -1; + +template<typename R, typename HandleRef, typename ...T> +struct std::coroutine_traits<R, HandleRef, T...> { + struct promise_type { + promise_type (HandleRef h, T ...args) + { h = std::coroutine_handle<promise_type>::from_promise (*this); + PRINT ("Created Promise"); + g_promise = 1; + } + ~promise_type () { PRINT ("Destroyed Promise"); g_promise = 0;} + void get_return_object() {} + + auto initial_suspend() { + return std::suspend_always{}; + } + auto final_suspend() { return std::suspend_never{}; } + + void return_void() {} + void unhandled_exception() {} + }; +}; + +int +my_coro (std::coroutine_handle<>& h) +{ + PRINT ("coro1: about to return"); + co_return; +} // dg-error {cannot initialize a return object of type ‘int’ with an rvalue of type ‘void’} + +int main () +{ + std::coroutine_handle<> h; + int t = my_coro (h); + + if (h.done()) + { + PRINT ("main: apparently was already done..."); + abort (); + } + + // initial suspend. + h.resume (); + + // The coro should have self-destructed. + if (g_promise) + { + PRINT ("main: apparently we did not complete..."); + abort (); + } + + PRINT ("main: returning"); + return 0; +}
On 6/12/20 4:06 PM, Iain Sandoe wrote: > Iain Sandoe <iain@sandoe.co.uk> wrote: > >> Nathan Sidwell <nathan@acm.org> wrote: >> >>> On 6/8/20 5:17 AM, Iain Sandoe wrote: > >>>> + r = gro_is_void_p ? integer_zero_node : rvalue (gro); >>>> + /* The return object is constructed, even if the gro is void. */ >>> >>> Would error_mark_node work here? I presume we've already diagnosed the problem, so will result in no cascade of errors. >> >> We have not diagnosed the problem - it’s valid to have a void g-r-o and a non-void function >> return. >> >> I am not sure the circumstance has been identified specifically where this (valid) permutation >> is found together with a specialization of the traits that has a non-class return type. (I spotted >> this as a corner-case while working on the code). >> >> Perhaps I should construct a test-case and see how the other compilers handle it. > > I did this anyway … > > To answer your original question, we need to diagnose this specifically. > clang does so, I’ve now matched the diagnostic for GCC. > we can then propagate an error_mark_node onwards. > > OK now? > Iain ok, thanks! > > === > > The PR reports that we fail to destroy the object initially created from > the get-return-object call. Fixed by adding a cleanup when the DTOR is > non-trivial. In addition, to meet the specific wording that the call to > get_return_object creates the glvalue for the return, we must construct > that in-place in the return object to avoid a second copy/move CTOR. > > gcc/cp/ChangeLog: > > PR c++/95477 > * coroutines.cc (morph_fn_to_coro): Apply a cleanup to > the get return object when the DTOR is non-trivial. > > gcc/testsuite/ChangeLog: > > PR c++/95477 > * g++.dg/coroutines/pr95477.C: New test. > * g++.dg/coroutines/void-gro-non-class-coro.C: New test. > --- > gcc/cp/coroutines.cc | 69 ++++++++++++++++--- > gcc/testsuite/g++.dg/coroutines/pr95477.C | 37 ++++++++++ > .../coroutines/void-gro-non-class-coro.C | 59 ++++++++++++++++ > 3 files changed, 155 insertions(+), 10 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95477.C > create mode 100644 gcc/testsuite/g++.dg/coroutines/void-gro-non-class-coro.C > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index 11fca9954ac..d4f582e91e2 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -4280,12 +4280,34 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > > tree gro = NULL_TREE; > tree gro_bind_vars = NULL_TREE; > + tree gro_cleanup_stmt = NULL_TREE; > /* We have to sequence the call to get_return_object before initial > suspend. */ > if (gro_is_void_p) > - finish_expr_stmt (get_ro); > + r = get_ro; > + else if (same_type_p (gro_type, fn_return_type)) > + { > + /* [dcl.fct.def.coroutine] / 7 > + The expression promise.get_return_object() is used to initialize the > + glvalue result or... (see below) > + Construct the return result directly. */ > + if (TYPE_NEEDS_CONSTRUCTING (gro_type)) > + { > + vec<tree, va_gc> *arg = make_tree_vector_single (get_ro); > + r = build_special_member_call (DECL_RESULT (orig), > + complete_ctor_identifier, > + &arg, gro_type, LOOKUP_NORMAL, > + tf_warning_or_error); > + release_tree_vector (arg); > + } > + else > + r = build2_loc (fn_start, INIT_EXPR, gro_type, > + DECL_RESULT (orig), get_ro); > + } > else > { > + /* ... or ... Construct an object that will be used as the single > + param to the CTOR for the return object. */ > gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"), gro_type); > DECL_CONTEXT (gro) = current_scope (); > DECL_ARTIFICIAL (gro) = true; > @@ -4302,8 +4324,21 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > } > else > r = build2_loc (fn_start, INIT_EXPR, gro_type, gro, get_ro); > - finish_expr_stmt (r); > + /* The constructed object might require a cleanup. */ > + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type)) > + { > + tree cleanup > + = build_special_member_call (gro, complete_dtor_identifier, > + NULL, gro_type, LOOKUP_NORMAL, > + tf_warning_or_error); > + gro_cleanup_stmt = build_stmt (input_location, CLEANUP_STMT, NULL, > + cleanup, gro); > + } > } > + finish_expr_stmt (r); > + > + if (gro_cleanup_stmt) > + CLEANUP_BODY (gro_cleanup_stmt) = push_stmt_list (); > > /* Initialize the resume_idx_name to 0, meaning "not started". */ > tree resume_idx_m > @@ -4345,16 +4380,15 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > promise was constructed. We now supply a reference to that var, > either as the return value (if it's the same type) or to the CTOR > for an object of the return type. */ > - if (gro_is_void_p) > - r = NULL_TREE; > - else > - r = rvalue (gro); > > - if (!same_type_p (gro_type, fn_return_type)) > + if (same_type_p (gro_type, fn_return_type)) > + r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig); > + else > { > - /* The return object is , even if the gro is void. */ > if (CLASS_TYPE_P (fn_return_type)) > { > + /* For class type return objects, we can attempt to construct, > + even if the gro is void. */ > vec<tree, va_gc> *args = NULL; > vec<tree, va_gc> **arglist = NULL; > if (!gro_is_void_p) > @@ -4370,12 +4404,27 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > if (args) > release_tree_vector (args); > } > - else /* ??? suppose we have non-class return and void gro? */ > - r = build1_loc (input_location, CONVERT_EXPR, fn_return_type, r); > + else if (gro_is_void_p) > + { > + /* We can't initialize a non-class return value from void. */ > + error_at (input_location, "cannot initialize a return object of type" > + " %qT with an rvalue of type %<void%>", fn_return_type); > + r = error_mark_node; > + } > + else > + r = build1_loc (input_location, CONVERT_EXPR, > + fn_return_type, rvalue (gro)); > } > > finish_return_stmt (r); > > + if (gro_cleanup_stmt) > + { > + CLEANUP_BODY (gro_cleanup_stmt) > + = pop_stmt_list (CLEANUP_BODY (gro_cleanup_stmt)); > + add_stmt (gro_cleanup_stmt); > + } > + > /* 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); > diff --git a/gcc/testsuite/g++.dg/coroutines/pr95477.C b/gcc/testsuite/g++.dg/coroutines/pr95477.C > new file mode 100644 > index 00000000000..7050aee0078 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/coroutines/pr95477.C > @@ -0,0 +1,37 @@ > +// { dg-do run } > + > +#include "coro.h" > + > +struct simple { > + static inline int alive = 0; > + simple() { ++alive; } > + simple(simple&&) { ++alive; } > + ~simple() { --alive; } > + > + struct promise_type { > + simple get_return_object() { return simple{}; } > + void return_void() {} > + void unhandled_exception() {} > + auto initial_suspend() noexcept { return coro::suspend_never{}; } > + auto final_suspend() noexcept { return coro::suspend_never{}; } > + }; > +}; > + > +simple > +f() > +{ > + co_return; > +} > + > +int main() { > + { > + f(); > + } > + > + if (simple::alive != 0) > + { > + PRINTF ("something wrong with dtors: %d\n", simple::alive); > + abort (); > + } > + return 0; > +} > diff --git a/gcc/testsuite/g++.dg/coroutines/void-gro-non-class-coro.C b/gcc/testsuite/g++.dg/coroutines/void-gro-non-class-coro.C > new file mode 100644 > index 00000000000..d9e53fe4c76 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/coroutines/void-gro-non-class-coro.C > @@ -0,0 +1,59 @@ > +// Test handling of the case where we have a void g-r-o and a non-void > +// and non-class-type ramp return. > + > +#include "coro.h" > + > +int g_promise = -1; > + > +template<typename R, typename HandleRef, typename ...T> > +struct std::coroutine_traits<R, HandleRef, T...> { > + struct promise_type { > + promise_type (HandleRef h, T ...args) > + { h = std::coroutine_handle<promise_type>::from_promise (*this); > + PRINT ("Created Promise"); > + g_promise = 1; > + } > + ~promise_type () { PRINT ("Destroyed Promise"); g_promise = 0;} > + void get_return_object() {} > + > + auto initial_suspend() { > + return std::suspend_always{}; > + } > + auto final_suspend() { return std::suspend_never{}; } > + > + void return_void() {} > + void unhandled_exception() {} > + }; > +}; > + > +int > +my_coro (std::coroutine_handle<>& h) > +{ > + PRINT ("coro1: about to return"); > + co_return; > +} // dg-error {cannot initialize a return object of type ‘int’ with an rvalue of type ‘void’} > + > +int main () > +{ > + std::coroutine_handle<> h; > + int t = my_coro (h); > + > + if (h.done()) > + { > + PRINT ("main: apparently was already done..."); > + abort (); > + } > + > + // initial suspend. > + h.resume (); > + > + // The coro should have self-destructed. > + if (g_promise) > + { > + PRINT ("main: apparently we did not complete..."); > + abort (); > + } > + > + PRINT ("main: returning"); > + return 0; > +} >
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index f2d7853477d..5a78bec1c9a 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -4284,12 +4284,34 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) tree gro = NULL_TREE; tree gro_bind_vars = NULL_TREE; + tree gro_cleanup_stmt = NULL_TREE; /* We have to sequence the call to get_return_object before initial suspend. */ if (gro_is_void_p) - finish_expr_stmt (get_ro); + r = get_ro; + else if (same_type_p (gro_type, fn_return_type)) + { + /* [dcl.fct.def.coroutine] / 7 + The expression promise.get_return_object() is used to initialize the + glvalue result or... (see below) + Construct the return result directly. */ + if (TYPE_NEEDS_CONSTRUCTING (gro_type)) + { + vec<tree, va_gc> *arg = make_tree_vector_single (get_ro); + r = build_special_member_call (DECL_RESULT (orig), + complete_ctor_identifier, + &arg, gro_type, LOOKUP_NORMAL, + tf_warning_or_error); + release_tree_vector (arg); + } + else + r = build2_loc (fn_start, INIT_EXPR, gro_type, + DECL_RESULT (orig), get_ro); + } else { + /* ... or ... Construct an object that will be used as the single + param to the CTOR for the return object. */ gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"), gro_type); DECL_CONTEXT (gro) = current_scope (); DECL_ARTIFICIAL (gro) = true; @@ -4306,8 +4328,21 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) } else r = build2_loc (fn_start, INIT_EXPR, gro_type, gro, get_ro); - finish_expr_stmt (r); + /* The constructed object might require a cleanup. */ + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type)) + { + tree cleanup + = build_special_member_call (gro, complete_dtor_identifier, + NULL, gro_type, LOOKUP_NORMAL, + tf_warning_or_error); + gro_cleanup_stmt = build_stmt (input_location, CLEANUP_STMT, NULL, + cleanup, gro); + } } + finish_expr_stmt (r); + + if (gro_cleanup_stmt) + CLEANUP_BODY (gro_cleanup_stmt) = push_stmt_list (); /* Initialize the resume_idx_name to 0, meaning "not started". */ tree resume_idx_m @@ -4349,14 +4384,15 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) promise was constructed. We now supply a reference to that var, either as the return value (if it's the same type) or to the CTOR for an object of the return type. */ - if (gro_is_void_p) - r = NULL_TREE; - else - r = rvalue (gro); - if (!same_type_p (gro_type, fn_return_type)) + if (same_type_p (gro_type, fn_return_type)) + r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig); + else { - /* The return object is , even if the gro is void. */ + /* If we have void gro and a non-class return type, then pick a + defensive initialisation value. */ + r = gro_is_void_p ? integer_zero_node : rvalue (gro); + /* The return object is constructed, even if the gro is void. */ if (CLASS_TYPE_P (fn_return_type)) { vec<tree, va_gc> *args = NULL; @@ -4374,12 +4410,19 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) if (args) release_tree_vector (args); } - else /* ??? suppose we have non-class return and void gro? */ + else r = build1_loc (input_location, CONVERT_EXPR, fn_return_type, r); } finish_return_stmt (r); + if (gro_cleanup_stmt) + { + CLEANUP_BODY (gro_cleanup_stmt) + = pop_stmt_list (CLEANUP_BODY (gro_cleanup_stmt)); + add_stmt (gro_cleanup_stmt); + } + /* 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); diff --git a/gcc/testsuite/g++.dg/coroutines/pr95477.C b/gcc/testsuite/g++.dg/coroutines/pr95477.C new file mode 100644 index 00000000000..7050aee0078 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr95477.C @@ -0,0 +1,37 @@ +// { dg-do run } + +#include "coro.h" + +struct simple { + static inline int alive = 0; + simple() { ++alive; } + simple(simple&&) { ++alive; } + ~simple() { --alive; } + + struct promise_type { + simple get_return_object() { return simple{}; } + void return_void() {} + void unhandled_exception() {} + auto initial_suspend() noexcept { return coro::suspend_never{}; } + auto final_suspend() noexcept { return coro::suspend_never{}; } + }; +}; + +simple +f() +{ + co_return; +} + +int main() { + { + f(); + } + + if (simple::alive != 0) + { + PRINTF ("something wrong with dtors: %d\n", simple::alive); + abort (); + } + return 0; +}