On Fri, 6 May 2022, Jason Merrill wrote:
> On 5/6/22 11:22, Patrick Palka wrote:
> > Here ever since r10-7313-gb599bf9d6d1e18, reduced_constant_expression_p
> > in C++11/14 is rejecting the marked sub-aggregate initializer (of type S)
> >
> > W w = {.D.2445={.s={.D.2387={.m=0}, .b=0}}}
> > ^
> >
> > ultimately because said initializer has CONSTRUCTOR_NO_CLEARING set, and
> > so the function proceeds to verify that all fields of S are initialized.
> > And before C++17 we don't expect to see base class fields (since
> > next_initializable_field skips over the), so the base class initializer
> > causes r_c_e_p to return false.
>
> That seems like the primary bug. I guess r_c_e_p shouldn't be using
> next_initializable_field. Really that function should only be used for
> aggregates.
I see, I'll try replacing it in r_c_e_p. Would that be in addition to
or instead of the clear_no_implicit_zero approach?
>
> > The base class initializer comes from
> > the constructor call S::S(int).
> >
> > The reason this is caused by r10-7313-gb599bf9d6d1e18 is because in that
> > commit we began using CONSTRUCTOR_NO_CLEARING to also track whether we're
> > in middle of activating a union member. This overloaded use of the flag
> > affects clear_no_implicit_zero, which recurses into sub-aggregate
> > initializers only if the outer initializer has CONSTRUCTOR_NO_CLEARING
> > set.
>
> Is that really overloaded? In both union and non-union cases, it indicates
> that the object is not completely initialized.
Ah yes, makes sense. More accurately the immediate clearing of
CONSTRUCTOR_NO_CLEARING after activating a union member at the end of
cxx_eval_store_expression is what affects clear_no_implicit_zero later.
>
> > After that commit, the outer union initializer above no longer has
> > the flag set at this point and so clear_no_implicit_zero no longer clears
> > CONSTRUCTOR_NO_CLEARING for the marked inner initializer.
>
> Why wasn't it cleared for the inner initializer as part of that evaluation?
Looks like the inner initializer {.D.2387={.m=0}, .b=0} is formed during
the subobject constructor call:
V::V (&((struct S *) this)->D.2120);
after the evaluation of which, 'result' in cxx_eval_call_expression is NULL
(presumably because it's a CALL_EXPR, not AGGR_INIT_EXPR?):
/* This can be null for a subobject constructor call, in
which case what we care about is the initialization
side-effects rather than the value. We could get at the
value by evaluating *this, but we don't bother; there's
no need to put such a call in the hash table. */
result = lval ? ctx->object : ctx->ctor;
so we end up not calling clear_no_implicit_zero for the inner initializer
directly. We only call clear_no_implicit_zero after evaluating the
AGGR_INIT_EXPR for outermost initializer (of type W).
>
> > This patch fixes this by restoring the recursive behavior of
> > clear_no_implicit_zero for union initializers. Arguably we should
> > we could improve reduced_constant_expression_p to accept the marked
> > initializer in C++11/14 even if it has CONSTRUCTOR_NO_CLEARING set, but
> > adjusting clear_no_implicit_zero seems safer to backport.
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk/12/11/10?
> >
> > PR c++/105491
> >
> > gcc/cp/ChangeLog:
> >
> > * constexpr.cc (clear_no_implicit_zero): Recurse into a union
> > initializer even if CONSTRUCTOR_NO_CLEARING is already cleared.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp0x/constexpr-union7.C: New test.
> > * g++.dg/cpp0x/constexpr-union7a.C: New test.
> > * g++.dg/cpp2a/constinit17.C: New test.
> > ---
> > gcc/cp/constexpr.cc | 7 +++++-
> > gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C | 17 +++++++++++++
> > .../g++.dg/cpp0x/constexpr-union7a.C | 15 ++++++++++++
> > gcc/testsuite/g++.dg/cpp2a/constinit17.C | 24 +++++++++++++++++++
> > 4 files changed, 62 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-union7a.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constinit17.C
> >
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 9b1e71857fc..75fecbcbcb7 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -1886,7 +1886,12 @@ cxx_eval_internal_function (const constexpr_ctx *ctx,
> > tree t,
> > static void
> > clear_no_implicit_zero (tree ctor)
> > {
> > - if (CONSTRUCTOR_NO_CLEARING (ctor))
> > + if (CONSTRUCTOR_NO_CLEARING (ctor)
> > + /* For a union initializer, the flag could already be cleared but not
> > + necessarily yet for its sub-aggregates, since for unions the flag is
> > + also used by cxx_eval_store_expression to track whether we're in the
> > + middle of activating one of its members. */
> > + || TREE_CODE (TREE_TYPE (ctor)) == UNION_TYPE)
> > {
> > CONSTRUCTOR_NO_CLEARING (ctor) = false;
> > for (auto &e: CONSTRUCTOR_ELTS (ctor))
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C
> > b/gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C
> > new file mode 100644
> > index 00000000000..b3147d9db50
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C
> > @@ -0,0 +1,17 @@
> > +// PR c++/105491
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct V {
> > + int m = 0;
> > +};
> > +struct S : V {
> > + constexpr S(int) : b() { }
> > + bool b;
> > +};
> > +struct W {
> > + constexpr W() : s(0) { }
> > + union {
> > + S s;
> > + };
> > +};
> > +constexpr W w;
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-union7a.C
> > b/gcc/testsuite/g++.dg/cpp0x/constexpr-union7a.C
> > new file mode 100644
> > index 00000000000..b676e7d1748
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-union7a.C
> > @@ -0,0 +1,15 @@
> > +// PR c++/105491
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct V {
> > + int m = 0;
> > +};
> > +struct S : V {
> > + constexpr S(int) : b() { }
> > + bool b;
> > +};
> > +union W {
> > + constexpr W() : s(0) { }
> > + S s;
> > +};
> > +constexpr W w;
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/constinit17.C
> > b/gcc/testsuite/g++.dg/cpp2a/constinit17.C
> > new file mode 100644
> > index 00000000000..6431654ac85
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/constinit17.C
> > @@ -0,0 +1,24 @@
> > +// PR c++/105491
> > +// { dg-do compile { target c++11 } }
> > +
> > +class Message {
> > + virtual int GetMetadata();
> > +};
> > +class ProtobufCFileOptions : Message {
> > +public:
> > + constexpr ProtobufCFileOptions(int);
> > + bool no_generate_;
> > + bool const_strings_;
> > + bool use_oneof_field_name_;
> > + bool gen_pack_helpers_;
> > + bool gen_init_helpers_;
> > +};
> > +constexpr ProtobufCFileOptions::ProtobufCFileOptions(int)
> > + : no_generate_(), const_strings_(), use_oneof_field_name_(),
> > + gen_pack_helpers_(), gen_init_helpers_() {}
> > +struct ProtobufCFileOptionsDefaultTypeInternal {
> > + constexpr ProtobufCFileOptionsDefaultTypeInternal() : _instance({}) {}
> > + union {
> > + ProtobufCFileOptions _instance;
> > + };
> > +} __constinit _ProtobufCFileOptions_default_instance_;
>
>
