From: Iain Sandoe Date: Mon, 1 Jun 2020 07:28:35 +0000 (+0100) Subject: coroutines: Correct handling of references in parm copies [PR95350]. X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=88f48e2967ead9be262483618238efa9c7c842ec;p=gcc.git coroutines: Correct handling of references in parm copies [PR95350]. Adjust to handle rvalue refs the same way as clang, and to correct the handling of moves when a copy CTOR is present. This is one area where we could make things easier for the end-user (as was implemented before this change), however there needs to be agreement about when the full statement containing a coroutine call ends (i.e. when the ramp terminates or when the coroutine terminates). gcc/cp/ChangeLog: PR c++/95350 * coroutines.cc (struct param_info): Remove rv_ref field. (build_actor_fn): Remove specifial rvalue ref handling. (morph_fn_to_coro): Likewise. gcc/testsuite/ChangeLog: PR c++/95350 * g++.dg/coroutines/torture/func-params-08.C: Adjust test to reflect that all rvalue refs are dangling. * g++.dg/coroutines/torture/func-params-09-awaitable-parms.C: Likewise. * g++.dg/coroutines/pr95350.C: New test. --- diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 0abc579e0cb..e90387b4c8d 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -1803,7 +1803,6 @@ struct param_info tree frame_type; /* The type used to represent this parm in the frame. */ tree orig_type; /* The original type of the parm (not as passed). */ bool by_ref; /* Was passed by reference. */ - bool rv_ref; /* Was an rvalue reference. */ bool pt_ref; /* Was a pointer to object. */ bool trivial_dtor; /* The frame type has a trivial DTOR. */ bool this_ptr; /* Is 'this' */ @@ -2073,12 +2072,6 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, if (parm.pt_ref) fld_idx = build1_loc (loc, CONVERT_EXPR, TREE_TYPE (arg), fld_idx); - /* We expect an rvalue ref. here. */ - if (parm.rv_ref) - fld_idx = convert_to_reference (DECL_ARG_TYPE (arg), fld_idx, - CONV_STATIC, LOOKUP_NORMAL, - NULL_TREE, tf_warning_or_error); - int i; tree *puse; FOR_EACH_VEC_ELT (*parm.body_uses, i, puse) @@ -3766,15 +3759,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) if (actual_type == NULL_TREE) actual_type = error_mark_node; parm.orig_type = actual_type; - parm.by_ref = parm.rv_ref = parm.pt_ref = false; - if (TREE_CODE (actual_type) == REFERENCE_TYPE - && TYPE_REF_IS_RVALUE (DECL_ARG_TYPE (arg))) - { - parm.rv_ref = true; - actual_type = TREE_TYPE (actual_type); - parm.frame_type = actual_type; - } - else if (TREE_CODE (actual_type) == REFERENCE_TYPE) + parm.by_ref = parm.pt_ref = false; + if (TREE_CODE (actual_type) == REFERENCE_TYPE) { /* If the user passes by reference, then we will save the pointer to the original. As noted in @@ -3782,16 +3768,12 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) referenced item ends and then the coroutine is resumed, we have UB; well, the user asked for it. */ actual_type = build_pointer_type (TREE_TYPE (actual_type)); - parm.frame_type = actual_type; parm.pt_ref = true; } else if (TYPE_REF_P (DECL_ARG_TYPE (arg))) - { - parm.by_ref = true; - parm.frame_type = actual_type; - } - else - parm.frame_type = actual_type; + parm.by_ref = true; + + parm.frame_type = actual_type; parm.this_ptr = is_this_parameter (arg); /* See PR94807. When a lambda is in a template instantiation, the @@ -4175,17 +4157,16 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) } else if (parm.by_ref) vec_safe_push (promise_args, fld_idx); - else if (parm.rv_ref) - vec_safe_push (promise_args, rvalue (fld_idx)); else vec_safe_push (promise_args, arg); if (TYPE_NEEDS_CONSTRUCTING (parm.frame_type)) { vec *p_in; - if (parm.by_ref - && classtype_has_non_deleted_move_ctor (parm.frame_type) - && !classtype_has_non_deleted_copy_ctor (parm.frame_type)) + if (CLASS_TYPE_P (parm.frame_type) + && classtype_has_non_deleted_move_ctor (parm.frame_type)) + p_in = make_tree_vector_single (move (arg)); + else if (lvalue_p (arg)) p_in = make_tree_vector_single (rvalue (arg)); else p_in = make_tree_vector_single (arg); @@ -4198,9 +4179,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) } else { - if (parm.rv_ref) - r = convert_from_reference (arg); - else if (!same_type_p (parm.frame_type, DECL_ARG_TYPE (arg))) + if (!same_type_p (parm.frame_type, DECL_ARG_TYPE (arg))) r = build1_loc (DECL_SOURCE_LOCATION (arg), CONVERT_EXPR, parm.frame_type, arg); else diff --git a/gcc/testsuite/g++.dg/coroutines/pr95350.C b/gcc/testsuite/g++.dg/coroutines/pr95350.C new file mode 100644 index 00000000000..1915032c471 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr95350.C @@ -0,0 +1,28 @@ +#if __has_include() +#include +#else +#include +namespace std { using namespace experimental; } +#endif +#include + +struct task { + struct promise_type { + task get_return_object(); + void return_void(); + void unhandled_exception(); + std::suspend_always initial_suspend() noexcept; + std::suspend_always final_suspend() noexcept; + }; +}; + +struct move_only { + move_only(); + move_only(const move_only&) = delete; + move_only(move_only&) = delete; + move_only(move_only&&) = default; +}; + +task f(move_only x) { + co_return; +} diff --git a/gcc/testsuite/g++.dg/coroutines/torture/func-params-08.C b/gcc/testsuite/g++.dg/coroutines/torture/func-params-08.C index c166745e052..cce1521b226 100644 --- a/gcc/testsuite/g++.dg/coroutines/torture/func-params-08.C +++ b/gcc/testsuite/g++.dg/coroutines/torture/func-params-08.C @@ -69,8 +69,9 @@ my_coro (Foo t_lv, Foo& t_ref, Foo&& t_rv_ref) PRINT ("my_coro 1"); sum += co_await t_ref; PRINT ("my_coro 2"); - sum += co_await t_rv_ref; - PRINT ("my_coro 3"); + // This can't work for the rvalue ref, it's always dangling. + //sum += co_await t_rv_ref; + //PRINT ("my_coro 3"); co_return sum; } @@ -90,17 +91,17 @@ int main () // now do the three co_awaits. while(!x.handle.done()) x.handle.resume(); - PRINT ("main: after resuming 3 co_awaits"); + PRINT ("main: after resuming 2 co_awaits"); /* Now we should have the co_returned value. */ int y = x.handle.promise().get_value(); - if (y != 14) + if (y != 10) { PRINTF ("main: wrong result (%d).", y); abort (); } - if (regular != 3 || copy != 1 || move != 1) + if (regular != 3 || copy != 0 || move != 1) { PRINTF ("main: unexpected ctor use (R:%d, C:%d, M:%d)\n", regular, copy, move); diff --git a/gcc/testsuite/g++.dg/coroutines/torture/func-params-09-awaitable-parms.C b/gcc/testsuite/g++.dg/coroutines/torture/func-params-09-awaitable-parms.C index 7d376b91f13..81430bf4d54 100644 --- a/gcc/testsuite/g++.dg/coroutines/torture/func-params-09-awaitable-parms.C +++ b/gcc/testsuite/g++.dg/coroutines/torture/func-params-09-awaitable-parms.C @@ -62,8 +62,9 @@ my_coro (FooAwaitable t_lv, FooAwaitable& t_ref, FooAwaitable&& t_rv_ref) PRINT ("my_coro 1"); sum += co_await t_ref; PRINT ("my_coro 2"); - sum += co_await t_rv_ref; - PRINT ("my_coro 3"); + // This can't work for the rvalue ref, it's always dangling. + //sum += co_await t_rv_ref; + //PRINT ("my_coro 3"); co_return sum; } @@ -83,17 +84,17 @@ int main () // now do the three co_awaits. while(!x.handle.done()) x.handle.resume(); - PRINT ("main: after resuming 3 co_awaits"); + PRINT ("main: after resuming 2 co_awaits"); /* Now we should have the co_returned value. */ int y = x.handle.promise().get_value(); - if (y != 14) + if (y != 10) { PRINTF ("main: wrong result (%d).", y); abort (); } - if (regular != 3 || copy != 1 || move != 1) + if (regular != 3 || copy != 0 || move != 1) { PRINTF ("main: unexpected ctor use (R:%d, C:%d, M:%d)\n", regular, copy, move);