Re: [PATCH] c++: constexpr init of union sub-aggr w/ base [PR105491]

[Patrick Palka <ppalka@redhat.com>]

On Fri, 6 May 2022, Patrick Palka wrote:

> On Fri, 6 May 2022, Jason Merrill wrote:
> 
> > On 5/6/22 14:00, Patrick Palka wrote:
> > > On Fri, 6 May 2022, Patrick Palka wrote:
> > > 
> > > > 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?
> > > 
> > > I'm testing the following, which uses a custom predicate instead of
> > > next_initializable_field in r_c_e_p.
> > 
> > Let's make it a public predicate, not internal to r_c_e_p.  Maybe it could be
> > next_subobject_field, and the current next_initializable_field change to
> > next_aggregate_field?
> 
> Will do.
> 
> > 
> > > 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).
> > 
> > Maybe for constructors we could call it on ctx->ctor instead of result, or
> > call r_c_e_p in C++20+?
> 
> But both ctx->ctor and ->object are NULL during a subobject constructor
> call (since we apparently clear these fields when entering a
> STATEMENT_LIST):
> 
> So I tried instead obtaining the constructor by evaluating new_obj via
> 
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -2993,6 +2988,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
>       in order to detect reading an unitialized object in constexpr instead
>       of value-initializing it.  (reduced_constant_expression_p is expected to
>       take care of clearing the flag.)  */
> +  if (new_obj && DECL_CONSTRUCTOR_P (fun))
> +    result = cxx_eval_constant_expression (ctx, new_obj, /*lval=*/false,
> +                                          non_constant_p, overflow_p);
>    if (TREE_CODE (result) == CONSTRUCTOR
>        && (cxx_dialect < cxx20
>           || !DECL_CONSTRUCTOR_P (fun)))
> 
> but that seems to break e.g. g++.dg/cpp2a/constexpr-init12.C because
> after the subobject constructor call
> 
>   S::S (&((struct W *) this)->s, NON_LVALUE_EXPR <8>);
> 
> the constructor for the subobject a.s in new_obj is still completely
> missing (I suppose because S::S doesn't initialize any of its members)
> so trying to obtain it causes us to complain too soon from
> cxx_eval_component_reference:
> 
> constexpr-init12.C:16:24:   in ‘constexpr’ expansion of ‘W(42)’
> constexpr-init12.C:10:22:   in ‘constexpr’ expansion of ‘((W*)this)->W::s.S::S(8)’
> constexpr-init12.C:16:24: error: accessing uninitialized member ‘W::s’
>    16 | constexpr auto a = W(42); // { dg-error "not a constant expression" }
>       |                        ^
> 
> > 
> > It does seem dubious that we would clear the flag on an outer ctor when it's
> > still set on an inner ctor, should probably add an assert somewhere.
> 
> Makes sense, not sure where the best place would be..

On second thought, if I'm understanding your suggestion correctly, I
don't think we can generally enforce such a property for
CONSTRUCTOR_NO_CLEARING, given how cxx_eval_store_expression uses it for
unions:

  union U {
    struct { int x, y; } a;
  } u;
  u.a.x = 0;

Here after evaluating the assignment, the outer ctor for the union will
have CONSTRUCTOR_NO_CLEARING cleared to indicate we finished activating
the union member, but the inner ctor is certainly not fully initialized
so it'll have CONSTRUCTOR_NO_CLEARING set still.

> 
> > 
> > Jason
> > 
> >