On Thu, Mar 15, 2018 at 4:28 PM, Jakub Jelinek <[email protected]> wrote:
> On Thu, Mar 15, 2018 at 03:33:12PM -0400, Jason Merrill wrote:
>> Folding away the INDIRECT_REF is fine. It's the TARGET_EXPR handling
>> that is wrong.
>
> Ah, ok.
>
>> > types of TARGET_EXPR, or ask some langhook whether it is ok to do so
>> > (say not ok if find_placeholders (t))? Or contains_placeholder_p?
>> > Though the last one could also affect Ada and could return true even if
>> > the PLACEHOLDER_EXPRs are for some nested TARGET_EXPR in it.
>>
>> The existing test for whether to collapse a TARGET_EXPR is
>>
>> if (init
>> && !VOID_TYPE_P (TREE_TYPE (init)))
>>
>> so we need this test to fail somehow when the constructor contains
>> placeholders, either by adding an additional test (like the ones you
>> mention) or by making the initialization void in a way that other
>> gimplification knows how to handle.
>
> So what about this? It uses an unused bit on TARGET_EXPRs rather than
> a langhook, but if you prefer a langhook, I can add it.
>
> Tested so far just with
> make check-c++-all RUNTESTFLAGS="dg.exp='pr79937* pr82410.C nsdmi*'"
>
> 2018-03-15 Jakub Jelinek <[email protected]>
>
> PR c++/79937
> PR c++/82410
> * tree.h (TARGET_EXPR_NO_ELIDE): Define.
> * gimplify.c (gimplify_arg, gimplify_modify_expr_rhs): Don't elide
> TARGET_EXPRs with TARGET_EXPR_NO_ELIDE flag set.
>
> * cp-tree.h (CONSTRUCTOR_PLACEHOLDER_BOUNDARY): Define.
> (find_placeholder): Declare.
> * tree.c (struct replace_placeholders_t): Add exp member.
> (replace_placeholders_r): Don't walk into ctors with
> CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set, unless they are equal to
> d->exp.
> (replace_placeholders): Initialize data.exp.
> (find_placeholders_r, find_placeholders): New functions.
> * typeck2.c (process_init_constructor_record,
> process_init_constructor_union): Set CONSTRUCTOR_PLACEHOLDER_BOUNDARY
> if adding NSDMI on which find_placeholder returns true.
> * call.c (build_over_call): Don't call replace_placeholders here.
> * cp-gimplify.c (cp_genericize_r): Set TARGET_EXPR_NO_ELIDE on
> TARGET_EXPRs with CONSTRUCTOR_PLACEHOLDER_BOUNDARY set on
> TARGET_EXPR_INITIAL.
> (cp_fold): Copy over CONSTRUCTOR_PLACEHOLDER_BOUNDARY bit to new
> ctor.
>
> * g++.dg/cpp1y/pr79937-1.C: New test.
> * g++.dg/cpp1y/pr79937-2.C: New test.
> * g++.dg/cpp1y/pr79937-3.C: New test.
> * g++.dg/cpp1y/pr79937-4.C: New test.
> * g++.dg/cpp1y/pr82410.C: New test.
>
> +/* Don't elide the initialization of TARGET_EXPR_SLOT for this TARGET_EXPR.
> */
> +#define TARGET_EXPR_NO_ELIDE(NODE) (TARGET_EXPR_CHECK
> (NODE)->base.private_flag)
This should be specifically on the rhs of a MODIFY_EXPR; it's OK to
elide on the rhs of an INIT_EXPR.
> @@ -3155,7 +3155,8 @@ gimplify_arg (tree *arg_p, gimple_seq *p
> {
> test = is_gimple_lvalue, fb = fb_either;
> /* Also strip a TARGET_EXPR that would force an extra copy. */
> - if (TREE_CODE (*arg_p) == TARGET_EXPR)
> + if (TREE_CODE (*arg_p) == TARGET_EXPR
> + && !TARGET_EXPR_NO_ELIDE (*arg_p))
This is also an initialization context, so we don't need to check it here.
> @@ -5211,6 +5212,7 @@ gimplify_modify_expr_rhs (tree *expr_p,
> tree init = TARGET_EXPR_INITIAL (*from_p);
>
> if (init
> + && !TARGET_EXPR_NO_ELIDE (*from_p)
Here should check TREE_CODE (*expr_p).
Jason
