On Thu, 19 Mar 2020, Jason Merrill wrote:
> On 3/19/20 12:35 PM, Patrick Palka wrote:
> > This patch adds a check to detect changing the active union member during
> > initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as a
> > proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning to
> > in
> > cxx_eval_store_expression is in the process of being initialized, which
> > seems to
> > work well.
>
> > In order for this check to work reliably, we also have to adjust
> > cxx_eval_bare_aggregate to set the active union member before processing the
> > initializer.
> >
> > Does this look OK to commit after testing?
>
> That makes sense. OK.
Thanks. The issue Marek points out unfortunately makes this approach of
using CONSTRUCTOR_NO_CLEARING unreliable and regresses other testcases,
and I'm afraid I don't know how to salvage this approach...
>
> > gcc/cp/ChangeLog:
> >
> > PR c++/94066
> > * constexpr.c (cxx_eval_bare_aggregate): When constructing a union,
> > always set the active union member before evaluating the initializer.
> > Relax assertion that verifies the index of the constructor element
> > we're
> > initializing hasn't been changed.
> > (cxx_eval_store_expression): Diagnose changing the active union member
> > while the union is in the process of being initialized.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/94066
> > * g++.dg/cpp1y/pr94066.C: New test.
> > * g++.dg/cpp1y/pr94066-2.C: New test.
> > * g++.dg/cpp1y/pr94066-3.C: New test.
> > ---
> > gcc/cp/constexpr.c | 25 ++++++++++++++++++++++++-
> > gcc/testsuite/g++.dg/cpp1y/pr94066-2.C | 19 +++++++++++++++++++
> > gcc/testsuite/g++.dg/cpp1y/pr94066-3.C | 18 ++++++++++++++++++
> > gcc/testsuite/g++.dg/cpp1y/pr94066.C | 18 ++++++++++++++++++
> > 4 files changed, 79 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066.C
> >
> > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > index 192face9a3a..97fe5572f71 100644
> > --- a/gcc/cp/constexpr.c
> > +++ b/gcc/cp/constexpr.c
> > @@ -3751,6 +3751,11 @@ 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);
> > + else if (TREE_CODE (type) == UNION_TYPE)
> > + /* If we're constructing a union, set the active union member now so
> > + that we can later detect if the initializer attempts to activate
> > + another member. */
> > + CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
> > tree elt = cxx_eval_constant_expression (&new_ctx, value,
> > lval,
> > non_constant_p, overflow_p);
> > @@ -3784,7 +3789,12 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx,
> > tree t,
> > }
> > else
> > {
> > - if (new_ctx.ctor != ctx->ctor)
> > + if (TREE_CODE (type) == UNION_TYPE
> > + && (*p)->last().index != index)
> > + /* The initializer may have erroneously changed the active union
> > + member we were initializing. */
> > + gcc_assert (*non_constant_p);
> > + else if (new_ctx.ctor != ctx->ctor)
> > {
> > /* We appended this element above; update the value. */
> > gcc_assert ((*p)->last().index == index);
> > @@ -4647,6 +4657,19 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
> > tree t,
> > index);
> > *non_constant_p = true;
> > }
> > + else if (TREE_CODE (t) == MODIFY_EXPR
> > + && CONSTRUCTOR_NO_CLEARING (*valp))
> > + {
> > + /* Diagnose changing the active union member while the union
> > + is in the process of being initialized. */
> > + if (!ctx->quiet)
> > + error_at (cp_expr_loc_or_input_loc (t),
> > + "change of the active member of a union "
> > + "from %qD to %qD during initialization",
> > + CONSTRUCTOR_ELT (*valp, 0)->index,
> > + index);
> > + *non_constant_p = true;
> > + }
> > /* Changing active member. */
> > vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
> > no_zero_init = true;
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
> > b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
> > new file mode 100644
> > index 00000000000..1c00b650961
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
> > @@ -0,0 +1,19 @@
> > +// PR c++/94066
> > +// { dg-do compile { target c++14 } }
> > +
> > +struct A { long x; };
> > +
> > +union U;
> > +constexpr A foo(U *up);
> > +
> > +union U {
> > + U() = default;
> > + A a = foo(this); int y;
> > +};
> > +
> > +constexpr A foo(U *up) {
> > + up->y = 11; // { dg-error "'U::a' to 'U::y'" }
> > + return {42};
> > +}
> > +
> > +extern constexpr U u = {};
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
> > b/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
> > new file mode 100644
> > index 00000000000..6bf1ec81885
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
> > @@ -0,0 +1,18 @@
> > +// PR c++/94066
> > +// { dg-do compile { target c++14 } }
> > +
> > +struct A { long x; };
> > +
> > +union U;
> > +constexpr int foo(U *up);
> > +
> > +union U {
> > + int a = foo(this); int y;
> > +};
> > +
> > +constexpr int foo(U *up) {
> > + up->y = 11; // { dg-error "'U::a' to 'U::y'" }
> > + return {42};
> > +}
> > +
> > +extern constexpr U u = {};
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066.C
> > b/gcc/testsuite/g++.dg/cpp1y/pr94066.C
> > new file mode 100644
> > index 00000000000..6725c8c737f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1y/pr94066.C
> > @@ -0,0 +1,18 @@
> > +// PR c++/94066
> > +// { dg-do compile { target c++14 } }
> > +
> > +struct A { long x; };
> > +
> > +union U;
> > +constexpr A foo(U *up);
> > +
> > +union U {
> > + A a = foo(this); int y;
> > +};
> > +
> > +constexpr A foo(U *up) {
> > + up->y = 11; // { dg-error "'U::a' to 'U::y'" }
> > + return {42};
> > +}
> > +
> > +extern constexpr U u = {};
> >
>
>
