| Message ID | 20180314223346.GN8577@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410) | expand |
On Wed, Mar 14, 2018 at 6:33 PM, Jakub Jelinek <jakub@redhat.com> wrote: > The following patch is an attempt to fix PLACEHOLDER_EXPR handling. > As e.g. > struct Y > { > static Y bar (Y y) { return y; } > int i; > int n = bar (Y{2,i}).m + bar {Y{2,i,i}).n; > int m = i; > }; > is rejected - one can't use incomplete ctors which would need NSDMIs > until the class is defined, I believe PLACEHOLDER_EXPRs can't be arbitrarily > intermixed, rather some CONSTRUCTORs into which we've added the NSDMI > expressions can contain PLACEHOLDER_EXPRs and other PLACEHOLDER_EXPRs can be > only found inside such CONSTRUCTORs added earlier. > > So, this patch adds a new flag on CONSTRUCTORs, which acts as a barrier > boundary for replace_placeholders, where we replace just PLACEHOLDER_EXPRs > from the outermost expressions and don't dive into CONSTRUCTORs into which > we've added other PLACEHOLDER_EXPRs - those will be handled later on > separately, by cp_gimplify_init_expr calling replace_placeholders. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-03-14 Jakub Jelinek <jakub@redhat.com> > > PR c++/79937 > PR c++/82410 > * 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. > > * 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/cp/cp-tree.h.jj 2018-03-14 16:21:50.745925263 +0100 > +++ gcc/cp/cp-tree.h 2018-03-14 15:57:51.171323825 +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-14 11:49:58.926816421 +0100 > +++ gcc/cp/tree.c 2018-03-14 16:24:29.036987505 +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,70 @@ 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. */ > + while (TREE_CODE (exp) == INDIRECT_REF) > + { > + tree t = TREE_OPERAND (exp, 0); > + STRIP_NOPS (t); > + if (TREE_CODE (t) == ADDR_EXPR > + && (t = TREE_OPERAND (t, 0)) > + && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (exp), > + TREE_TYPE (t))) > + exp = t; > + else > + break; > + } What's the motivation for this? I suspect that it should only apply if REFERENCE_REF_P. Jason
On Wed, Mar 14, 2018 at 08:55:47PM -0400, Jason Merrill wrote: > > @@ -3192,16 +3198,70 @@ 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. */ > > + while (TREE_CODE (exp) == INDIRECT_REF) > > + { > > + tree t = TREE_OPERAND (exp, 0); > > + STRIP_NOPS (t); > > + if (TREE_CODE (t) == ADDR_EXPR > > + && (t = TREE_OPERAND (t, 0)) > > + && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (exp), > > + TREE_TYPE (t))) > > + exp = t; > > + else > > + break; > > + } > > What's the motivation for this? g++.dg/cpp0x/nsdmi13.C ICEs without that, we have there: a = A({}); and build_over_call does: 8163 else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (as_base))) 8164 { 8165 arg = cp_build_fold_indirect_ref (arg); 8166 val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); 8167 /* Handle NSDMI that refer to the object being initialized. */ 8168 replace_placeholders (arg, to); 8169 } where arg is after the cp_build_fold_indirect_ref *(struct A &) &TARGET_EXPR <D.2403, {.p=&<PLACEHOLDER_EXPR struct A>}> This is in MODIFY_EXPR rather than INIT_EXPR and the gimplifier through gimple_fold_indirect_ref_rhs folds the *(struct A &) & away and so there is no further temporary and thus cp_gimplify_init_expr isn't called to replace_placeholders, so if we don't replace it here (with to being the a VAR_DECL), we don't replace it ever. > I suspect that it should only apply if REFERENCE_REF_P. Incremental --- gcc/cp/tree.c~ 2018-03-15 09:13:38.000000000 +0100 +++ gcc/cp/tree.c 2018-03-15 09:15:43.029087739 +0100 @@ -3199,7 +3199,7 @@ replace_placeholders (tree exp, tree obj tree *tp = &exp; /* Use exp instead of *(type *)&exp. */ - while (TREE_CODE (exp) == INDIRECT_REF) + while (REFERENCE_REF_P (exp)) { tree t = TREE_OPERAND (exp, 0); STRIP_NOPS (t); works too. I think the gimplifier would gimple_fold_indirect_ref_rhs away even *(A *) &TARGET_EXPR, but maybe that just can't happen in C++. The point in the replace_placeholders data.exp computation is to differentiate between binding a CONSTRUCTOR to a particular object and e.g. the C c = bar (X {1}); case from pr79937-1.C where it is just an expression that contains a CONSTRUCTOR in it. In the former case, e.g. the nsdmi13.C, we want to replace placeholders in it, while in the latter we don't; in both cases there is just one CONSTRUCTOR, so say ignoring the new flag on outermost CONSTRUCTOR we process doesn't work. Jakub
On Thu, Mar 15, 2018 at 4:32 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Mar 14, 2018 at 08:55:47PM -0400, Jason Merrill wrote: >> > @@ -3192,16 +3198,70 @@ 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. */ >> > + while (TREE_CODE (exp) == INDIRECT_REF) >> > + { >> > + tree t = TREE_OPERAND (exp, 0); >> > + STRIP_NOPS (t); >> > + if (TREE_CODE (t) == ADDR_EXPR >> > + && (t = TREE_OPERAND (t, 0)) >> > + && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (exp), >> > + TREE_TYPE (t))) >> > + exp = t; >> > + else >> > + break; >> > + } >> >> What's the motivation for this? > > g++.dg/cpp0x/nsdmi13.C ICEs without that, we have there: > a = A({}); > and build_over_call does: > 8163 else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (as_base))) > 8164 { > 8165 arg = cp_build_fold_indirect_ref (arg); > 8166 val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); > 8167 /* Handle NSDMI that refer to the object being initialized. */ > 8168 replace_placeholders (arg, to); > 8169 } > where arg is after the cp_build_fold_indirect_ref > *(struct A &) &TARGET_EXPR <D.2403, {.p=&<PLACEHOLDER_EXPR struct A>}> > This is in MODIFY_EXPR rather than INIT_EXPR and the gimplifier through > gimple_fold_indirect_ref_rhs folds the *(struct A &) & away and so there is > no further temporary and thus cp_gimplify_init_expr isn't called to > replace_placeholders, so if we don't replace it here (with to being the a > VAR_DECL), we don't replace it ever. Ah. That's a problem: the language says there's a temporary, so after the assignment a.p should not point to a. So the existing call to replace_placeholders in build_over_call is wrong. Seems like if the initializer for a TARGET_EXPR involves PLACEHOLDER_EXPR, the gimple_fold_modify_expr_rhs treatment of TARGET_EXPR isn't safe for MODIFY_EXPR, for the same reason that cp_gimplify_expr treats INIT_EXPR and MODIFY_EXPR differently. Jason
On Thu, Mar 15, 2018 at 01:15:53PM -0400, Jason Merrill wrote: > > g++.dg/cpp0x/nsdmi13.C ICEs without that, we have there: > > a = A({}); > > and build_over_call does: > > 8163 else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (as_base))) > > 8164 { > > 8165 arg = cp_build_fold_indirect_ref (arg); > > 8166 val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); > > 8167 /* Handle NSDMI that refer to the object being initialized. */ > > 8168 replace_placeholders (arg, to); > > 8169 } > > where arg is after the cp_build_fold_indirect_ref > > *(struct A &) &TARGET_EXPR <D.2403, {.p=&<PLACEHOLDER_EXPR struct A>}> > > This is in MODIFY_EXPR rather than INIT_EXPR and the gimplifier through > > gimple_fold_indirect_ref_rhs folds the *(struct A &) & away and so there is > > no further temporary and thus cp_gimplify_init_expr isn't called to > > replace_placeholders, so if we don't replace it here (with to being the a > > VAR_DECL), we don't replace it ever. > > Ah. That's a problem: the language says there's a temporary, so after > the assignment a.p should not point to a. Seems the gimplify.c stuff: case INDIRECT_REF: { /* If we have code like *(const A*)(A*)&x where the type of "x" is a (possibly cv-qualified variant of "A"), treat the entire expression as identical to "x". This kind of code arises in C++ when an object is bound to a const reference, and if "x" is a TARGET_EXPR we want to take advantage of the optimization below. */ ... has been added in r92539 as part of PR16405 fix. So, do we want to stop doing that unconditionally if t is a TARGET_EXPR, or for selected kinds of 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. > So the existing call to replace_placeholders in build_over_call is wrong. Shall it be just dropped if we tweak the gimplifier somehow? > Seems like if the initializer for a TARGET_EXPR involves > PLACEHOLDER_EXPR, the gimple_fold_modify_expr_rhs treatment of > TARGET_EXPR isn't safe for MODIFY_EXPR, for the same reason that > cp_gimplify_expr treats INIT_EXPR and MODIFY_EXPR differently. Jakub
On Thu, Mar 15, 2018 at 1:59 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Mar 15, 2018 at 01:15:53PM -0400, Jason Merrill wrote: >> > g++.dg/cpp0x/nsdmi13.C ICEs without that, we have there: >> > a = A({}); >> > and build_over_call does: >> > 8163 else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (as_base))) >> > 8164 { >> > 8165 arg = cp_build_fold_indirect_ref (arg); >> > 8166 val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); >> > 8167 /* Handle NSDMI that refer to the object being initialized. */ >> > 8168 replace_placeholders (arg, to); >> > 8169 } >> > where arg is after the cp_build_fold_indirect_ref >> > *(struct A &) &TARGET_EXPR <D.2403, {.p=&<PLACEHOLDER_EXPR struct A>}> >> > This is in MODIFY_EXPR rather than INIT_EXPR and the gimplifier through >> > gimple_fold_indirect_ref_rhs folds the *(struct A &) & away and so there is >> > no further temporary and thus cp_gimplify_init_expr isn't called to >> > replace_placeholders, so if we don't replace it here (with to being the a >> > VAR_DECL), we don't replace it ever. >> >> Ah. That's a problem: the language says there's a temporary, so after >> the assignment a.p should not point to a. > > Seems the gimplify.c stuff: > case INDIRECT_REF: > { > /* If we have code like > > *(const A*)(A*)&x > > where the type of "x" is a (possibly cv-qualified variant > of "A"), treat the entire expression as identical to "x". > This kind of code arises in C++ when an object is bound > to a const reference, and if "x" is a TARGET_EXPR we want > to take advantage of the optimization below. */ > ... > has been added in r92539 as part of PR16405 fix. So, do we want to stop > doing that unconditionally if t is a TARGET_EXPR, or for selected kinds of Folding away the INDIRECT_REF is fine. It's the TARGET_EXPR handling that is wrong. > 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 the existing call to replace_placeholders in build_over_call is wrong. > > Shall it be just dropped if we tweak the gimplifier somehow? It must be dropped. >> Seems like if the initializer for a TARGET_EXPR involves >> PLACEHOLDER_EXPR, the gimple_fold_modify_expr_rhs treatment of >> TARGET_EXPR isn't safe for MODIFY_EXPR, for the same reason that >> cp_gimplify_expr treats INIT_EXPR and MODIFY_EXPR differently.
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 <jakub@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
On Thu, Mar 15, 2018 at 4:28 PM, Jakub Jelinek <jakub@redhat.com> 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 <jakub@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. > > +/* 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
On Thu, Mar 15, 2018 at 04:50:57PM -0400, Jason Merrill wrote: > > +/* 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. Ah, ok, changed in the patch. > > @@ -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). The following (except the *t = unshare_expr (x) change) has successfully bootstrapped/regtested on x86_64-linux and i686-linux (without the unshare_expr it regressed g++.dg/cpp0x/pr83556.C, but with that change it succeeds; I'm afraid right now there is no easy way to determine if we need to unshare or not, if replace_placeholders is called before unshare_body at the start of gimplification, we don't need to unshare, but during gimplification we have to). Ok for trunk? 2018-03-15 Jakub Jelinek <jakub@redhat.com> PR c++/79937 PR c++/82410 * tree.h (TARGET_EXPR_NO_ELIDE): Define. * gimplify.c (gimplify_modify_expr_rhs): Don't elide TARGET_EXPRs with TARGET_EXPR_NO_ELIDE flag set unless *expr_p is INIT_EXPR. * 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 PLACEHOLDER_EXPR with unshare_expr (x) rather than x. (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,9 @@ 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 + on rhs of MODIFY_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 @@ -5211,6 +5211,8 @@ gimplify_modify_expr_rhs (tree *expr_p, tree init = TARGET_EXPR_INITIAL (*from_p); if (init + && (TREE_CODE (*expr_p) != MODIFY_EXPR + || !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. */ }; @@ -3124,7 +3125,7 @@ replace_placeholders_r (tree* t, int* wa TREE_TYPE (x)); x = TREE_OPERAND (x, 0)) gcc_assert (TREE_CODE (x) == COMPONENT_REF); - *t = x; + *t = unshare_expr (x); *walk_subtrees = false; d->seen = true; } @@ -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
On Thu, Mar 15, 2018 at 7:33 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Mar 15, 2018 at 04:50:57PM -0400, Jason Merrill wrote: >> > +/* 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. > > Ah, ok, changed in the patch. > >> > @@ -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). > > The following (except the *t = unshare_expr (x) change) has successfully > bootstrapped/regtested on x86_64-linux and i686-linux (without the > unshare_expr it regressed g++.dg/cpp0x/pr83556.C, but with that change it > succeeds; I'm afraid right now there is no easy way to determine if we > need to unshare or not, if replace_placeholders is called before > unshare_body at the start of gimplification, we don't need to unshare, but > during gimplification we have to). Ok for trunk? > 2018-03-15 Jakub Jelinek <jakub@redhat.com> > > PR c++/79937 > PR c++/82410 > * tree.h (TARGET_EXPR_NO_ELIDE): Define. > * gimplify.c (gimplify_modify_expr_rhs): Don't elide TARGET_EXPRs with > TARGET_EXPR_NO_ELIDE flag set unless *expr_p is INIT_EXPR. > > * 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 PLACEHOLDER_EXPR with unshare_expr (x) rather than x. > (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,9 @@ 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 > + on rhs of MODIFY_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 > @@ -5211,6 +5211,8 @@ gimplify_modify_expr_rhs (tree *expr_p, > tree init = TARGET_EXPR_INITIAL (*from_p); > > if (init > + && (TREE_CODE (*expr_p) != MODIFY_EXPR > + || !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. */ > }; > @@ -3124,7 +3125,7 @@ replace_placeholders_r (tree* t, int* wa > TREE_TYPE (x)); > x = TREE_OPERAND (x, 0)) > gcc_assert (TREE_CODE (x) == COMPONENT_REF); > - *t = x; > + *t = unshare_expr (x); > *walk_subtrees = false; > d->seen = true; > } > @@ -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. */ This comment should have been removed with the code it described. OK with that change. > 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
--- gcc/cp/cp-tree.h.jj 2018-03-14 16:21:50.745925263 +0100 +++ gcc/cp/cp-tree.h 2018-03-14 15:57:51.171323825 +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-14 11:49:58.926816421 +0100 +++ gcc/cp/tree.c 2018-03-14 16:24:29.036987505 +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,70 @@ 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. */ + while (TREE_CODE (exp) == INDIRECT_REF) + { + tree t = TREE_OPERAND (exp, 0); + STRIP_NOPS (t); + if (TREE_CODE (t) == ADDR_EXPR + && (t = TREE_OPERAND (t, 0)) + && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (exp), + TREE_TYPE (t))) + exp = t; + else + break; + } 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-14 16:25:49.622019191 +0100 +++ gcc/cp/typeck2.c 2018-03-14 16:02:36.093441530 +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/testsuite/g++.dg/cpp1y/pr79937-1.C.jj 2018-03-14 14:32:12.230097853 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr79937-1.C 2018-03-14 14:32:12.230097853 +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-14 14:32:12.231097853 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr79937-2.C 2018-03-14 14:32:12.231097853 +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-14 14:32:12.231097853 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr79937-3.C 2018-03-14 14:32:12.231097853 +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-14 16:14:50.842757308 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr79937-4.C 2018-03-14 16:14:45.761755127 +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-14 16:20:07.180884544 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr82410.C 2018-03-14 16:20:01.626882356 +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}); +}
Hi! The following patch is an attempt to fix PLACEHOLDER_EXPR handling. As e.g. struct Y { static Y bar (Y y) { return y; } int i; int n = bar (Y{2,i}).m + bar {Y{2,i,i}).n; int m = i; }; is rejected - one can't use incomplete ctors which would need NSDMIs until the class is defined, I believe PLACEHOLDER_EXPRs can't be arbitrarily intermixed, rather some CONSTRUCTORs into which we've added the NSDMI expressions can contain PLACEHOLDER_EXPRs and other PLACEHOLDER_EXPRs can be only found inside such CONSTRUCTORs added earlier. So, this patch adds a new flag on CONSTRUCTORs, which acts as a barrier boundary for replace_placeholders, where we replace just PLACEHOLDER_EXPRs from the outermost expressions and don't dive into CONSTRUCTORs into which we've added other PLACEHOLDER_EXPRs - those will be handled later on separately, by cp_gimplify_init_expr calling replace_placeholders. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-03-14 Jakub Jelinek <jakub@redhat.com> PR c++/79937 PR c++/82410 * 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. * 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. Jakub