Re: [PATCH] coroutines: Use cp_build_init_expr consistently.

[Iain Sandoe <iain@sandoe.co.uk>]

> On 12 Oct 2022, at 00:19, Jason Merrill <jason@redhat.com> wrote:
> 
> On 10/11/22 18:17, Iain Sandoe wrote:
>> Hi Jason
>>> On 11 Oct 2022, at 23:06, Jason Merrill <jason@redhat.com> wrote:
>>> 
>>> On 10/11/22 17:58, Iain Sandoe wrote:

>>>> 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.
>>> 
>>> Hmm, I wouldn't expect this to be necessary, since cp_build_modify_expr calls cp_build_init_expr for INIT_EXPR.

You are, of course, right..

>>>  Perhaps the similarity of the function names is a trap...
>> not sure exactly what trap you mean.
> 
> The names are close, but cp_build_modify_expr is a lot more complicated; it handles conversions and overloaded operators.

This is one of the tricky parts about synthesizing AST from scratch - knowing which API is the “right one” to use in each case - e.g. here:  cp_build_modify_expr or  cp_build_init_expr or even build2 (INIT_EXPR..

So, it seems that where there could be a mismatch to be diagnosed, I should use cp_build_modify_expr, but maybe in some of the cases where there is no checking needed (e.g. set a guard var to true) it would be appropriate to use one of the simpler forms.

>> It seems that on my local set of additional tests (for some of the open prs) there are some
>> progressions from this change,

That’s a spurious observation, because cp_build_init_expr does not do the diagnostics.

patch withdrawn.
thanks for the explanations,
Iain