Re: [C++ PATCH] c++/91353 - P1331R2: Allow trivial default init in constexpr contexts.

[Jason Merrill <jason at redhat dot com>]

On 11/27/19 6:35 PM, Marek Polacek wrote:
> On Wed, Nov 27, 2019 at 04:47:01PM -0500, Jason Merrill wrote:
> > On 11/27/19 2:36 PM, Marek Polacek wrote:
> > > On Sun, Nov 24, 2019 at 12:24:48PM -0500, Jason Merrill wrote:
> > > > On 11/16/19 5:23 PM, Marek Polacek wrote:
> > > > > [ Working virtually on Baker Island. ]
> > > > >
> > > > > This patch implements C++20 P1331, allowing trivial default initialization in
> > > > > constexpr contexts.
> > > > >
> > > > > I used Jakub's patch from the PR which allowed uninitialized variables in
> > > > > constexpr contexts.  But the hard part was handling CONSTRUCTOR_NO_CLEARING
> > > > > which is always cleared in cxx_eval_call_expression.  We need to set it in
> > > > > the case a constexpr constructor doesn't initialize all the members, so that
> > > > > we can give proper diagnostic instead of value-initializing.  A lot of my
> > > > > attempts flopped but then I came up with this approach, which handles various
> > > > > cases as tested in constexpr-init8.C, where S is initialized by a non-default
> > > > > constexpr constructor, and constexpr-init9.C, using delegating constructors.
> > > > > And the best part is that I didn't need any new cx_check_missing_mem_inits
> > > > > calls!  Just save the information whether a constructor is missing an init
> > > > > into constexpr_fundef_table and retrieve it when needed.
> > > >
> > > > Is it necessary to clear the flag for constructors that do happen to
> > > > initialize all the members?  I would think that leaving that clearing to
> > > > reduced_constant_expression_p would be enough.
> > >
> > > It seems so: if I tweak cxx_eval_call_expression to only call clear_no_implicit_zero
> > > when 'fun' isn't DECL_CONSTRUCTOR_P, then a lot breaks, e.g. constexpr-base.C
> > > where the constructor initializes all the members.  By breaking I mean spurious
> > > errors coming from
> > >
> > > 5937   if (TREE_CODE (r) == CONSTRUCTOR && CONSTRUCTOR_NO_CLEARING (r))
> > > 5938     {
> > > 5939       if (!allow_non_constant)
> > > 5940         error ("%qE is not a constant expression because it refers to "
> > > 5941                "an incompletely initialized variable", t);
> > > 5942       TREE_CONSTANT (r) = false;
> > > 5943       non_constant_p = true;
> > > 5944     }
> >
> > Why didn't reduced_constant_expression_p unset CONSTRUCTOR_NO_CLEARING?
>
> We have a constructor that initializes a base class and members of a class:
>
>    {.D.2364={.i=12}, .a={.i=24}, .j=36}
>
> Say we don't clear CONSTRUCTOR_NO_CLEARING in this ctor in cxx_eval_call_expression.
> Then soon in reduced_constant_expression_p we do
> 2221             field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
> and since "Implement P0017R1, C++17 aggregates with bases. / r241187" we skip
> base fields in C++17 so 'field' is set to 'a'.

Hmm?

> next_initializable_field (tree field)
> {
>   while (field
>          && (TREE_CODE (field) != FIELD_DECL
>              || DECL_UNNAMED_BIT_FIELD (field)
>              || (DECL_ARTIFICIAL (field)
>                  && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field)))))
>     field = DECL_CHAIN (field);

This skips artificial fields except that in C++17 and up base fields are 
*not* skipped.

How are you getting field starting with 'a'?  Are you compiling in a 
lower standard mode?  The code using next_initializable_field doesn't 
work for lower -std because of skipping base fields.

So perhaps we want to always clear_no_implicit_zero before c++20, and 
always for c++20 and up?

> Now we look at each element of the constructor; the first one is
> D.2364={.i=12}.  But we hit
>
> 2233               if (idx != field)
> 2234                 return false;
>
> because field is 'a' and idx is D.2364.  Were you thinking of tweaking
> reduced_constant_expression_p's behavior in C++20 and dropping the whole
> cxx_eval_call_expression hunk?
>
> --
> Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA