From 14ed21f8749ae359690d9c4a69ca38cc45d0d1b0 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Tue, 4 May 2021 21:33:36 -0400 Subject: [PATCH] c++: don't call 'rvalue' in coroutines code A change to check glvalue_p rather than specifically for TARGET_EXPR revealed issues with the coroutines code's use of the 'rvalue' function, which shouldn't be used on class glvalues, so I've removed those calls. In build_co_await I just dropped them, because I don't see anything in the co_await specification that indicates that we would want to move from an lvalue result of operator co_await. And simplified that code while I was touching it; cp_build_modify_expr (...INIT_EXPR...) will call the constructor. In morph_fn_to_coro I changed the handling of the rvalue reference coroutine frame field to use move, to treat the rval ref as an xvalue. I used forward_parm to pass the function parms to the constructor for the field. And I simplified the return handling so we get the desired rvalue semantics from the normal implicit move on return. I question default-initializing the non-void return value of the function if get_return_object returns void; I'm not messing with it here, but I've filed PR100476 about it. gcc/cp/ChangeLog: * coroutines.cc (build_co_await): Don't call 'rvalue'. (flatten_await_stmt): Simplify initialization. (morph_fn_to_coro): Change 'rvalue' to 'move'. Simplify. gcc/testsuite/ChangeLog: * g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C: Adjust diagnostic. --- gcc/cp/coroutines.cc | 117 +++++------------- .../coro-bad-gro-00-class-gro-scalar-return.C | 2 +- 2 files changed, 31 insertions(+), 88 deletions(-) diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index dbd703a67cc8..71662061f5a9 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -950,18 +950,11 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) e_proxy = o; o = NULL_TREE; /* The var is already present. */ } - else if (type_build_ctor_call (o_type)) - { - e_proxy = get_awaitable_var (suspend_kind, o_type); - releasing_vec arg (make_tree_vector_single (rvalue (o))); - o = build_special_member_call (e_proxy, complete_ctor_identifier, - &arg, o_type, LOOKUP_NORMAL, - tf_warning_or_error); - } else { e_proxy = get_awaitable_var (suspend_kind, o_type); - o = build2 (INIT_EXPR, o_type, e_proxy, rvalue (o)); + o = cp_build_modify_expr (loc, e_proxy, INIT_EXPR, o, + tf_warning_or_error); } /* I suppose we could check that this is contextually convertible to bool. */ @@ -2989,15 +2982,8 @@ flatten_await_stmt (var_nest_node *n, hash_set *promoted, gcc_checking_assert (!already_present); tree inner = TREE_OPERAND (init, 1); gcc_checking_assert (TREE_CODE (inner) != COND_EXPR); - if (type_build_ctor_call (var_type)) - { - releasing_vec p_in (make_tree_vector_single (init)); - init = build_special_member_call (var, complete_ctor_identifier, - &p_in, var_type, LOOKUP_NORMAL, - tf_warning_or_error); - } - else - init = build2 (INIT_EXPR, var_type, var, init); + init = cp_build_modify_expr (input_location, var, INIT_EXPR, init, + tf_warning_or_error); /* Simplify for the case that we have an init containing the temp alone. */ if (t == n->init && n->var == NULL_TREE) @@ -4862,43 +4848,19 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) vec_safe_push (promise_args, this_ref); } else if (parm.rv_ref) - vec_safe_push (promise_args, rvalue(fld_idx)); + vec_safe_push (promise_args, move (fld_idx)); else vec_safe_push (promise_args, fld_idx); if (parm.rv_ref || parm.pt_ref) /* Initialise the frame reference field directly. */ - r = build_modify_expr (fn_start, TREE_OPERAND (fld_idx, 0), - parm.frame_type, INIT_EXPR, - DECL_SOURCE_LOCATION (arg), arg, - DECL_ARG_TYPE (arg)); - else if (type_build_ctor_call (parm.frame_type)) - { - vec *p_in; - 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); - /* Construct in place or move as relevant. */ - r = build_special_member_call (fld_idx, complete_ctor_identifier, - &p_in, parm.frame_type, - LOOKUP_NORMAL, - tf_warning_or_error); - release_tree_vector (p_in); - } + r = cp_build_modify_expr (fn_start, TREE_OPERAND (fld_idx, 0), + INIT_EXPR, arg, tf_warning_or_error); else { - 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 - r = arg; - r = build_modify_expr (fn_start, fld_idx, parm.frame_type, - INIT_EXPR, DECL_SOURCE_LOCATION (arg), r, - TREE_TYPE (r)); + r = forward_parm (arg); + r = cp_build_modify_expr (fn_start, fld_idx, INIT_EXPR, r, + tf_warning_or_error); } finish_expr_stmt (r); if (!parm.trivial_dtor) @@ -5044,16 +5006,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) DECL_IGNORED_P (gro) = true; add_decl_expr (gro); gro_bind_vars = gro; - if (type_build_ctor_call (gro_type)) - { - vec *arg = make_tree_vector_single (get_ro); - r = build_special_member_call (gro, 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, gro, get_ro); + r = cp_build_modify_expr (input_location, gro, INIT_EXPR, get_ro, + tf_warning_or_error); /* The constructed object might require a cleanup. */ if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type)) { @@ -5111,37 +5065,26 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) if (same_type_p (gro_type, fn_return_type)) r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig); + else if (!gro_is_void_p) + /* check_return_expr will automatically return gro as an rvalue via + treat_lvalue_as_rvalue_p. */ + r = gro; + else if (CLASS_TYPE_P (fn_return_type)) + { + /* For class type return objects, we can attempt to construct, + even if the gro is void. ??? Citation ??? c++/100476 */ + r = build_special_member_call (NULL_TREE, + complete_ctor_identifier, NULL, + fn_return_type, LOOKUP_NORMAL, + tf_warning_or_error); + r = build_cplus_new (fn_return_type, r, tf_warning_or_error); + } else { - if (CLASS_TYPE_P (fn_return_type)) - { - /* For class type return objects, we can attempt to construct, - even if the gro is void. */ - vec *args = NULL; - vec **arglist = NULL; - if (!gro_is_void_p) - { - args = make_tree_vector_single (rvalue (gro)); - arglist = &args; - } - r = build_special_member_call (NULL_TREE, - complete_ctor_identifier, arglist, - fn_return_type, LOOKUP_NORMAL, - tf_warning_or_error); - r = build_cplus_new (fn_return_type, r, tf_warning_or_error); - if (args) - release_tree_vector (args); - } - 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 %", fn_return_type); - r = error_mark_node; - } - else - r = build1_loc (input_location, CONVERT_EXPR, - fn_return_type, rvalue (gro)); + /* 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 %", fn_return_type); + r = error_mark_node; } finish_return_stmt (r); diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C index f9e8a5f398bc..0512f03f4d0a 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C @@ -37,7 +37,7 @@ my_coro (std::coroutine_handle<>& h) { PRINT ("coro1: about to return"); co_return; -} // { dg-error {'struct Thing' used where a 'int' was expected} } +} // { dg-error {cannot convert 'Thing' to 'int' in return} } int main () { -- 2.43.5