From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp001-out.apm-internet.net (smtp001-out.apm-internet.net [85.119.248.222]) by sourceware.org (Postfix) with ESMTPS id AA90A3857361 for ; Tue, 11 Oct 2022 22:17:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AA90A3857361 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=sandoe.co.uk Received: (qmail 32343 invoked from network); 11 Oct 2022 22:17:42 -0000 X-APM-Out-ID: 16655266623233 X-APM-Authkey: 257869/1(257869/1) 8 Received: from unknown (HELO ?192.168.1.95?) (81.138.1.83) by smtp001.apm-internet.net with SMTP; 11 Oct 2022 22:17:42 -0000 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.7\)) Subject: Re: [PATCH] coroutines: Use cp_build_init_expr consistently. From: Iain Sandoe In-Reply-To: <44bcfe36-d570-c8d4-7980-4e8b2a2aac60@redhat.com> Date: Tue, 11 Oct 2022 23:17:41 +0100 Cc: GCC Patches Content-Transfer-Encoding: quoted-printable Message-Id: <9403FB3B-D300-4F62-B5D8-8BA83BB66F42@sandoe.co.uk> References: <20221011215831.67154-1-iain@sandoe.co.uk> <44bcfe36-d570-c8d4-7980-4e8b2a2aac60@redhat.com> To: Jason Merrill X-Mailer: Apple Mail (2.3608.120.23.2.7) X-Spam-Status: No, score=-14.3 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_COUK,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Jason > On 11 Oct 2022, at 23:06, Jason Merrill wrote: >=20 > On 10/11/22 17:58, Iain Sandoe wrote: >> Tested on x86_64-darwin19, OK for master? >> thanks >> Iain >> -- >8 -- >> Now we have the TARGET_EXPR_ELIDING_P flag, it is important to ensure = it >> is set properly on target exprs. The code here has a mixture of APIs = used >> to build inits. This patch changes that to use cp_build_init_expr() = where >> possible, since that handles setting the flag. >=20 > Hmm, I wouldn't expect this to be necessary, since = cp_build_modify_expr calls cp_build_init_expr for INIT_EXPR. It seems that path is only taken if the init is a CONSTRUCTOR. > Perhaps the similarity of the function names is a trap... not sure exactly what trap you mean. It seems that on my local set of additional tests (for some of the open = prs) there are some progressions from this change, I will have to track down which change is = the cause. thanks Iain >=20 >> --- >> gcc/cp/coroutines.cc | 25 ++++++++----------------- >> 1 file changed, 8 insertions(+), 17 deletions(-) >> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc >> index 01a3e831ee5..20d309445eb 100644 >> --- a/gcc/cp/coroutines.cc >> +++ b/gcc/cp/coroutines.cc >> @@ -1025,8 +1025,7 @@ build_co_await (location_t loc, tree a, = suspend_point_kind suspend_kind) >> else >> { >> e_proxy =3D get_awaitable_var (suspend_kind, o_type); >> - o =3D cp_build_modify_expr (loc, e_proxy, INIT_EXPR, o, >> - tf_warning_or_error); >> + o =3D cp_build_init_expr (loc, e_proxy, o); >> } >> /* I suppose we could check that this is contextually = convertible to bool. */ >> @@ -2889,8 +2888,7 @@ flatten_await_stmt (var_nest_node *n, = hash_set *promoted, >> gcc_checking_assert (!already_present); >> tree inner =3D TREE_OPERAND (init, 1); >> gcc_checking_assert (TREE_CODE (inner) !=3D COND_EXPR); >> - init =3D cp_build_modify_expr (input_location, var, INIT_EXPR, = init, >> - tf_warning_or_error); >> + init =3D cp_build_init_expr (var, init); >> /* Simplify for the case that we have an init containing the = temp >> alone. */ >> if (t =3D=3D n->init && n->var =3D=3D NULL_TREE) >> @@ -3656,8 +3654,7 @@ await_statement_walker (tree *stmt, int = *do_subtree, void *d) >> if (TREE_CODE (cond_inner) =3D=3D CLEANUP_POINT_EXPR) >> cond_inner =3D TREE_OPERAND (cond_inner, 0); >> location_t sloc =3D EXPR_LOCATION (SWITCH_STMT_COND = (sw_stmt)); >> - tree new_s =3D cp_build_init_expr (sloc, newvar, >> - cond_inner); >> + tree new_s =3D cp_build_init_expr (sloc, newvar, = cond_inner); >> finish_expr_stmt (new_s); >> SWITCH_STMT_COND (sw_stmt) =3D newvar; >> /* Now add the switch statement with the condition re- >> @@ -4902,9 +4899,8 @@ morph_fn_to_coro (tree orig, tree *resumer, = tree *destroyer) >> if (flag_exceptions) >> { >> /* This var is now live. */ >> - r =3D build_modify_expr (fn_start, parm.guard_var, >> - boolean_type_node, INIT_EXPR, = fn_start, >> - boolean_true_node, = boolean_type_node); >> + r =3D cp_build_init_expr (fn_start, parm.guard_var, >> + boolean_true_node); >> finish_expr_stmt (r); >> } >> } >> @@ -4948,9 +4944,7 @@ morph_fn_to_coro (tree orig, tree *resumer, = tree *destroyer) >> r =3D coro_build_cvt_void_expr_stmt (r, fn_start); >> finish_expr_stmt (r); >> - r =3D build_modify_expr (fn_start, coro_promise_live, = boolean_type_node, >> - INIT_EXPR, fn_start, boolean_true_node, >> - boolean_type_node); >> + r =3D cp_build_init_expr (fn_start, coro_promise_live, = boolean_true_node); >> finish_expr_stmt (r); >> promise_dtor >> @@ -5031,8 +5025,7 @@ morph_fn_to_coro (tree orig, tree *resumer, = tree *destroyer) >> NULL_TREE); >> add_decl_expr (gro); >> gro_bind_vars =3D gro; >> - r =3D cp_build_modify_expr (input_location, gro, INIT_EXPR, = get_ro, >> - tf_warning_or_error); >> + r =3D cp_build_init_expr (gro, get_ro); >> /* The constructed object might require a cleanup. */ >> if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type)) >> { >> @@ -5053,9 +5046,7 @@ morph_fn_to_coro (tree orig, tree *resumer, = tree *destroyer) >> cleanup. */ >> if (gro_ret_dtor) >> { >> - r =3D build_modify_expr (fn_start, coro_gro_live, = boolean_type_node, >> - INIT_EXPR, fn_start, boolean_true_node, >> - boolean_type_node); >> + r =3D cp_build_init_expr (fn_start, coro_gro_live, = boolean_true_node); >> finish_expr_stmt (r); >> } >> /* Initialize the resume_idx_var to 0, meaning "not started". */ >=20