Message ID | 20200413131843.1164597-1-ppalka@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: More self-modifying constexpr init [PR94470] | expand |
On 4/13/20 9:18 AM, Patrick Palka wrote: > In this PR we're incorrectly rejecting a self-modifying constexpr initializer as > a consequence of the fix for PR78572. > > It looks like however that the fix for PR78572 is obsoleted by the fix for > PR89336: the testcase from the former PR successfully compiles even with its fix > reverted. > > But then further testing showed that the analogous testcase of PR78572 where the > array has an aggregate element type is still problematic (i.e. we ICE) even with > the fix for PR78572 applied. The reason is that in cxx_eval_bare_aggregate we > attach a constructor_elt of aggregate type always to the end of the new > CONSTRUCTOR, but that's not necessarily correct if the CONSTRUCTOR is > self-modifying. We should instead be using get_or_insert_ctor_field to attach > the constructor_elt. > > So this patch reverts the PR78572 fix and makes the appropriate changes to > cxx_eval_bare_aggregate. This fixes PR94470, and we now are also able to reduce > the initializers of 'arr' and 'arr2' in the new test array57.C to constant > initializers. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to commit? OK. > gcc/cp/ChangeLog: > > PR c++/94470 > * constexpr.c (get_or_insert_ctor_field): Set default value of parameter > 'pos_hint' to -1. > (cxx_eval_bare_aggregate): Use get_or_insert_ctor_field instead of > assuming the the next index belongs at the end of the new CONSTRUCTOR. > (cxx_eval_store_expression): Revert PR c++/78572 fix. > > gcc/testsuite/ChangeLog: > > PR c++/94470 > * g++.dg/cpp1y/constexpr-nsdmi8.C: New test. > * g++.dg/cpp1y/constexpr-nsdmi9.C: New test. > * g++.dg/init/array57.C: New test. > --- > gcc/cp/constexpr.c | 19 +++++++------------ > gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi8.C | 11 +++++++++++ > gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi9.C | 16 ++++++++++++++++ > gcc/testsuite/g++.dg/init/array57.C | 16 ++++++++++++++++ > 4 files changed, 50 insertions(+), 12 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi8.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi9.C > create mode 100644 gcc/testsuite/g++.dg/init/array57.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 4cf5812e8a5..182da1af8a3 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -3212,7 +3212,7 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert) > for the (integer) index of the matching constructor_elt within CTOR. */ > > static constructor_elt * > -get_or_insert_ctor_field (tree ctor, tree index, int pos_hint) > +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint = -1) > { > /* Check the hint first. */ > if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor) > @@ -3911,14 +3911,15 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t, > { > /* If we built a new CONSTRUCTOR, attach it now so that other > initializers can refer to it. */ > - CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor); > - pos_hint = vec_safe_length (*p) - 1; > + constructor_elt *cep = get_or_insert_ctor_field (ctx->ctor, index); > + cep->value = new_ctx.ctor; > + pos_hint = cep - (*p)->begin(); > } > else if (TREE_CODE (type) == UNION_TYPE) > /* Otherwise if we're constructing a non-aggregate union member, set > the active union member now so that we can later detect and diagnose > if its initializer attempts to activate another member. */ > - CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE); > + get_or_insert_ctor_field (ctx->ctor, index); > tree elt = cxx_eval_constant_expression (&new_ctx, value, > lval, > non_constant_p, overflow_p); > @@ -4718,13 +4719,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > /* And then find/build up our initializer for the path to the subobject > we're initializing. */ > tree *valp; > - if (object == ctx->object && VAR_P (object) > - && DECL_NAME (object) && ctx->call == NULL) > - /* The variable we're building up an aggregate initializer for is outside > - the constant-expression, so don't evaluate the store. We check > - DECL_NAME to handle TARGET_EXPR temporaries, which are fair game. */ > - valp = NULL; > - else if (DECL_P (object)) > + if (DECL_P (object)) > valp = ctx->global->values.get (object); > else > valp = NULL; > @@ -4820,7 +4815,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > vec_safe_push (indexes, index); > > constructor_elt *cep > - = get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1); > + = get_or_insert_ctor_field (*valp, index); > index_pos_hints.safe_push (cep - CONSTRUCTOR_ELTS (*valp)->begin()); > > if (code == UNION_TYPE) > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi8.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi8.C > new file mode 100644 > index 00000000000..6b77432f61a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi8.C > @@ -0,0 +1,11 @@ > +// PR c++/94470 > +// { dg-do compile { target c++14 } } > + > +struct X > +{ > + int b = (c=5); > + int c = (b=1); > +}; > + > +constexpr X o = { }; > +static_assert(o.b == 1 && o.c == 1, ""); > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi9.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi9.C > new file mode 100644 > index 00000000000..d51465d8439 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi9.C > @@ -0,0 +1,16 @@ > +// PR c++/94470 > +// { dg-do compile { target c++14 } } > + > +struct Y > +{ > + int a; > +}; > + > +struct X > +{ > + Y b = (c={5}); > + Y c = (b={1}); > +}; > + > +constexpr X o = { }; > +static_assert(o.b.a == 1 && o.c.a == 1, ""); > diff --git a/gcc/testsuite/g++.dg/init/array57.C b/gcc/testsuite/g++.dg/init/array57.C > new file mode 100644 > index 00000000000..2f012b7a231 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/init/array57.C > @@ -0,0 +1,16 @@ > +// PR c++/78572 > +// { dg-do run { target c++11 } } > + > +static int arr[10] = { arr[3]=5, arr[7]=3, }; > + > +struct X { int v; }; > +static X arr2[10] = { arr2[3]={5}, arr2[7]={3}, }; > + > +int > +main() > +{ > + const int expected[10] = {5,3,0,5,0,0,0,3,0,0}; > + for (int i = 0; i < 10; i++) > + if (arr[i] != expected[i] || arr2[i].v != expected[i]) > + __builtin_abort(); > +} >
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 4cf5812e8a5..182da1af8a3 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -3212,7 +3212,7 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert) for the (integer) index of the matching constructor_elt within CTOR. */ static constructor_elt * -get_or_insert_ctor_field (tree ctor, tree index, int pos_hint) +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint = -1) { /* Check the hint first. */ if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor) @@ -3911,14 +3911,15 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t, { /* If we built a new CONSTRUCTOR, attach it now so that other initializers can refer to it. */ - CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor); - pos_hint = vec_safe_length (*p) - 1; + constructor_elt *cep = get_or_insert_ctor_field (ctx->ctor, index); + cep->value = new_ctx.ctor; + pos_hint = cep - (*p)->begin(); } else if (TREE_CODE (type) == UNION_TYPE) /* Otherwise if we're constructing a non-aggregate union member, set the active union member now so that we can later detect and diagnose if its initializer attempts to activate another member. */ - CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE); + get_or_insert_ctor_field (ctx->ctor, index); tree elt = cxx_eval_constant_expression (&new_ctx, value, lval, non_constant_p, overflow_p); @@ -4718,13 +4719,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, /* And then find/build up our initializer for the path to the subobject we're initializing. */ tree *valp; - if (object == ctx->object && VAR_P (object) - && DECL_NAME (object) && ctx->call == NULL) - /* The variable we're building up an aggregate initializer for is outside - the constant-expression, so don't evaluate the store. We check - DECL_NAME to handle TARGET_EXPR temporaries, which are fair game. */ - valp = NULL; - else if (DECL_P (object)) + if (DECL_P (object)) valp = ctx->global->values.get (object); else valp = NULL; @@ -4820,7 +4815,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, vec_safe_push (indexes, index); constructor_elt *cep - = get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1); + = get_or_insert_ctor_field (*valp, index); index_pos_hints.safe_push (cep - CONSTRUCTOR_ELTS (*valp)->begin()); if (code == UNION_TYPE) diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi8.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi8.C new file mode 100644 index 00000000000..6b77432f61a --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi8.C @@ -0,0 +1,11 @@ +// PR c++/94470 +// { dg-do compile { target c++14 } } + +struct X +{ + int b = (c=5); + int c = (b=1); +}; + +constexpr X o = { }; +static_assert(o.b == 1 && o.c == 1, ""); diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi9.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi9.C new file mode 100644 index 00000000000..d51465d8439 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi9.C @@ -0,0 +1,16 @@ +// PR c++/94470 +// { dg-do compile { target c++14 } } + +struct Y +{ + int a; +}; + +struct X +{ + Y b = (c={5}); + Y c = (b={1}); +}; + +constexpr X o = { }; +static_assert(o.b.a == 1 && o.c.a == 1, ""); diff --git a/gcc/testsuite/g++.dg/init/array57.C b/gcc/testsuite/g++.dg/init/array57.C new file mode 100644 index 00000000000..2f012b7a231 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/array57.C @@ -0,0 +1,16 @@ +// PR c++/78572 +// { dg-do run { target c++11 } } + +static int arr[10] = { arr[3]=5, arr[7]=3, }; + +struct X { int v; }; +static X arr2[10] = { arr2[3]={5}, arr2[7]={3}, }; + +int +main() +{ + const int expected[10] = {5,3,0,5,0,0,0,3,0,0}; + for (int i = 0; i < 10; i++) + if (arr[i] != expected[i] || arr2[i].v != expected[i]) + __builtin_abort(); +}