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 <ja...@redhat.com> 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. --- gcc/tree.h.jj 2018-02-22 12:37:02.566387732 +0100 +++ gcc/tree.h 2018-03-15 20:42:57.968858551 +0100 @@ -1197,6 +1197,8 @@ extern tree maybe_wrap_with_location (tr #define TARGET_EXPR_SLOT(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 0) #define TARGET_EXPR_INITIAL(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 1) #define TARGET_EXPR_CLEANUP(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 2) +/* 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) /* DECL_EXPR accessor. This gives access to the DECL associated with the given declaration statement. */ --- gcc/gimplify.c.jj 2018-01-18 21:28:49.743301440 +0100 +++ gcc/gimplify.c 2018-03-15 21:06:15.290828320 +0100 @@ -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)) { tree init = TARGET_EXPR_INITIAL (*arg_p); if (init @@ -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) && !VOID_TYPE_P (TREE_TYPE (init))) { *from_p = init; --- gcc/cp/cp-tree.h.jj 2018-03-15 18:44:21.646300907 +0100 +++ gcc/cp/cp-tree.h 2018-03-15 20:40:49.280818226 +0100 @@ -425,6 +425,7 @@ extern GTY(()) tree cp_global_trees[CPTI DECL_VTABLE_OR_VTT_P (in VAR_DECL) FUNCTION_RVALUE_QUALIFIED (in FUNCTION_TYPE, METHOD_TYPE) CALL_EXPR_REVERSE_ARGS (in CALL_EXPR, AGGR_INIT_EXPR) + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (in CONSTRUCTOR) 6: IDENTIFIER_REPO_CHOSEN (in IDENTIFIER_NODE) DECL_CONSTRUCTION_VTABLE_P (in VAR_DECL) TYPE_MARKED_P (in _TYPE) @@ -4144,6 +4145,12 @@ more_aggr_init_expr_args_p (const aggr_i #define CONSTRUCTOR_C99_COMPOUND_LITERAL(NODE) \ (TREE_LANG_FLAG_3 (CONSTRUCTOR_CHECK (NODE))) +/* True if this CONSTRUCTOR contains PLACEHOLDER_EXPRs referencing the + CONSTRUCTOR's type not nested inside another CONSTRUCTOR marked with + CONSTRUCTOR_PLACEHOLDER_BOUNDARY. */ +#define CONSTRUCTOR_PLACEHOLDER_BOUNDARY(NODE) \ + (TREE_LANG_FLAG_5 (CONSTRUCTOR_CHECK (NODE))) + #define DIRECT_LIST_INIT_P(NODE) \ (BRACE_ENCLOSED_INITIALIZER_P (NODE) && CONSTRUCTOR_IS_DIRECT_INIT (NODE)) @@ -7021,6 +7028,7 @@ extern tree array_type_nelts_top (tree) extern tree break_out_target_exprs (tree); extern tree build_ctor_subob_ref (tree, tree, tree); extern tree replace_placeholders (tree, tree, bool * = NULL); +extern bool find_placeholders (tree); extern tree get_type_decl (tree); extern tree decl_namespace_context (tree); extern bool decl_anon_ns_mem_p (const_tree); --- gcc/cp/tree.c.jj 2018-03-15 18:44:21.661300889 +0100 +++ gcc/cp/tree.c 2018-03-15 20:49:03.518973106 +0100 @@ -3096,6 +3096,7 @@ build_ctor_subob_ref (tree index, tree t struct replace_placeholders_t { tree obj; /* The object to be substituted for a PLACEHOLDER_EXPR. */ + tree exp; /* The outermost exp. */ bool seen; /* Whether we've encountered a PLACEHOLDER_EXPR. */ hash_set<tree> *pset; /* To avoid walking same trees multiple times. */ }; @@ -3134,7 +3135,12 @@ replace_placeholders_r (tree* t, int* wa { constructor_elt *ce; vec<constructor_elt,va_gc> *v = CONSTRUCTOR_ELTS (*t); - if (d->pset->add (*t)) + /* Don't walk into CONSTRUCTOR_PLACEHOLDER_BOUNDARY ctors + other than the d->exp one, those have PLACEHOLDER_EXPRs + related to another object. */ + if ((CONSTRUCTOR_PLACEHOLDER_BOUNDARY (*t) + && *t != d->exp) + || d->pset->add (*t)) { *walk_subtrees = false; return NULL_TREE; @@ -3192,16 +3198,58 @@ replace_placeholders (tree exp, tree obj return exp; tree *tp = &exp; - hash_set<tree> pset; - replace_placeholders_t data = { obj, false, &pset }; + /* Use exp instead of *(type *)&exp. */ if (TREE_CODE (exp) == TARGET_EXPR) tp = &TARGET_EXPR_INITIAL (exp); + hash_set<tree> pset; + replace_placeholders_t data = { obj, *tp, false, &pset }; cp_walk_tree (tp, replace_placeholders_r, &data, NULL); if (seen_p) *seen_p = data.seen; return exp; } +/* Callback function for find_placeholders. */ + +static tree +find_placeholders_r (tree *t, int *walk_subtrees, void *) +{ + if (TYPE_P (*t) || TREE_CONSTANT (*t)) + { + *walk_subtrees = false; + return NULL_TREE; + } + + switch (TREE_CODE (*t)) + { + case PLACEHOLDER_EXPR: + return *t; + + case CONSTRUCTOR: + if (CONSTRUCTOR_PLACEHOLDER_BOUNDARY (*t)) + *walk_subtrees = false; + break; + + default: + break; + } + + return NULL_TREE; +} + +/* Return true if EXP contains a PLACEHOLDER_EXPR. Don't walk into + ctors with CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set. */ + +bool +find_placeholders (tree exp) +{ + /* This is only relevant for C++14. */ + if (cxx_dialect < cxx14) + return false; + + return cp_walk_tree_without_duplicates (&exp, find_placeholders_r, NULL); +} + /* Similar to `build_nt', but for template definitions of dependent expressions */ --- gcc/cp/typeck2.c.jj 2018-03-15 18:44:21.661300889 +0100 +++ gcc/cp/typeck2.c 2018-03-15 20:40:49.282818227 +0100 @@ -1470,6 +1470,9 @@ process_init_constructor_record (tree ty } /* C++14 aggregate NSDMI. */ next = get_nsdmi (field, /*ctor*/false, complain); + if (!CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) + && find_placeholders (next)) + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1; } else if (type_build_ctor_call (TREE_TYPE (field))) { @@ -1608,10 +1611,11 @@ process_init_constructor_union (tree typ if (TREE_CODE (field) == FIELD_DECL && DECL_INITIAL (field) != NULL_TREE) { - CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (init), - field, - get_nsdmi (field, /*in_ctor=*/false, - complain)); + tree val = get_nsdmi (field, /*in_ctor=*/false, complain); + if (!CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) + && find_placeholders (val)) + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1; + CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (init), field, val); break; } } --- gcc/cp/call.c.jj 2018-03-11 17:48:36.359061434 +0100 +++ gcc/cp/call.c 2018-03-15 21:07:31.781961783 +0100 @@ -8164,8 +8164,6 @@ build_over_call (struct z_candidate *can { arg = cp_build_fold_indirect_ref (arg); val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); - /* Handle NSDMI that refer to the object being initialized. */ - replace_placeholders (arg, to); } else { --- gcc/cp/cp-gimplify.c.jj 2018-03-07 22:51:58.742478684 +0100 +++ gcc/cp/cp-gimplify.c 2018-03-15 21:19:27.353619519 +0100 @@ -1515,6 +1515,13 @@ cp_genericize_r (tree *stmt_p, int *walk } break; + case TARGET_EXPR: + if (TARGET_EXPR_INITIAL (stmt) + && TREE_CODE (TARGET_EXPR_INITIAL (stmt)) == CONSTRUCTOR + && CONSTRUCTOR_PLACEHOLDER_BOUNDARY (TARGET_EXPR_INITIAL (stmt))) + TARGET_EXPR_NO_ELIDE (stmt) = 1; + break; + default: if (IS_TYPE_OR_DECL_P (stmt)) *walk_subtrees = 0; @@ -2469,7 +2476,11 @@ cp_fold (tree x) } } if (nelts) - x = build_constructor (TREE_TYPE (x), nelts); + { + x = build_constructor (TREE_TYPE (x), nelts); + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (x) + = CONSTRUCTOR_PLACEHOLDER_BOUNDARY (org_x); + } break; } case TREE_VEC: --- gcc/testsuite/g++.dg/cpp1y/pr79937-1.C.jj 2018-03-15 20:40:49.283818227 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr79937-1.C 2018-03-15 20:40:49.282818227 +0100 @@ -0,0 +1,23 @@ +// PR c++/79937 +// { dg-do run { target c++14 } } + +struct C {}; + +struct X { + unsigned i; + unsigned n = i; +}; + +C +bar (X x) +{ + if (x.i != 1 || x.n != 1) + __builtin_abort (); + return {}; +} + +int +main () +{ + C c = bar (X {1}); +} --- gcc/testsuite/g++.dg/cpp1y/pr79937-2.C.jj 2018-03-15 20:40:49.283818227 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr79937-2.C 2018-03-15 20:40:49.283818227 +0100 @@ -0,0 +1,24 @@ +// PR c++/79937 +// { dg-do run { target c++14 } } + +struct C {}; + +struct X { + unsigned i; + unsigned n = i; + unsigned m = i; +}; + +C +bar (X x) +{ + if (x.i != 1 || x.n != 2 || x.m != 1) + __builtin_abort (); + return {}; +} + +int +main () +{ + C c = bar (X {1, X {2}.n}); +} --- gcc/testsuite/g++.dg/cpp1y/pr79937-3.C.jj 2018-03-15 20:40:49.283818227 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr79937-3.C 2018-03-15 20:40:49.283818227 +0100 @@ -0,0 +1,24 @@ +// PR c++/79937 +// { dg-do run { target c++14 } } + +struct X { + unsigned i; + unsigned n = i; + unsigned m = i; +}; + +X +bar (X x) +{ + if (x.i != 1 || x.n != 2 || x.m != 1) + __builtin_abort (); + return x; +} + +int +main () +{ + X x = bar (X {1, X {2}.n}); + if (x.i != 1 || x.n != 2 || x.m != 1) + __builtin_abort (); +} --- gcc/testsuite/g++.dg/cpp1y/pr79937-4.C.jj 2018-03-15 20:40:49.283818227 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr79937-4.C 2018-03-15 20:40:49.283818227 +0100 @@ -0,0 +1,32 @@ +// PR c++/79937 +// { dg-do run { target c++14 } } + +struct X { + unsigned i; + unsigned n = i; +}; + +X +bar (X x) +{ + return x; +} + +struct Y +{ + static Y bar (Y y) { return y; } + unsigned i; + unsigned n = bar (Y{2,i}).n; +}; + +int +main () +{ + X x { 1, bar (X{2}).n }; + if (x.n != 2) + __builtin_abort (); + + Y y { 1 }; + if (y.n != 1) + __builtin_abort (); +} --- gcc/testsuite/g++.dg/cpp1y/pr82410.C.jj 2018-03-15 20:40:49.284818227 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr82410.C 2018-03-15 20:40:49.283818227 +0100 @@ -0,0 +1,16 @@ +// PR c++/82410 +// { dg-do compile { target c++14 } } + +int +main () +{ + struct A {}; + struct S + { + int & p; + int x = p; + operator A () { return {}; } + }; + int l; + [] (A) {} (S{l}); +} Jakub