> On 12 Oct 2022, at 00:19, Jason Merrill <ja...@redhat.com> wrote:
>
> On 10/11/22 18:17, Iain Sandoe wrote:
>> Hi Jason
>>> On 11 Oct 2022, at 23:06, Jason Merrill <ja...@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
