This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Jason Merrill <jason at redhat dot com>
- Cc: gcc-patches List <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 15 Mar 2018 21:28:08 +0100
- Subject: Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
- References: <20180314223346.GN8577@tucnak> <CADzB+2=4vdr9OocJSCNejyBLaRubK1L4ZhbA=LdYgUy-EhN2+Q@mail.gmail.com> <20180315083243.GP8577@tucnak> <CADzB+2mhu9aNApCRyPZ4ksMTMfeivqJcneUCdd3VuORcVh=qMg@mail.gmail.com> <20180315175900.GU8577@tucnak> <CADzB+2kOgLepQv6-K0GVSdyObNZ6Fkp+QkxvadTkqPVj2=G4TA@mail.gmail.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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