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

  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.  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.  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.

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
  

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.

> 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.

> 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?

> 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_;
  

On Fri, May 06, 2022 at 11:56:30AM -0400, Jason Merrill via Gcc-patches 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.

Can we please make a note to this effect in the n_i_f comment?

Thanks,
Marek
  

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_;
> 
>
  

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.

-- >8 --

gcc/cp/ChangeLog:

	* constexpr.cc (reduced_constant_expression_p): Replace use of
	next_initializable_field with custom predicate.  Refactor to
	remove the use of goto.
	* decl.cc (next_initializable_field): Skip over vptr fields.
	Document that this function is to be used only for aggregate
	classes.
---
 gcc/cp/constexpr.cc | 65 ++++++++++++++++++++++++++-------------------
 gcc/cp/decl.cc      | 15 +++++------
 2 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9b1e71857fc..d1cd556591c 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3016,7 +3016,6 @@ reduced_constant_expression_p (tree t)
 
     case CONSTRUCTOR:
       /* And we need to handle PTRMEM_CST wrapped in a CONSTRUCTOR.  */
-      tree field;
       if (CONSTRUCTOR_NO_CLEARING (t))
 	{
 	  if (TREE_CODE (TREE_TYPE (t)) == VECTOR_TYPE)
@@ -3041,42 +3040,54 @@ reduced_constant_expression_p (tree t)
 		}
 	      if (find_array_ctor_elt (t, max) == -1)
 		return false;
-	      goto ok;
 	    }
-	  else if (cxx_dialect >= cxx20
-		   && TREE_CODE (TREE_TYPE (t)) == UNION_TYPE)
+	  else if (TREE_CODE (TREE_TYPE (t)) == UNION_TYPE)
 	    {
-	      if (CONSTRUCTOR_NELTS (t) == 0)
+	      if (CONSTRUCTOR_NELTS (t) != 1)
 		/* An initialized union has a constructor element.  */
 		return false;
-	      /* And it only initializes one member.  */
-	      field = NULL_TREE;
+	      if (!reduced_constant_expression_p (CONSTRUCTOR_ELT (t, 0)->value))
+		return false;
 	    }
 	  else
-	    field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
+	    {
+	      /* Similar to the predicate used in next_initializable_field,
+		 except that we additionally skip over empty types (for which
+		 we don't require an initializer), and we always consider base
+		 class fields (not just in C++17 mode) and vptr fields.  */
+	      auto ignorable_field_p = [] (tree field) {
+		if (!field)
+		  return false;
+		return (TREE_CODE (field) != FIELD_DECL
+			|| DECL_UNNAMED_BIT_FIELD (field)
+			|| (DECL_ARTIFICIAL (field)
+			    && !DECL_FIELD_IS_BASE (field)
+			    && !DECL_VIRTUAL_P (field))
+			|| is_really_empty_class (TREE_TYPE (field),
+						  /*ignore_vptr*/false));
+	      };
+	      tree field = TYPE_FIELDS (TREE_TYPE (t));
+	      for (auto &e: CONSTRUCTOR_ELTS (t))
+		{
+		  if (!reduced_constant_expression_p (e.value))
+		    return false;
+		  while (e.index != field && ignorable_field_p (field))
+		    field = DECL_CHAIN (field);
+		  if (e.index == field)
+		    field = DECL_CHAIN (field);
+		  else
+		    return false;
+		}
+	      while (ignorable_field_p (field))
+		field = DECL_CHAIN (field);
+	      if (field)
+		return false;
+	    }
 	}
       else
-	field = NULL_TREE;
-      for (auto &e: CONSTRUCTOR_ELTS (t))
-	{
-	  /* If VAL is null, we're in the middle of initializing this
-	     element.  */
+	for (auto &e: CONSTRUCTOR_ELTS (t))
 	  if (!reduced_constant_expression_p (e.value))
 	    return false;
-	  /* Empty class field may or may not have an initializer.  */
-	  for (; field && e.index != field;
-	       field = next_initializable_field (DECL_CHAIN (field)))
-	    if (!is_really_empty_class (TREE_TYPE (field),
-					/*ignore_vptr*/false))
-	      return false;
-	  if (field)
-	    field = next_initializable_field (DECL_CHAIN (field));
-	}
-      /* There could be a non-empty field at the end.  */
-      for (; field; field = next_initializable_field (DECL_CHAIN (field)))
-	if (!is_really_empty_class (TREE_TYPE (field), /*ignore_vptr*/false))
-	  return false;
-ok:
       if (CONSTRUCTOR_NO_CLEARING (t))
 	/* All the fields are initialized.  */
 	CONSTRUCTOR_NO_CLEARING (t) = false;
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index c9110db796a..c564e5e596d 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6382,10 +6382,10 @@ struct reshape_iter
 
 static tree reshape_init_r (tree, reshape_iter *, tree, tsubst_flags_t);
 
-/* FIELD is an element of TYPE_FIELDS or NULL.  In the former case, the value
-   returned is the next FIELD_DECL (possibly FIELD itself) that can be
-   initialized.  If there are no more such fields, the return value
-   will be NULL.  */
+/* FIELD is an element of TYPE_FIELDS of an aggregate class, or NULL.  In the
+   former case, the value returned is the next FIELD_DECL (possibly FIELD
+   itself) that can be initialized.  If there are no more such fields, the
+   return value will be NULL.  */
 
 tree
 next_initializable_field (tree field)
@@ -6394,11 +6394,8 @@ next_initializable_field (tree field)
 	 && (TREE_CODE (field) != FIELD_DECL
 	     || DECL_UNNAMED_BIT_FIELD (field)
 	     || (DECL_ARTIFICIAL (field)
-		 /* In C++17, don't skip base class fields.  */
-		 && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field))
-		 /* Don't skip vptr fields.  We might see them when we're
-		    called from reduced_constant_expression_p.  */
-		 && !DECL_VIRTUAL_P (field))))
+		 /* In C++17, aggregates can have base classes.  */
+		 && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field)))))
     field = DECL_CHAIN (field);
 
   return field;
  

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?

> 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+?

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.

Jason
  

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..

> 
> Jason
> 
>
  

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
> > 
> >
  

On 5/6/22 16:10, Patrick Palka wrote:
> 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.

Why clear the flag on the union before the inner ctor is fully 
initialized, if the intent is to prevent changing the active member 
during initialization?

In the loop over ctors in cxx_eval_store_expression, I'd think if we 
encounter a CONSTRUCTOR_NO_CLEARING ctor along the way, we shouldn't 
clear the flag on an outer ctor.

Jason
  

On Fri, 6 May 2022, Jason Merrill wrote:

> On 5/6/22 16:10, Patrick Palka wrote:
> > 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.
> 
> Why clear the flag on the union before the inner ctor is fully initialized, if
> the intent is to prevent changing the active member during initialization?
> 
> In the loop over ctors in cxx_eval_store_expression, I'd think if we encounter
> a CONSTRUCTOR_NO_CLEARING ctor along the way, we shouldn't clear the flag on
> an outer ctor.

Wouldn't that prevent us from later activating a different member, which is
allowed in C++20?  It would cause us to reject e.g.

    constexpr auto f() {
      union U {
        struct { int x, y; } a;
        int b;
      } u;
      u.a.x = 0;
      u.b = 0;
      return u;
    }

    static_assert(f().b == 0);

even in C++20 with

<source>:7:7: error: change of the active member of a union from ‘f()::U::a’ to ‘f()::U::b’ during initialization
    7 |   u.b = 0;
      |   ~~~~^~~

because when evaluating the second assignment we'd see that the
CONSTRUCTOR_NO_CLEARING flag is still set on the union while it already
has an activated member.


On a related note, here's the patch that mechanically renames
next_initializable_field to next_aggregate_field and makes it skip over vptr
fields, and defines next_subobject_field alongside it for use by r_c_e_p.
Bootstrap and regtesting in progress.


-- >8 --

	PR c++/105491

gcc/cp/ChangeLog:

	* call.cc (field_in_pset): Adjust after next_initializable_field
	renaming.
	(build_aggr_conv): Likewise.
	(convert_like_internal): Likewise.
	(type_has_extended_temps): Likewise.
	* class.cc (default_init_uninitialized_part): Likewise.
	(finish_struct): Likewise.
	* constexpr.cc (cx_check_missing_mem_inits): Likewise.
	(reduced_constant_expression_p): Use next_subobject_field
	instead.
	* cp-gimplify.cc (get_source_location_impl_type): Adjust after
	next_initializable_field renaming.
	(fold_builtin_source_location): Likewise.
	* cp-tree.h (next_initializable_field): Rename to ...
	(next_aggregate_field): ... this.
	(next_subobject_field): Declare.
	* decl.cc (next_aggregate_field): Renamed from ...
	(next_initializable_field): ... this.  Skip over vptr fields.
	Document that this is now intended for aggregate classes.
	(next_subobject_field): Define.
	(reshape_init_class): Adjust after next_initializable_field
	renaming.
	* init.cc (build_value_init_noctor): Likewise.
	(emit_mem_initializers): Likewise.
	* lambda.cc (build_capture_proxy): Likewise.
	* method.cc (build_comparison_op): Likewise.
	* pt.cc (maybe_aggr_guide): Likewise.
	* tree.cc (structural_type_p): Likewise.
	* typeck2.cc (split_nonconstant_init_1): Likewise.
	(digest_init_r): Likewise.

gcc/testsuite/ChangeLog:

	* gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C: New test.
	* gcc/testsuite/g++.dg/cpp0x/constexpr-union7a.C: New test.
	* gcc/testsuite/g++.dg/cpp2a/constinit17.C: New test.
---
 gcc/cp/call.cc                                | 14 +++----
 gcc/cp/class.cc                               |  8 ++--
 gcc/cp/constexpr.cc                           | 10 ++---
 gcc/cp/cp-gimplify.cc                         |  4 +-
 gcc/cp/cp-tree.h                              |  3 +-
 gcc/cp/decl.cc                                | 38 +++++++++++++------
 gcc/cp/init.cc                                |  6 +--
 gcc/cp/lambda.cc                              |  4 +-
 gcc/cp/method.cc                              |  8 ++--
 gcc/cp/pt.cc                                  |  2 +-
 gcc/cp/tree.cc                                |  4 +-
 gcc/cp/typeck2.cc                             |  4 +-
 gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C | 17 +++++++++
 .../g++.dg/cpp0x/constexpr-union7a.C          | 15 ++++++++
 gcc/testsuite/g++.dg/cpp2a/constinit17.C      | 24 ++++++++++++
 15 files changed, 117 insertions(+), 44 deletions(-)
 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/call.cc b/gcc/cp/call.cc
index 959279d6216..0240e364324 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -948,7 +948,7 @@ field_in_pset (hash_set<tree, true> &pset, tree field)
     for (field = TYPE_FIELDS (TREE_TYPE (field));
 	 field; field = DECL_CHAIN (field))
       {
-	field = next_initializable_field (field);
+	field = next_aggregate_field (field);
 	if (field == NULL_TREE)
 	  break;
 	if (field_in_pset (pset, field))
@@ -965,7 +965,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
 {
   unsigned HOST_WIDE_INT i = 0;
   conversion *c;
-  tree field = next_initializable_field (TYPE_FIELDS (type));
+  tree field = next_aggregate_field (TYPE_FIELDS (type));
   tree empty_ctor = NULL_TREE;
   hash_set<tree, true> pset;
 
@@ -1011,7 +1011,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
 	}
     }
 
-  for (; field; field = next_initializable_field (DECL_CHAIN (field)))
+  for (; field; field = next_aggregate_field (DECL_CHAIN (field)))
     {
       tree ftype = TREE_TYPE (field);
       tree val;
@@ -8098,10 +8098,10 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
 	totype = complete_type_or_maybe_complain (totype, NULL_TREE, complain);
 	if (!totype)
 	  return error_mark_node;
-	tree field = next_initializable_field (TYPE_FIELDS (totype));
+	tree field = next_aggregate_field (TYPE_FIELDS (totype));
 	vec<constructor_elt, va_gc> *vec = NULL;
 	CONSTRUCTOR_APPEND_ELT (vec, field, array);
-	field = next_initializable_field (DECL_CHAIN (field));
+	field = next_aggregate_field (DECL_CHAIN (field));
 	CONSTRUCTOR_APPEND_ELT (vec, field, size_int (len));
 	tree new_ctor = build_constructor (totype, vec);
 	return get_target_expr_sfinae (new_ctor, complain);
@@ -13267,8 +13267,8 @@ type_has_extended_temps (tree type)
     {
       if (is_std_init_list (type))
 	return true;
-      for (tree f = next_initializable_field (TYPE_FIELDS (type));
-	   f; f = next_initializable_field (DECL_CHAIN (f)))
+      for (tree f = next_aggregate_field (TYPE_FIELDS (type));
+	   f; f = next_aggregate_field (DECL_CHAIN (f)))
 	if (type_has_extended_temps (TREE_TYPE (f)))
 	  return true;
     }
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index bc94ed45e17..3c195b35396 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -5494,8 +5494,8 @@ default_init_uninitialized_part (tree type)
       if (r)
 	return r;
     }
-  for (t = next_initializable_field (TYPE_FIELDS (type)); t;
-       t = next_initializable_field (DECL_CHAIN (t)))
+  for (t = next_aggregate_field (TYPE_FIELDS (type)); t;
+       t = next_aggregate_field (DECL_CHAIN (t)))
     if (!DECL_INITIAL (t) && !DECL_ARTIFICIAL (t))
       {
 	r = default_init_uninitialized_part (TREE_TYPE (t));
@@ -7781,10 +7781,10 @@ finish_struct (tree t, tree attributes)
       bool ok = false;
       if (processing_template_decl)
 	{
-	  tree f = next_initializable_field (TYPE_FIELDS (t));
+	  tree f = next_aggregate_field (TYPE_FIELDS (t));
 	  if (f && TYPE_PTR_P (TREE_TYPE (f)))
 	    {
-	      f = next_initializable_field (DECL_CHAIN (f));
+	      f = next_aggregate_field (DECL_CHAIN (f));
 	      if (f && same_type_p (TREE_TYPE (f), size_type_node))
 		ok = true;
 	    }
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9b1e71857fc..24717b4f8d5 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -784,7 +784,7 @@ cx_check_missing_mem_inits (tree ctype, tree body, bool complain)
 
   if (TREE_CODE (ctype) == UNION_TYPE)
     {
-      if (nelts == 0 && next_initializable_field (field))
+      if (nelts == 0 && next_aggregate_field (field))
 	{
 	  if (complain)
 	    error ("%<constexpr%> constructor for union %qT must "
@@ -3053,7 +3053,7 @@ reduced_constant_expression_p (tree t)
 	      field = NULL_TREE;
 	    }
 	  else
-	    field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
+	    field = next_subobject_field (TYPE_FIELDS (TREE_TYPE (t)));
 	}
       else
 	field = NULL_TREE;
@@ -3065,15 +3065,15 @@ reduced_constant_expression_p (tree t)
 	    return false;
 	  /* Empty class field may or may not have an initializer.  */
 	  for (; field && e.index != field;
-	       field = next_initializable_field (DECL_CHAIN (field)))
+	       field = next_subobject_field (DECL_CHAIN (field)))
 	    if (!is_really_empty_class (TREE_TYPE (field),
 					/*ignore_vptr*/false))
 	      return false;
 	  if (field)
-	    field = next_initializable_field (DECL_CHAIN (field));
+	    field = next_subobject_field (DECL_CHAIN (field));
 	}
       /* There could be a non-empty field at the end.  */
-      for (; field; field = next_initializable_field (DECL_CHAIN (field)))
+      for (; field; field = next_subobject_field (DECL_CHAIN (field)))
 	if (!is_really_empty_class (TREE_TYPE (field), /*ignore_vptr*/false))
 	  return false;
 ok:
diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index b52d9cb5754..ee47a4bfd37 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -3106,7 +3106,7 @@ get_source_location_impl_type (location_t loc)
 
   int cnt = 0;
   for (tree field = TYPE_FIELDS (type);
-       (field = next_initializable_field (field)) != NULL_TREE;
+       (field = next_aggregate_field (field)) != NULL_TREE;
        field = DECL_CHAIN (field))
     {
       if (DECL_NAME (field) != NULL_TREE)
@@ -3281,7 +3281,7 @@ fold_builtin_source_location (location_t loc)
       vec<constructor_elt, va_gc> *v = NULL;
       vec_alloc (v, 4);
       for (tree field = TYPE_FIELDS (source_location_impl);
-	   (field = next_initializable_field (field)) != NULL_TREE;
+	   (field = next_aggregate_field (field)) != NULL_TREE;
 	   field = DECL_CHAIN (field))
 	{
 	  const char *n = IDENTIFIER_POINTER (DECL_NAME (field));
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 663fe7a20fc..abcef7c20c4 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6870,7 +6870,8 @@ extern bool is_direct_enum_init			(tree, tree);
 extern void initialize_artificial_var		(tree, vec<constructor_elt, va_gc> *);
 extern tree check_var_type			(tree, tree, location_t);
 extern tree reshape_init                        (tree, tree, tsubst_flags_t);
-extern tree next_initializable_field (tree);
+extern tree next_aggregate_field		(tree);
+extern tree next_subobject_field		(tree);
 extern tree first_field				(const_tree);
 extern tree fndecl_declared_return_type		(tree);
 extern bool undeduced_auto_decl			(tree);
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index c9110db796a..d5ab2d0ad87 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6382,22 +6382,38 @@ struct reshape_iter
 
 static tree reshape_init_r (tree, reshape_iter *, tree, tsubst_flags_t);
 
+/* FIELD is an element of TYPE_FIELDS of an aggregate class, or NULL.  In the
+   former case, the value returned is the next FIELD_DECL (possibly FIELD
+   itself) that can be initialized.  If there are no more such fields, the
+   return value will be NULL.  */
+
+tree
+next_aggregate_field (tree field)
+{
+  while (field
+	 && (TREE_CODE (field) != FIELD_DECL
+	     || DECL_UNNAMED_BIT_FIELD (field)
+	     || (DECL_ARTIFICIAL (field)
+		 /* In C++17, aggregates can have base classes.  */
+		 && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field)))))
+    field = DECL_CHAIN (field);
+
+  return field;
+}
+
 /* FIELD is an element of TYPE_FIELDS or NULL.  In the former case, the value
-   returned is the next FIELD_DECL (possibly FIELD itself) that can be
-   initialized.  If there are no more such fields, the return value
-   will be NULL.  */
+   returned is the next FIELD_DECL (possibly FIELD itself) that corresponds
+   to a subobject.  If there are no more such fields, the return value will be
+   NULL.  */
 
 tree
-next_initializable_field (tree field)
+next_subobject_field (tree field)
 {
   while (field
 	 && (TREE_CODE (field) != FIELD_DECL
 	     || DECL_UNNAMED_BIT_FIELD (field)
 	     || (DECL_ARTIFICIAL (field)
-		 /* In C++17, don't skip base class fields.  */
-		 && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field))
-		 /* Don't skip vptr fields.  We might see them when we're
-		    called from reduced_constant_expression_p.  */
+		 && !DECL_FIELD_IS_BASE (field)
 		 && !DECL_VIRTUAL_P (field))))
     field = DECL_CHAIN (field);
 
@@ -6595,7 +6611,7 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
   if (base_binfo)
     field = base_binfo;
   else
-    field = next_initializable_field (TYPE_FIELDS (type));
+    field = next_aggregate_field (TYPE_FIELDS (type));
 
   if (!field)
     {
@@ -6760,10 +6776,10 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
 	  if (BINFO_BASE_ITERATE (binfo, ++binfo_idx, base_binfo))
 	    field = base_binfo;
 	  else
-	    field = next_initializable_field (TYPE_FIELDS (type));
+	    field = next_aggregate_field (TYPE_FIELDS (type));
 	}
       else
-	field = next_initializable_field (DECL_CHAIN (field));
+	field = next_aggregate_field (DECL_CHAIN (field));
     }
 
   /* A trailing aggregate element that is a pack expansion is assumed to
diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index 75ab965a218..f1ed9336dc9 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -422,7 +422,7 @@ build_value_init_noctor (tree type, tsubst_flags_t complain)
 		  && !COMPLETE_TYPE_P (ftype)
 		  && !TYPE_DOMAIN (ftype)
 		  && COMPLETE_TYPE_P (TREE_TYPE (ftype))
-		  && (next_initializable_field (DECL_CHAIN (field))
+		  && (next_aggregate_field (DECL_CHAIN (field))
 		      == NULL_TREE))
 		continue;
 
@@ -1477,9 +1477,9 @@ emit_mem_initializers (tree mem_inits)
 
   /* Initially that is all of them.  */
   if (warn_uninitialized)
-    for (tree f = next_initializable_field (TYPE_FIELDS (current_class_type));
+    for (tree f = next_aggregate_field (TYPE_FIELDS (current_class_type));
 	 f != NULL_TREE;
-	 f = next_initializable_field (DECL_CHAIN (f)))
+	 f = next_aggregate_field (DECL_CHAIN (f)))
       if (!DECL_ARTIFICIAL (f)
 	  && !is_really_empty_class (TREE_TYPE (f), /*ignore_vptr*/false))
 	uninitialized.add (f);
diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
index afac53b6d7c..27704a578af 100644
--- a/gcc/cp/lambda.cc
+++ b/gcc/cp/lambda.cc
@@ -425,9 +425,9 @@ build_capture_proxy (tree member, tree init)
   if (DECL_VLA_CAPTURE_P (member))
     {
       /* Rebuild the VLA type from the pointer and maxindex.  */
-      tree field = next_initializable_field (TYPE_FIELDS (type));
+      tree field = next_aggregate_field (TYPE_FIELDS (type));
       tree ptr = build_simple_component_ref (object, field);
-      field = next_initializable_field (DECL_CHAIN (field));
+      field = next_aggregate_field (DECL_CHAIN (field));
       tree max = build_simple_component_ref (object, field);
       type = build_cplus_array_type (TREE_TYPE (TREE_TYPE (ptr)),
 				     build_index_type (max));
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 903ee666ef3..0dffd648b0b 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -1465,7 +1465,7 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
   /* A defaulted comparison operator function for class C is defined as
      deleted if ... C has variant members.  */
   if (TREE_CODE (ctype) == UNION_TYPE
-      && next_initializable_field (TYPE_FIELDS (ctype)))
+      && next_aggregate_field (TYPE_FIELDS (ctype)))
     {
       if (complain & tf_error)
 	inform (info.loc, "cannot default compare union %qT", ctype);
@@ -1518,9 +1518,9 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
 	}
 
       /* Now compare the field subobjects.  */
-      for (tree field = next_initializable_field (TYPE_FIELDS (ctype));
+      for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
 	   field;
-	   field = next_initializable_field (DECL_CHAIN (field)))
+	   field = next_aggregate_field (DECL_CHAIN (field)))
 	{
 	  if (DECL_VIRTUAL_P (field) || DECL_FIELD_IS_BASE (field))
 	    /* We ignore the vptr, and we already handled bases.  */
@@ -1542,7 +1542,7 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
 	      continue;
 	    }
 	  else if (ANON_UNION_TYPE_P (expr_type)
-		   && next_initializable_field (TYPE_FIELDS (expr_type)))
+		   && next_aggregate_field (TYPE_FIELDS (expr_type)))
 	    {
 	      if (complain & tf_error)
 		inform (field_loc, "cannot default compare "
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index ac002907a41..86e7212daa5 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -29578,7 +29578,7 @@ maybe_aggr_guide (tree tmpl, tree init, vec<tree,va_gc> *args)
 	   len;
 	   --len, field = DECL_CHAIN (field))
 	{
-	  field = next_initializable_field (field);
+	  field = next_aggregate_field (field);
 	  if (!field)
 	    return NULL_TREE;
 	  tree ftype = finish_decltype_type (field, true, complain);
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index ed0d0d22950..aba695116c5 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -4852,8 +4852,8 @@ structural_type_p (tree t, bool explain)
 	explain_non_literal_class (t);
       return false;
     }
-  for (tree m = next_initializable_field (TYPE_FIELDS (t)); m;
-       m = next_initializable_field (DECL_CHAIN (m)))
+  for (tree m = next_aggregate_field (TYPE_FIELDS (t)); m;
+       m = next_aggregate_field (DECL_CHAIN (m)))
     {
       if (TREE_PRIVATE (m) || TREE_PROTECTED (m))
 	{
diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
index 63d95c1529a..1d92310edd0 100644
--- a/gcc/cp/typeck2.cc
+++ b/gcc/cp/typeck2.cc
@@ -606,7 +606,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool last,
 				  : TYPE_FIELDS (type));
 		     ; prev = DECL_CHAIN (prev))
 		  {
-		    prev = next_initializable_field (prev);
+		    prev = next_aggregate_field (prev);
 		    if (prev == field_index)
 		      break;
 		    tree ptype = TREE_TYPE (prev);
@@ -1304,7 +1304,7 @@ digest_init_r (tree type, tree init, int nested, int flags,
 	    the first element of d, which is the B base subobject.  The base
 	    of type B is copy-initialized from the D temporary, causing
 	    object slicing.  */
-	  tree field = next_initializable_field (TYPE_FIELDS (type));
+	  tree field = next_aggregate_field (TYPE_FIELDS (type));
 	  if (field && DECL_FIELD_IS_BASE (field))
 	    {
 	      if (warning_at (loc, 0, "initializing a base class of type %qT "
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_;
  

On 5/6/22 16:46, Patrick Palka wrote:
> On Fri, 6 May 2022, Jason Merrill wrote:
> 
>> On 5/6/22 16:10, Patrick Palka wrote:
>>> 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.
>>
>> Why clear the flag on the union before the inner ctor is fully initialized, if
>> the intent is to prevent changing the active member during initialization?
>>
>> In the loop over ctors in cxx_eval_store_expression, I'd think if we encounter
>> a CONSTRUCTOR_NO_CLEARING ctor along the way, we shouldn't clear the flag on
>> an outer ctor.
> 
> Wouldn't that prevent us from later activating a different member, which is
> allowed in C++20?  It would cause us to reject e.g.
> 
>      constexpr auto f() {
>        union U {
>          struct { int x, y; } a;
>          int b;
>        } u;
>        u.a.x = 0;
>        u.b = 0;
>        return u;
>      }
> 
>      static_assert(f().b == 0);
> 
> even in C++20 with
> 
> <source>:7:7: error: change of the active member of a union from ‘f()::U::a’ to ‘f()::U::b’ during initialization
>      7 |   u.b = 0;
>        |   ~~~~^~~
> 
> because when evaluating the second assignment we'd see that the
> CONSTRUCTOR_NO_CLEARING flag is still set on the union while it already
> has an activated member.

Ah, makes sense.

> On a related note, here's the patch that mechanically renames
> next_initializable_field to next_aggregate_field and makes it skip over vptr
> fields, and defines next_subobject_field alongside it for use by r_c_e_p.
> Bootstrap and regtesting in progress.

OK if successful.

> 
> -- >8 --
> 
> 	PR c++/105491
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (field_in_pset): Adjust after next_initializable_field
> 	renaming.
> 	(build_aggr_conv): Likewise.
> 	(convert_like_internal): Likewise.
> 	(type_has_extended_temps): Likewise.
> 	* class.cc (default_init_uninitialized_part): Likewise.
> 	(finish_struct): Likewise.
> 	* constexpr.cc (cx_check_missing_mem_inits): Likewise.
> 	(reduced_constant_expression_p): Use next_subobject_field
> 	instead.
> 	* cp-gimplify.cc (get_source_location_impl_type): Adjust after
> 	next_initializable_field renaming.
> 	(fold_builtin_source_location): Likewise.
> 	* cp-tree.h (next_initializable_field): Rename to ...
> 	(next_aggregate_field): ... this.
> 	(next_subobject_field): Declare.
> 	* decl.cc (next_aggregate_field): Renamed from ...
> 	(next_initializable_field): ... this.  Skip over vptr fields.
> 	Document that this is now intended for aggregate classes.
> 	(next_subobject_field): Define.
> 	(reshape_init_class): Adjust after next_initializable_field
> 	renaming.
> 	* init.cc (build_value_init_noctor): Likewise.
> 	(emit_mem_initializers): Likewise.
> 	* lambda.cc (build_capture_proxy): Likewise.
> 	* method.cc (build_comparison_op): Likewise.
> 	* pt.cc (maybe_aggr_guide): Likewise.
> 	* tree.cc (structural_type_p): Likewise.
> 	* typeck2.cc (split_nonconstant_init_1): Likewise.
> 	(digest_init_r): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C: New test.
> 	* gcc/testsuite/g++.dg/cpp0x/constexpr-union7a.C: New test.
> 	* gcc/testsuite/g++.dg/cpp2a/constinit17.C: New test.
> ---
>   gcc/cp/call.cc                                | 14 +++----
>   gcc/cp/class.cc                               |  8 ++--
>   gcc/cp/constexpr.cc                           | 10 ++---
>   gcc/cp/cp-gimplify.cc                         |  4 +-
>   gcc/cp/cp-tree.h                              |  3 +-
>   gcc/cp/decl.cc                                | 38 +++++++++++++------
>   gcc/cp/init.cc                                |  6 +--
>   gcc/cp/lambda.cc                              |  4 +-
>   gcc/cp/method.cc                              |  8 ++--
>   gcc/cp/pt.cc                                  |  2 +-
>   gcc/cp/tree.cc                                |  4 +-
>   gcc/cp/typeck2.cc                             |  4 +-
>   gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C | 17 +++++++++
>   .../g++.dg/cpp0x/constexpr-union7a.C          | 15 ++++++++
>   gcc/testsuite/g++.dg/cpp2a/constinit17.C      | 24 ++++++++++++
>   15 files changed, 117 insertions(+), 44 deletions(-)
>   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/call.cc b/gcc/cp/call.cc
> index 959279d6216..0240e364324 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -948,7 +948,7 @@ field_in_pset (hash_set<tree, true> &pset, tree field)
>       for (field = TYPE_FIELDS (TREE_TYPE (field));
>   	 field; field = DECL_CHAIN (field))
>         {
> -	field = next_initializable_field (field);
> +	field = next_aggregate_field (field);
>   	if (field == NULL_TREE)
>   	  break;
>   	if (field_in_pset (pset, field))
> @@ -965,7 +965,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>   {
>     unsigned HOST_WIDE_INT i = 0;
>     conversion *c;
> -  tree field = next_initializable_field (TYPE_FIELDS (type));
> +  tree field = next_aggregate_field (TYPE_FIELDS (type));
>     tree empty_ctor = NULL_TREE;
>     hash_set<tree, true> pset;
>   
> @@ -1011,7 +1011,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>   	}
>       }
>   
> -  for (; field; field = next_initializable_field (DECL_CHAIN (field)))
> +  for (; field; field = next_aggregate_field (DECL_CHAIN (field)))
>       {
>         tree ftype = TREE_TYPE (field);
>         tree val;
> @@ -8098,10 +8098,10 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
>   	totype = complete_type_or_maybe_complain (totype, NULL_TREE, complain);
>   	if (!totype)
>   	  return error_mark_node;
> -	tree field = next_initializable_field (TYPE_FIELDS (totype));
> +	tree field = next_aggregate_field (TYPE_FIELDS (totype));
>   	vec<constructor_elt, va_gc> *vec = NULL;
>   	CONSTRUCTOR_APPEND_ELT (vec, field, array);
> -	field = next_initializable_field (DECL_CHAIN (field));
> +	field = next_aggregate_field (DECL_CHAIN (field));
>   	CONSTRUCTOR_APPEND_ELT (vec, field, size_int (len));
>   	tree new_ctor = build_constructor (totype, vec);
>   	return get_target_expr_sfinae (new_ctor, complain);
> @@ -13267,8 +13267,8 @@ type_has_extended_temps (tree type)
>       {
>         if (is_std_init_list (type))
>   	return true;
> -      for (tree f = next_initializable_field (TYPE_FIELDS (type));
> -	   f; f = next_initializable_field (DECL_CHAIN (f)))
> +      for (tree f = next_aggregate_field (TYPE_FIELDS (type));
> +	   f; f = next_aggregate_field (DECL_CHAIN (f)))
>   	if (type_has_extended_temps (TREE_TYPE (f)))
>   	  return true;
>       }
> diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
> index bc94ed45e17..3c195b35396 100644
> --- a/gcc/cp/class.cc
> +++ b/gcc/cp/class.cc
> @@ -5494,8 +5494,8 @@ default_init_uninitialized_part (tree type)
>         if (r)
>   	return r;
>       }
> -  for (t = next_initializable_field (TYPE_FIELDS (type)); t;
> -       t = next_initializable_field (DECL_CHAIN (t)))
> +  for (t = next_aggregate_field (TYPE_FIELDS (type)); t;
> +       t = next_aggregate_field (DECL_CHAIN (t)))
>       if (!DECL_INITIAL (t) && !DECL_ARTIFICIAL (t))
>         {
>   	r = default_init_uninitialized_part (TREE_TYPE (t));
> @@ -7781,10 +7781,10 @@ finish_struct (tree t, tree attributes)
>         bool ok = false;
>         if (processing_template_decl)
>   	{
> -	  tree f = next_initializable_field (TYPE_FIELDS (t));
> +	  tree f = next_aggregate_field (TYPE_FIELDS (t));
>   	  if (f && TYPE_PTR_P (TREE_TYPE (f)))
>   	    {
> -	      f = next_initializable_field (DECL_CHAIN (f));
> +	      f = next_aggregate_field (DECL_CHAIN (f));
>   	      if (f && same_type_p (TREE_TYPE (f), size_type_node))
>   		ok = true;
>   	    }
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index 9b1e71857fc..24717b4f8d5 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -784,7 +784,7 @@ cx_check_missing_mem_inits (tree ctype, tree body, bool complain)
>   
>     if (TREE_CODE (ctype) == UNION_TYPE)
>       {
> -      if (nelts == 0 && next_initializable_field (field))
> +      if (nelts == 0 && next_aggregate_field (field))
>   	{
>   	  if (complain)
>   	    error ("%<constexpr%> constructor for union %qT must "
> @@ -3053,7 +3053,7 @@ reduced_constant_expression_p (tree t)
>   	      field = NULL_TREE;
>   	    }
>   	  else
> -	    field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
> +	    field = next_subobject_field (TYPE_FIELDS (TREE_TYPE (t)));
>   	}
>         else
>   	field = NULL_TREE;
> @@ -3065,15 +3065,15 @@ reduced_constant_expression_p (tree t)
>   	    return false;
>   	  /* Empty class field may or may not have an initializer.  */
>   	  for (; field && e.index != field;
> -	       field = next_initializable_field (DECL_CHAIN (field)))
> +	       field = next_subobject_field (DECL_CHAIN (field)))
>   	    if (!is_really_empty_class (TREE_TYPE (field),
>   					/*ignore_vptr*/false))
>   	      return false;
>   	  if (field)
> -	    field = next_initializable_field (DECL_CHAIN (field));
> +	    field = next_subobject_field (DECL_CHAIN (field));
>   	}
>         /* There could be a non-empty field at the end.  */
> -      for (; field; field = next_initializable_field (DECL_CHAIN (field)))
> +      for (; field; field = next_subobject_field (DECL_CHAIN (field)))
>   	if (!is_really_empty_class (TREE_TYPE (field), /*ignore_vptr*/false))
>   	  return false;
>   ok:
> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
> index b52d9cb5754..ee47a4bfd37 100644
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
> @@ -3106,7 +3106,7 @@ get_source_location_impl_type (location_t loc)
>   
>     int cnt = 0;
>     for (tree field = TYPE_FIELDS (type);
> -       (field = next_initializable_field (field)) != NULL_TREE;
> +       (field = next_aggregate_field (field)) != NULL_TREE;
>          field = DECL_CHAIN (field))
>       {
>         if (DECL_NAME (field) != NULL_TREE)
> @@ -3281,7 +3281,7 @@ fold_builtin_source_location (location_t loc)
>         vec<constructor_elt, va_gc> *v = NULL;
>         vec_alloc (v, 4);
>         for (tree field = TYPE_FIELDS (source_location_impl);
> -	   (field = next_initializable_field (field)) != NULL_TREE;
> +	   (field = next_aggregate_field (field)) != NULL_TREE;
>   	   field = DECL_CHAIN (field))
>   	{
>   	  const char *n = IDENTIFIER_POINTER (DECL_NAME (field));
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 663fe7a20fc..abcef7c20c4 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6870,7 +6870,8 @@ extern bool is_direct_enum_init			(tree, tree);
>   extern void initialize_artificial_var		(tree, vec<constructor_elt, va_gc> *);
>   extern tree check_var_type			(tree, tree, location_t);
>   extern tree reshape_init                        (tree, tree, tsubst_flags_t);
> -extern tree next_initializable_field (tree);
> +extern tree next_aggregate_field		(tree);
> +extern tree next_subobject_field		(tree);
>   extern tree first_field				(const_tree);
>   extern tree fndecl_declared_return_type		(tree);
>   extern bool undeduced_auto_decl			(tree);
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index c9110db796a..d5ab2d0ad87 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -6382,22 +6382,38 @@ struct reshape_iter
>   
>   static tree reshape_init_r (tree, reshape_iter *, tree, tsubst_flags_t);
>   
> +/* FIELD is an element of TYPE_FIELDS of an aggregate class, or NULL.  In the
> +   former case, the value returned is the next FIELD_DECL (possibly FIELD
> +   itself) that can be initialized.  If there are no more such fields, the
> +   return value will be NULL.  */
> +
> +tree
> +next_aggregate_field (tree field)
> +{
> +  while (field
> +	 && (TREE_CODE (field) != FIELD_DECL
> +	     || DECL_UNNAMED_BIT_FIELD (field)
> +	     || (DECL_ARTIFICIAL (field)
> +		 /* In C++17, aggregates can have base classes.  */
> +		 && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field)))))
> +    field = DECL_CHAIN (field);
> +
> +  return field;
> +}
> +
>   /* FIELD is an element of TYPE_FIELDS or NULL.  In the former case, the value
> -   returned is the next FIELD_DECL (possibly FIELD itself) that can be
> -   initialized.  If there are no more such fields, the return value
> -   will be NULL.  */
> +   returned is the next FIELD_DECL (possibly FIELD itself) that corresponds
> +   to a subobject.  If there are no more such fields, the return value will be
> +   NULL.  */
>   
>   tree
> -next_initializable_field (tree field)
> +next_subobject_field (tree field)
>   {
>     while (field
>   	 && (TREE_CODE (field) != FIELD_DECL
>   	     || DECL_UNNAMED_BIT_FIELD (field)
>   	     || (DECL_ARTIFICIAL (field)
> -		 /* In C++17, don't skip base class fields.  */
> -		 && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field))
> -		 /* Don't skip vptr fields.  We might see them when we're
> -		    called from reduced_constant_expression_p.  */
> +		 && !DECL_FIELD_IS_BASE (field)
>   		 && !DECL_VIRTUAL_P (field))))
>       field = DECL_CHAIN (field);
>   
> @@ -6595,7 +6611,7 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
>     if (base_binfo)
>       field = base_binfo;
>     else
> -    field = next_initializable_field (TYPE_FIELDS (type));
> +    field = next_aggregate_field (TYPE_FIELDS (type));
>   
>     if (!field)
>       {
> @@ -6760,10 +6776,10 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
>   	  if (BINFO_BASE_ITERATE (binfo, ++binfo_idx, base_binfo))
>   	    field = base_binfo;
>   	  else
> -	    field = next_initializable_field (TYPE_FIELDS (type));
> +	    field = next_aggregate_field (TYPE_FIELDS (type));
>   	}
>         else
> -	field = next_initializable_field (DECL_CHAIN (field));
> +	field = next_aggregate_field (DECL_CHAIN (field));
>       }
>   
>     /* A trailing aggregate element that is a pack expansion is assumed to
> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> index 75ab965a218..f1ed9336dc9 100644
> --- a/gcc/cp/init.cc
> +++ b/gcc/cp/init.cc
> @@ -422,7 +422,7 @@ build_value_init_noctor (tree type, tsubst_flags_t complain)
>   		  && !COMPLETE_TYPE_P (ftype)
>   		  && !TYPE_DOMAIN (ftype)
>   		  && COMPLETE_TYPE_P (TREE_TYPE (ftype))
> -		  && (next_initializable_field (DECL_CHAIN (field))
> +		  && (next_aggregate_field (DECL_CHAIN (field))
>   		      == NULL_TREE))
>   		continue;
>   
> @@ -1477,9 +1477,9 @@ emit_mem_initializers (tree mem_inits)
>   
>     /* Initially that is all of them.  */
>     if (warn_uninitialized)
> -    for (tree f = next_initializable_field (TYPE_FIELDS (current_class_type));
> +    for (tree f = next_aggregate_field (TYPE_FIELDS (current_class_type));
>   	 f != NULL_TREE;
> -	 f = next_initializable_field (DECL_CHAIN (f)))
> +	 f = next_aggregate_field (DECL_CHAIN (f)))
>         if (!DECL_ARTIFICIAL (f)
>   	  && !is_really_empty_class (TREE_TYPE (f), /*ignore_vptr*/false))
>   	uninitialized.add (f);
> diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
> index afac53b6d7c..27704a578af 100644
> --- a/gcc/cp/lambda.cc
> +++ b/gcc/cp/lambda.cc
> @@ -425,9 +425,9 @@ build_capture_proxy (tree member, tree init)
>     if (DECL_VLA_CAPTURE_P (member))
>       {
>         /* Rebuild the VLA type from the pointer and maxindex.  */
> -      tree field = next_initializable_field (TYPE_FIELDS (type));
> +      tree field = next_aggregate_field (TYPE_FIELDS (type));
>         tree ptr = build_simple_component_ref (object, field);
> -      field = next_initializable_field (DECL_CHAIN (field));
> +      field = next_aggregate_field (DECL_CHAIN (field));
>         tree max = build_simple_component_ref (object, field);
>         type = build_cplus_array_type (TREE_TYPE (TREE_TYPE (ptr)),
>   				     build_index_type (max));
> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
> index 903ee666ef3..0dffd648b0b 100644
> --- a/gcc/cp/method.cc
> +++ b/gcc/cp/method.cc
> @@ -1465,7 +1465,7 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
>     /* A defaulted comparison operator function for class C is defined as
>        deleted if ... C has variant members.  */
>     if (TREE_CODE (ctype) == UNION_TYPE
> -      && next_initializable_field (TYPE_FIELDS (ctype)))
> +      && next_aggregate_field (TYPE_FIELDS (ctype)))
>       {
>         if (complain & tf_error)
>   	inform (info.loc, "cannot default compare union %qT", ctype);
> @@ -1518,9 +1518,9 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
>   	}
>   
>         /* Now compare the field subobjects.  */
> -      for (tree field = next_initializable_field (TYPE_FIELDS (ctype));
> +      for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
>   	   field;
> -	   field = next_initializable_field (DECL_CHAIN (field)))
> +	   field = next_aggregate_field (DECL_CHAIN (field)))
>   	{
>   	  if (DECL_VIRTUAL_P (field) || DECL_FIELD_IS_BASE (field))
>   	    /* We ignore the vptr, and we already handled bases.  */
> @@ -1542,7 +1542,7 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
>   	      continue;
>   	    }
>   	  else if (ANON_UNION_TYPE_P (expr_type)
> -		   && next_initializable_field (TYPE_FIELDS (expr_type)))
> +		   && next_aggregate_field (TYPE_FIELDS (expr_type)))
>   	    {
>   	      if (complain & tf_error)
>   		inform (field_loc, "cannot default compare "
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index ac002907a41..86e7212daa5 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -29578,7 +29578,7 @@ maybe_aggr_guide (tree tmpl, tree init, vec<tree,va_gc> *args)
>   	   len;
>   	   --len, field = DECL_CHAIN (field))
>   	{
> -	  field = next_initializable_field (field);
> +	  field = next_aggregate_field (field);
>   	  if (!field)
>   	    return NULL_TREE;
>   	  tree ftype = finish_decltype_type (field, true, complain);
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index ed0d0d22950..aba695116c5 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -4852,8 +4852,8 @@ structural_type_p (tree t, bool explain)
>   	explain_non_literal_class (t);
>         return false;
>       }
> -  for (tree m = next_initializable_field (TYPE_FIELDS (t)); m;
> -       m = next_initializable_field (DECL_CHAIN (m)))
> +  for (tree m = next_aggregate_field (TYPE_FIELDS (t)); m;
> +       m = next_aggregate_field (DECL_CHAIN (m)))
>       {
>         if (TREE_PRIVATE (m) || TREE_PROTECTED (m))
>   	{
> diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
> index 63d95c1529a..1d92310edd0 100644
> --- a/gcc/cp/typeck2.cc
> +++ b/gcc/cp/typeck2.cc
> @@ -606,7 +606,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool last,
>   				  : TYPE_FIELDS (type));
>   		     ; prev = DECL_CHAIN (prev))
>   		  {
> -		    prev = next_initializable_field (prev);
> +		    prev = next_aggregate_field (prev);
>   		    if (prev == field_index)
>   		      break;
>   		    tree ptype = TREE_TYPE (prev);
> @@ -1304,7 +1304,7 @@ digest_init_r (tree type, tree init, int nested, int flags,
>   	    the first element of d, which is the B base subobject.  The base
>   	    of type B is copy-initialized from the D temporary, causing
>   	    object slicing.  */
> -	  tree field = next_initializable_field (TYPE_FIELDS (type));
> +	  tree field = next_aggregate_field (TYPE_FIELDS (type));
>   	  if (field && DECL_FIELD_IS_BASE (field))
>   	    {
>   	      if (warning_at (loc, 0, "initializing a base class of type %qT "
> 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_;
  

On Sat, May 7, 2022 at 5:18 PM Jason Merrill <jason@redhat.com> wrote:
>
> On 5/6/22 16:46, Patrick Palka wrote:
> > On Fri, 6 May 2022, Jason Merrill wrote:
> >
> >> On 5/6/22 16:10, Patrick Palka wrote:
> >>> 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.
> >>
> >> Why clear the flag on the union before the inner ctor is fully initialized, if
> >> the intent is to prevent changing the active member during initialization?
> >>
> >> In the loop over ctors in cxx_eval_store_expression, I'd think if we encounter
> >> a CONSTRUCTOR_NO_CLEARING ctor along the way, we shouldn't clear the flag on
> >> an outer ctor.
> >
> > Wouldn't that prevent us from later activating a different member, which is
> > allowed in C++20?  It would cause us to reject e.g.
> >
> >      constexpr auto f() {
> >        union U {
> >          struct { int x, y; } a;
> >          int b;
> >        } u;
> >        u.a.x = 0;
> >        u.b = 0;
> >        return u;
> >      }
> >
> >      static_assert(f().b == 0);
> >
> > even in C++20 with
> >
> > <source>:7:7: error: change of the active member of a union from ‘f()::U::a’ to ‘f()::U::b’ during initialization
> >      7 |   u.b = 0;
> >        |   ~~~~^~~
> >
> > because when evaluating the second assignment we'd see that the
> > CONSTRUCTOR_NO_CLEARING flag is still set on the union while it already
> > has an activated member.
>
> Ah, makes sense.
>
> > On a related note, here's the patch that mechanically renames
> > next_initializable_field to next_aggregate_field and makes it skip over vptr
> > fields, and defines next_subobject_field alongside it for use by r_c_e_p.
> > Bootstrap and regtesting in progress.
>
> OK if successful.

Thanks, committed as r13-211-g0c7bce0ac184c0.  Would this patch be
suitable for backporting to 12?  It seems safe enough.  Alternatively
I guess we could backport the original workaround that tweaks
clear_no_implicit_zero.

>
> >
> > -- >8 --
> >
> >       PR c++/105491
> >
> > gcc/cp/ChangeLog:
> >
> >       * call.cc (field_in_pset): Adjust after next_initializable_field
> >       renaming.
> >       (build_aggr_conv): Likewise.
> >       (convert_like_internal): Likewise.
> >       (type_has_extended_temps): Likewise.
> >       * class.cc (default_init_uninitialized_part): Likewise.
> >       (finish_struct): Likewise.
> >       * constexpr.cc (cx_check_missing_mem_inits): Likewise.
> >       (reduced_constant_expression_p): Use next_subobject_field
> >       instead.
> >       * cp-gimplify.cc (get_source_location_impl_type): Adjust after
> >       next_initializable_field renaming.
> >       (fold_builtin_source_location): Likewise.
> >       * cp-tree.h (next_initializable_field): Rename to ...
> >       (next_aggregate_field): ... this.
> >       (next_subobject_field): Declare.
> >       * decl.cc (next_aggregate_field): Renamed from ...
> >       (next_initializable_field): ... this.  Skip over vptr fields.
> >       Document that this is now intended for aggregate classes.
> >       (next_subobject_field): Define.
> >       (reshape_init_class): Adjust after next_initializable_field
> >       renaming.
> >       * init.cc (build_value_init_noctor): Likewise.
> >       (emit_mem_initializers): Likewise.
> >       * lambda.cc (build_capture_proxy): Likewise.
> >       * method.cc (build_comparison_op): Likewise.
> >       * pt.cc (maybe_aggr_guide): Likewise.
> >       * tree.cc (structural_type_p): Likewise.
> >       * typeck2.cc (split_nonconstant_init_1): Likewise.
> >       (digest_init_r): Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C: New test.
> >       * gcc/testsuite/g++.dg/cpp0x/constexpr-union7a.C: New test.
> >       * gcc/testsuite/g++.dg/cpp2a/constinit17.C: New test.
> > ---
> >   gcc/cp/call.cc                                | 14 +++----
> >   gcc/cp/class.cc                               |  8 ++--
> >   gcc/cp/constexpr.cc                           | 10 ++---
> >   gcc/cp/cp-gimplify.cc                         |  4 +-
> >   gcc/cp/cp-tree.h                              |  3 +-
> >   gcc/cp/decl.cc                                | 38 +++++++++++++------
> >   gcc/cp/init.cc                                |  6 +--
> >   gcc/cp/lambda.cc                              |  4 +-
> >   gcc/cp/method.cc                              |  8 ++--
> >   gcc/cp/pt.cc                                  |  2 +-
> >   gcc/cp/tree.cc                                |  4 +-
> >   gcc/cp/typeck2.cc                             |  4 +-
> >   gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C | 17 +++++++++
> >   .../g++.dg/cpp0x/constexpr-union7a.C          | 15 ++++++++
> >   gcc/testsuite/g++.dg/cpp2a/constinit17.C      | 24 ++++++++++++
> >   15 files changed, 117 insertions(+), 44 deletions(-)
> >   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/call.cc b/gcc/cp/call.cc
> > index 959279d6216..0240e364324 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -948,7 +948,7 @@ field_in_pset (hash_set<tree, true> &pset, tree field)
> >       for (field = TYPE_FIELDS (TREE_TYPE (field));
> >        field; field = DECL_CHAIN (field))
> >         {
> > -     field = next_initializable_field (field);
> > +     field = next_aggregate_field (field);
> >       if (field == NULL_TREE)
> >         break;
> >       if (field_in_pset (pset, field))
> > @@ -965,7 +965,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
> >   {
> >     unsigned HOST_WIDE_INT i = 0;
> >     conversion *c;
> > -  tree field = next_initializable_field (TYPE_FIELDS (type));
> > +  tree field = next_aggregate_field (TYPE_FIELDS (type));
> >     tree empty_ctor = NULL_TREE;
> >     hash_set<tree, true> pset;
> >
> > @@ -1011,7 +1011,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
> >       }
> >       }
> >
> > -  for (; field; field = next_initializable_field (DECL_CHAIN (field)))
> > +  for (; field; field = next_aggregate_field (DECL_CHAIN (field)))
> >       {
> >         tree ftype = TREE_TYPE (field);
> >         tree val;
> > @@ -8098,10 +8098,10 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
> >       totype = complete_type_or_maybe_complain (totype, NULL_TREE, complain);
> >       if (!totype)
> >         return error_mark_node;
> > -     tree field = next_initializable_field (TYPE_FIELDS (totype));
> > +     tree field = next_aggregate_field (TYPE_FIELDS (totype));
> >       vec<constructor_elt, va_gc> *vec = NULL;
> >       CONSTRUCTOR_APPEND_ELT (vec, field, array);
> > -     field = next_initializable_field (DECL_CHAIN (field));
> > +     field = next_aggregate_field (DECL_CHAIN (field));
> >       CONSTRUCTOR_APPEND_ELT (vec, field, size_int (len));
> >       tree new_ctor = build_constructor (totype, vec);
> >       return get_target_expr_sfinae (new_ctor, complain);
> > @@ -13267,8 +13267,8 @@ type_has_extended_temps (tree type)
> >       {
> >         if (is_std_init_list (type))
> >       return true;
> > -      for (tree f = next_initializable_field (TYPE_FIELDS (type));
> > -        f; f = next_initializable_field (DECL_CHAIN (f)))
> > +      for (tree f = next_aggregate_field (TYPE_FIELDS (type));
> > +        f; f = next_aggregate_field (DECL_CHAIN (f)))
> >       if (type_has_extended_temps (TREE_TYPE (f)))
> >         return true;
> >       }
> > diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
> > index bc94ed45e17..3c195b35396 100644
> > --- a/gcc/cp/class.cc
> > +++ b/gcc/cp/class.cc
> > @@ -5494,8 +5494,8 @@ default_init_uninitialized_part (tree type)
> >         if (r)
> >       return r;
> >       }
> > -  for (t = next_initializable_field (TYPE_FIELDS (type)); t;
> > -       t = next_initializable_field (DECL_CHAIN (t)))
> > +  for (t = next_aggregate_field (TYPE_FIELDS (type)); t;
> > +       t = next_aggregate_field (DECL_CHAIN (t)))
> >       if (!DECL_INITIAL (t) && !DECL_ARTIFICIAL (t))
> >         {
> >       r = default_init_uninitialized_part (TREE_TYPE (t));
> > @@ -7781,10 +7781,10 @@ finish_struct (tree t, tree attributes)
> >         bool ok = false;
> >         if (processing_template_decl)
> >       {
> > -       tree f = next_initializable_field (TYPE_FIELDS (t));
> > +       tree f = next_aggregate_field (TYPE_FIELDS (t));
> >         if (f && TYPE_PTR_P (TREE_TYPE (f)))
> >           {
> > -           f = next_initializable_field (DECL_CHAIN (f));
> > +           f = next_aggregate_field (DECL_CHAIN (f));
> >             if (f && same_type_p (TREE_TYPE (f), size_type_node))
> >               ok = true;
> >           }
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 9b1e71857fc..24717b4f8d5 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -784,7 +784,7 @@ cx_check_missing_mem_inits (tree ctype, tree body, bool complain)
> >
> >     if (TREE_CODE (ctype) == UNION_TYPE)
> >       {
> > -      if (nelts == 0 && next_initializable_field (field))
> > +      if (nelts == 0 && next_aggregate_field (field))
> >       {
> >         if (complain)
> >           error ("%<constexpr%> constructor for union %qT must "
> > @@ -3053,7 +3053,7 @@ reduced_constant_expression_p (tree t)
> >             field = NULL_TREE;
> >           }
> >         else
> > -         field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
> > +         field = next_subobject_field (TYPE_FIELDS (TREE_TYPE (t)));
> >       }
> >         else
> >       field = NULL_TREE;
> > @@ -3065,15 +3065,15 @@ reduced_constant_expression_p (tree t)
> >           return false;
> >         /* Empty class field may or may not have an initializer.  */
> >         for (; field && e.index != field;
> > -            field = next_initializable_field (DECL_CHAIN (field)))
> > +            field = next_subobject_field (DECL_CHAIN (field)))
> >           if (!is_really_empty_class (TREE_TYPE (field),
> >                                       /*ignore_vptr*/false))
> >             return false;
> >         if (field)
> > -         field = next_initializable_field (DECL_CHAIN (field));
> > +         field = next_subobject_field (DECL_CHAIN (field));
> >       }
> >         /* There could be a non-empty field at the end.  */
> > -      for (; field; field = next_initializable_field (DECL_CHAIN (field)))
> > +      for (; field; field = next_subobject_field (DECL_CHAIN (field)))
> >       if (!is_really_empty_class (TREE_TYPE (field), /*ignore_vptr*/false))
> >         return false;
> >   ok:
> > diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
> > index b52d9cb5754..ee47a4bfd37 100644
> > --- a/gcc/cp/cp-gimplify.cc
> > +++ b/gcc/cp/cp-gimplify.cc
> > @@ -3106,7 +3106,7 @@ get_source_location_impl_type (location_t loc)
> >
> >     int cnt = 0;
> >     for (tree field = TYPE_FIELDS (type);
> > -       (field = next_initializable_field (field)) != NULL_TREE;
> > +       (field = next_aggregate_field (field)) != NULL_TREE;
> >          field = DECL_CHAIN (field))
> >       {
> >         if (DECL_NAME (field) != NULL_TREE)
> > @@ -3281,7 +3281,7 @@ fold_builtin_source_location (location_t loc)
> >         vec<constructor_elt, va_gc> *v = NULL;
> >         vec_alloc (v, 4);
> >         for (tree field = TYPE_FIELDS (source_location_impl);
> > -        (field = next_initializable_field (field)) != NULL_TREE;
> > +        (field = next_aggregate_field (field)) != NULL_TREE;
> >          field = DECL_CHAIN (field))
> >       {
> >         const char *n = IDENTIFIER_POINTER (DECL_NAME (field));
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 663fe7a20fc..abcef7c20c4 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -6870,7 +6870,8 @@ extern bool is_direct_enum_init                 (tree, tree);
> >   extern void initialize_artificial_var               (tree, vec<constructor_elt, va_gc> *);
> >   extern tree check_var_type                  (tree, tree, location_t);
> >   extern tree reshape_init                        (tree, tree, tsubst_flags_t);
> > -extern tree next_initializable_field (tree);
> > +extern tree next_aggregate_field             (tree);
> > +extern tree next_subobject_field             (tree);
> >   extern tree first_field                             (const_tree);
> >   extern tree fndecl_declared_return_type             (tree);
> >   extern bool undeduced_auto_decl                     (tree);
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index c9110db796a..d5ab2d0ad87 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -6382,22 +6382,38 @@ struct reshape_iter
> >
> >   static tree reshape_init_r (tree, reshape_iter *, tree, tsubst_flags_t);
> >
> > +/* FIELD is an element of TYPE_FIELDS of an aggregate class, or NULL.  In the
> > +   former case, the value returned is the next FIELD_DECL (possibly FIELD
> > +   itself) that can be initialized.  If there are no more such fields, the
> > +   return value will be NULL.  */
> > +
> > +tree
> > +next_aggregate_field (tree field)
> > +{
> > +  while (field
> > +      && (TREE_CODE (field) != FIELD_DECL
> > +          || DECL_UNNAMED_BIT_FIELD (field)
> > +          || (DECL_ARTIFICIAL (field)
> > +              /* In C++17, aggregates can have base classes.  */
> > +              && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field)))))
> > +    field = DECL_CHAIN (field);
> > +
> > +  return field;
> > +}
> > +
> >   /* FIELD is an element of TYPE_FIELDS or NULL.  In the former case, the value
> > -   returned is the next FIELD_DECL (possibly FIELD itself) that can be
> > -   initialized.  If there are no more such fields, the return value
> > -   will be NULL.  */
> > +   returned is the next FIELD_DECL (possibly FIELD itself) that corresponds
> > +   to a subobject.  If there are no more such fields, the return value will be
> > +   NULL.  */
> >
> >   tree
> > -next_initializable_field (tree field)
> > +next_subobject_field (tree field)
> >   {
> >     while (field
> >        && (TREE_CODE (field) != FIELD_DECL
> >            || DECL_UNNAMED_BIT_FIELD (field)
> >            || (DECL_ARTIFICIAL (field)
> > -              /* In C++17, don't skip base class fields.  */
> > -              && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field))
> > -              /* Don't skip vptr fields.  We might see them when we're
> > -                 called from reduced_constant_expression_p.  */
> > +              && !DECL_FIELD_IS_BASE (field)
> >                && !DECL_VIRTUAL_P (field))))
> >       field = DECL_CHAIN (field);
> >
> > @@ -6595,7 +6611,7 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
> >     if (base_binfo)
> >       field = base_binfo;
> >     else
> > -    field = next_initializable_field (TYPE_FIELDS (type));
> > +    field = next_aggregate_field (TYPE_FIELDS (type));
> >
> >     if (!field)
> >       {
> > @@ -6760,10 +6776,10 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
> >         if (BINFO_BASE_ITERATE (binfo, ++binfo_idx, base_binfo))
> >           field = base_binfo;
> >         else
> > -         field = next_initializable_field (TYPE_FIELDS (type));
> > +         field = next_aggregate_field (TYPE_FIELDS (type));
> >       }
> >         else
> > -     field = next_initializable_field (DECL_CHAIN (field));
> > +     field = next_aggregate_field (DECL_CHAIN (field));
> >       }
> >
> >     /* A trailing aggregate element that is a pack expansion is assumed to
> > diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> > index 75ab965a218..f1ed9336dc9 100644
> > --- a/gcc/cp/init.cc
> > +++ b/gcc/cp/init.cc
> > @@ -422,7 +422,7 @@ build_value_init_noctor (tree type, tsubst_flags_t complain)
> >                 && !COMPLETE_TYPE_P (ftype)
> >                 && !TYPE_DOMAIN (ftype)
> >                 && COMPLETE_TYPE_P (TREE_TYPE (ftype))
> > -               && (next_initializable_field (DECL_CHAIN (field))
> > +               && (next_aggregate_field (DECL_CHAIN (field))
> >                     == NULL_TREE))
> >               continue;
> >
> > @@ -1477,9 +1477,9 @@ emit_mem_initializers (tree mem_inits)
> >
> >     /* Initially that is all of them.  */
> >     if (warn_uninitialized)
> > -    for (tree f = next_initializable_field (TYPE_FIELDS (current_class_type));
> > +    for (tree f = next_aggregate_field (TYPE_FIELDS (current_class_type));
> >        f != NULL_TREE;
> > -      f = next_initializable_field (DECL_CHAIN (f)))
> > +      f = next_aggregate_field (DECL_CHAIN (f)))
> >         if (!DECL_ARTIFICIAL (f)
> >         && !is_really_empty_class (TREE_TYPE (f), /*ignore_vptr*/false))
> >       uninitialized.add (f);
> > diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
> > index afac53b6d7c..27704a578af 100644
> > --- a/gcc/cp/lambda.cc
> > +++ b/gcc/cp/lambda.cc
> > @@ -425,9 +425,9 @@ build_capture_proxy (tree member, tree init)
> >     if (DECL_VLA_CAPTURE_P (member))
> >       {
> >         /* Rebuild the VLA type from the pointer and maxindex.  */
> > -      tree field = next_initializable_field (TYPE_FIELDS (type));
> > +      tree field = next_aggregate_field (TYPE_FIELDS (type));
> >         tree ptr = build_simple_component_ref (object, field);
> > -      field = next_initializable_field (DECL_CHAIN (field));
> > +      field = next_aggregate_field (DECL_CHAIN (field));
> >         tree max = build_simple_component_ref (object, field);
> >         type = build_cplus_array_type (TREE_TYPE (TREE_TYPE (ptr)),
> >                                    build_index_type (max));
> > diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
> > index 903ee666ef3..0dffd648b0b 100644
> > --- a/gcc/cp/method.cc
> > +++ b/gcc/cp/method.cc
> > @@ -1465,7 +1465,7 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
> >     /* A defaulted comparison operator function for class C is defined as
> >        deleted if ... C has variant members.  */
> >     if (TREE_CODE (ctype) == UNION_TYPE
> > -      && next_initializable_field (TYPE_FIELDS (ctype)))
> > +      && next_aggregate_field (TYPE_FIELDS (ctype)))
> >       {
> >         if (complain & tf_error)
> >       inform (info.loc, "cannot default compare union %qT", ctype);
> > @@ -1518,9 +1518,9 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
> >       }
> >
> >         /* Now compare the field subobjects.  */
> > -      for (tree field = next_initializable_field (TYPE_FIELDS (ctype));
> > +      for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
> >          field;
> > -        field = next_initializable_field (DECL_CHAIN (field)))
> > +        field = next_aggregate_field (DECL_CHAIN (field)))
> >       {
> >         if (DECL_VIRTUAL_P (field) || DECL_FIELD_IS_BASE (field))
> >           /* We ignore the vptr, and we already handled bases.  */
> > @@ -1542,7 +1542,7 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
> >             continue;
> >           }
> >         else if (ANON_UNION_TYPE_P (expr_type)
> > -                && next_initializable_field (TYPE_FIELDS (expr_type)))
> > +                && next_aggregate_field (TYPE_FIELDS (expr_type)))
> >           {
> >             if (complain & tf_error)
> >               inform (field_loc, "cannot default compare "
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index ac002907a41..86e7212daa5 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -29578,7 +29578,7 @@ maybe_aggr_guide (tree tmpl, tree init, vec<tree,va_gc> *args)
> >          len;
> >          --len, field = DECL_CHAIN (field))
> >       {
> > -       field = next_initializable_field (field);
> > +       field = next_aggregate_field (field);
> >         if (!field)
> >           return NULL_TREE;
> >         tree ftype = finish_decltype_type (field, true, complain);
> > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > index ed0d0d22950..aba695116c5 100644
> > --- a/gcc/cp/tree.cc
> > +++ b/gcc/cp/tree.cc
> > @@ -4852,8 +4852,8 @@ structural_type_p (tree t, bool explain)
> >       explain_non_literal_class (t);
> >         return false;
> >       }
> > -  for (tree m = next_initializable_field (TYPE_FIELDS (t)); m;
> > -       m = next_initializable_field (DECL_CHAIN (m)))
> > +  for (tree m = next_aggregate_field (TYPE_FIELDS (t)); m;
> > +       m = next_aggregate_field (DECL_CHAIN (m)))
> >       {
> >         if (TREE_PRIVATE (m) || TREE_PROTECTED (m))
> >       {
> > diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
> > index 63d95c1529a..1d92310edd0 100644
> > --- a/gcc/cp/typeck2.cc
> > +++ b/gcc/cp/typeck2.cc
> > @@ -606,7 +606,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool last,
> >                                 : TYPE_FIELDS (type));
> >                    ; prev = DECL_CHAIN (prev))
> >                 {
> > -                 prev = next_initializable_field (prev);
> > +                 prev = next_aggregate_field (prev);
> >                   if (prev == field_index)
> >                     break;
> >                   tree ptype = TREE_TYPE (prev);
> > @@ -1304,7 +1304,7 @@ digest_init_r (tree type, tree init, int nested, int flags,
> >           the first element of d, which is the B base subobject.  The base
> >           of type B is copy-initialized from the D temporary, causing
> >           object slicing.  */
> > -       tree field = next_initializable_field (TYPE_FIELDS (type));
> > +       tree field = next_aggregate_field (TYPE_FIELDS (type));
> >         if (field && DECL_FIELD_IS_BASE (field))
> >           {
> >             if (warning_at (loc, 0, "initializing a base class of type %qT "
> > 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_;
>
  

On 5/17/22 12:34, Patrick Palka wrote:
> On Sat, May 7, 2022 at 5:18 PM Jason Merrill <jason@redhat.com> wrote:
>>
>> On 5/6/22 16:46, Patrick Palka wrote:
>>> On Fri, 6 May 2022, Jason Merrill wrote:
>>>
>>>> On 5/6/22 16:10, Patrick Palka wrote:
>>>>> 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.
>>>>
>>>> Why clear the flag on the union before the inner ctor is fully initialized, if
>>>> the intent is to prevent changing the active member during initialization?
>>>>
>>>> In the loop over ctors in cxx_eval_store_expression, I'd think if we encounter
>>>> a CONSTRUCTOR_NO_CLEARING ctor along the way, we shouldn't clear the flag on
>>>> an outer ctor.
>>>
>>> Wouldn't that prevent us from later activating a different member, which is
>>> allowed in C++20?  It would cause us to reject e.g.
>>>
>>>       constexpr auto f() {
>>>         union U {
>>>           struct { int x, y; } a;
>>>           int b;
>>>         } u;
>>>         u.a.x = 0;
>>>         u.b = 0;
>>>         return u;
>>>       }
>>>
>>>       static_assert(f().b == 0);
>>>
>>> even in C++20 with
>>>
>>> <source>:7:7: error: change of the active member of a union from ‘f()::U::a’ to ‘f()::U::b’ during initialization
>>>       7 |   u.b = 0;
>>>         |   ~~~~^~~
>>>
>>> because when evaluating the second assignment we'd see that the
>>> CONSTRUCTOR_NO_CLEARING flag is still set on the union while it already
>>> has an activated member.
>>
>> Ah, makes sense.
>>
>>> On a related note, here's the patch that mechanically renames
>>> next_initializable_field to next_aggregate_field and makes it skip over vptr
>>> fields, and defines next_subobject_field alongside it for use by r_c_e_p.
>>> Bootstrap and regtesting in progress.
>>
>> OK if successful.
> 
> Thanks, committed as r13-211-g0c7bce0ac184c0.  Would this patch be
> suitable for backporting to 12?  It seems safe enough.  Alternatively
> I guess we could backport the original workaround that tweaks
> clear_no_implicit_zero.

Maybe for 12 just add next_subobject_field without touching any other 
users of next_initializable_field?

>>> -- >8 --
>>>
>>>        PR c++/105491
>>>
>>> gcc/cp/ChangeLog:
>>>
>>>        * call.cc (field_in_pset): Adjust after next_initializable_field
>>>        renaming.
>>>        (build_aggr_conv): Likewise.
>>>        (convert_like_internal): Likewise.
>>>        (type_has_extended_temps): Likewise.
>>>        * class.cc (default_init_uninitialized_part): Likewise.
>>>        (finish_struct): Likewise.
>>>        * constexpr.cc (cx_check_missing_mem_inits): Likewise.
>>>        (reduced_constant_expression_p): Use next_subobject_field
>>>        instead.
>>>        * cp-gimplify.cc (get_source_location_impl_type): Adjust after
>>>        next_initializable_field renaming.
>>>        (fold_builtin_source_location): Likewise.
>>>        * cp-tree.h (next_initializable_field): Rename to ...
>>>        (next_aggregate_field): ... this.
>>>        (next_subobject_field): Declare.
>>>        * decl.cc (next_aggregate_field): Renamed from ...
>>>        (next_initializable_field): ... this.  Skip over vptr fields.
>>>        Document that this is now intended for aggregate classes.
>>>        (next_subobject_field): Define.
>>>        (reshape_init_class): Adjust after next_initializable_field
>>>        renaming.
>>>        * init.cc (build_value_init_noctor): Likewise.
>>>        (emit_mem_initializers): Likewise.
>>>        * lambda.cc (build_capture_proxy): Likewise.
>>>        * method.cc (build_comparison_op): Likewise.
>>>        * pt.cc (maybe_aggr_guide): Likewise.
>>>        * tree.cc (structural_type_p): Likewise.
>>>        * typeck2.cc (split_nonconstant_init_1): Likewise.
>>>        (digest_init_r): Likewise.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>        * gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C: New test.
>>>        * gcc/testsuite/g++.dg/cpp0x/constexpr-union7a.C: New test.
>>>        * gcc/testsuite/g++.dg/cpp2a/constinit17.C: New test.
>>> ---
>>>    gcc/cp/call.cc                                | 14 +++----
>>>    gcc/cp/class.cc                               |  8 ++--
>>>    gcc/cp/constexpr.cc                           | 10 ++---
>>>    gcc/cp/cp-gimplify.cc                         |  4 +-
>>>    gcc/cp/cp-tree.h                              |  3 +-
>>>    gcc/cp/decl.cc                                | 38 +++++++++++++------
>>>    gcc/cp/init.cc                                |  6 +--
>>>    gcc/cp/lambda.cc                              |  4 +-
>>>    gcc/cp/method.cc                              |  8 ++--
>>>    gcc/cp/pt.cc                                  |  2 +-
>>>    gcc/cp/tree.cc                                |  4 +-
>>>    gcc/cp/typeck2.cc                             |  4 +-
>>>    gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C | 17 +++++++++
>>>    .../g++.dg/cpp0x/constexpr-union7a.C          | 15 ++++++++
>>>    gcc/testsuite/g++.dg/cpp2a/constinit17.C      | 24 ++++++++++++
>>>    15 files changed, 117 insertions(+), 44 deletions(-)
>>>    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/call.cc b/gcc/cp/call.cc
>>> index 959279d6216..0240e364324 100644
>>> --- a/gcc/cp/call.cc
>>> +++ b/gcc/cp/call.cc
>>> @@ -948,7 +948,7 @@ field_in_pset (hash_set<tree, true> &pset, tree field)
>>>        for (field = TYPE_FIELDS (TREE_TYPE (field));
>>>         field; field = DECL_CHAIN (field))
>>>          {
>>> -     field = next_initializable_field (field);
>>> +     field = next_aggregate_field (field);
>>>        if (field == NULL_TREE)
>>>          break;
>>>        if (field_in_pset (pset, field))
>>> @@ -965,7 +965,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>>>    {
>>>      unsigned HOST_WIDE_INT i = 0;
>>>      conversion *c;
>>> -  tree field = next_initializable_field (TYPE_FIELDS (type));
>>> +  tree field = next_aggregate_field (TYPE_FIELDS (type));
>>>      tree empty_ctor = NULL_TREE;
>>>      hash_set<tree, true> pset;
>>>
>>> @@ -1011,7 +1011,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>>>        }
>>>        }
>>>
>>> -  for (; field; field = next_initializable_field (DECL_CHAIN (field)))
>>> +  for (; field; field = next_aggregate_field (DECL_CHAIN (field)))
>>>        {
>>>          tree ftype = TREE_TYPE (field);
>>>          tree val;
>>> @@ -8098,10 +8098,10 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
>>>        totype = complete_type_or_maybe_complain (totype, NULL_TREE, complain);
>>>        if (!totype)
>>>          return error_mark_node;
>>> -     tree field = next_initializable_field (TYPE_FIELDS (totype));
>>> +     tree field = next_aggregate_field (TYPE_FIELDS (totype));
>>>        vec<constructor_elt, va_gc> *vec = NULL;
>>>        CONSTRUCTOR_APPEND_ELT (vec, field, array);
>>> -     field = next_initializable_field (DECL_CHAIN (field));
>>> +     field = next_aggregate_field (DECL_CHAIN (field));
>>>        CONSTRUCTOR_APPEND_ELT (vec, field, size_int (len));
>>>        tree new_ctor = build_constructor (totype, vec);
>>>        return get_target_expr_sfinae (new_ctor, complain);
>>> @@ -13267,8 +13267,8 @@ type_has_extended_temps (tree type)
>>>        {
>>>          if (is_std_init_list (type))
>>>        return true;
>>> -      for (tree f = next_initializable_field (TYPE_FIELDS (type));
>>> -        f; f = next_initializable_field (DECL_CHAIN (f)))
>>> +      for (tree f = next_aggregate_field (TYPE_FIELDS (type));
>>> +        f; f = next_aggregate_field (DECL_CHAIN (f)))
>>>        if (type_has_extended_temps (TREE_TYPE (f)))
>>>          return true;
>>>        }
>>> diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
>>> index bc94ed45e17..3c195b35396 100644
>>> --- a/gcc/cp/class.cc
>>> +++ b/gcc/cp/class.cc
>>> @@ -5494,8 +5494,8 @@ default_init_uninitialized_part (tree type)
>>>          if (r)
>>>        return r;
>>>        }
>>> -  for (t = next_initializable_field (TYPE_FIELDS (type)); t;
>>> -       t = next_initializable_field (DECL_CHAIN (t)))
>>> +  for (t = next_aggregate_field (TYPE_FIELDS (type)); t;
>>> +       t = next_aggregate_field (DECL_CHAIN (t)))
>>>        if (!DECL_INITIAL (t) && !DECL_ARTIFICIAL (t))
>>>          {
>>>        r = default_init_uninitialized_part (TREE_TYPE (t));
>>> @@ -7781,10 +7781,10 @@ finish_struct (tree t, tree attributes)
>>>          bool ok = false;
>>>          if (processing_template_decl)
>>>        {
>>> -       tree f = next_initializable_field (TYPE_FIELDS (t));
>>> +       tree f = next_aggregate_field (TYPE_FIELDS (t));
>>>          if (f && TYPE_PTR_P (TREE_TYPE (f)))
>>>            {
>>> -           f = next_initializable_field (DECL_CHAIN (f));
>>> +           f = next_aggregate_field (DECL_CHAIN (f));
>>>              if (f && same_type_p (TREE_TYPE (f), size_type_node))
>>>                ok = true;
>>>            }
>>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>>> index 9b1e71857fc..24717b4f8d5 100644
>>> --- a/gcc/cp/constexpr.cc
>>> +++ b/gcc/cp/constexpr.cc
>>> @@ -784,7 +784,7 @@ cx_check_missing_mem_inits (tree ctype, tree body, bool complain)
>>>
>>>      if (TREE_CODE (ctype) == UNION_TYPE)
>>>        {
>>> -      if (nelts == 0 && next_initializable_field (field))
>>> +      if (nelts == 0 && next_aggregate_field (field))
>>>        {
>>>          if (complain)
>>>            error ("%<constexpr%> constructor for union %qT must "
>>> @@ -3053,7 +3053,7 @@ reduced_constant_expression_p (tree t)
>>>              field = NULL_TREE;
>>>            }
>>>          else
>>> -         field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
>>> +         field = next_subobject_field (TYPE_FIELDS (TREE_TYPE (t)));
>>>        }
>>>          else
>>>        field = NULL_TREE;
>>> @@ -3065,15 +3065,15 @@ reduced_constant_expression_p (tree t)
>>>            return false;
>>>          /* Empty class field may or may not have an initializer.  */
>>>          for (; field && e.index != field;
>>> -            field = next_initializable_field (DECL_CHAIN (field)))
>>> +            field = next_subobject_field (DECL_CHAIN (field)))
>>>            if (!is_really_empty_class (TREE_TYPE (field),
>>>                                        /*ignore_vptr*/false))
>>>              return false;
>>>          if (field)
>>> -         field = next_initializable_field (DECL_CHAIN (field));
>>> +         field = next_subobject_field (DECL_CHAIN (field));
>>>        }
>>>          /* There could be a non-empty field at the end.  */
>>> -      for (; field; field = next_initializable_field (DECL_CHAIN (field)))
>>> +      for (; field; field = next_subobject_field (DECL_CHAIN (field)))
>>>        if (!is_really_empty_class (TREE_TYPE (field), /*ignore_vptr*/false))
>>>          return false;
>>>    ok:
>>> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
>>> index b52d9cb5754..ee47a4bfd37 100644
>>> --- a/gcc/cp/cp-gimplify.cc
>>> +++ b/gcc/cp/cp-gimplify.cc
>>> @@ -3106,7 +3106,7 @@ get_source_location_impl_type (location_t loc)
>>>
>>>      int cnt = 0;
>>>      for (tree field = TYPE_FIELDS (type);
>>> -       (field = next_initializable_field (field)) != NULL_TREE;
>>> +       (field = next_aggregate_field (field)) != NULL_TREE;
>>>           field = DECL_CHAIN (field))
>>>        {
>>>          if (DECL_NAME (field) != NULL_TREE)
>>> @@ -3281,7 +3281,7 @@ fold_builtin_source_location (location_t loc)
>>>          vec<constructor_elt, va_gc> *v = NULL;
>>>          vec_alloc (v, 4);
>>>          for (tree field = TYPE_FIELDS (source_location_impl);
>>> -        (field = next_initializable_field (field)) != NULL_TREE;
>>> +        (field = next_aggregate_field (field)) != NULL_TREE;
>>>           field = DECL_CHAIN (field))
>>>        {
>>>          const char *n = IDENTIFIER_POINTER (DECL_NAME (field));
>>> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>>> index 663fe7a20fc..abcef7c20c4 100644
>>> --- a/gcc/cp/cp-tree.h
>>> +++ b/gcc/cp/cp-tree.h
>>> @@ -6870,7 +6870,8 @@ extern bool is_direct_enum_init                 (tree, tree);
>>>    extern void initialize_artificial_var               (tree, vec<constructor_elt, va_gc> *);
>>>    extern tree check_var_type                  (tree, tree, location_t);
>>>    extern tree reshape_init                        (tree, tree, tsubst_flags_t);
>>> -extern tree next_initializable_field (tree);
>>> +extern tree next_aggregate_field             (tree);
>>> +extern tree next_subobject_field             (tree);
>>>    extern tree first_field                             (const_tree);
>>>    extern tree fndecl_declared_return_type             (tree);
>>>    extern bool undeduced_auto_decl                     (tree);
>>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>>> index c9110db796a..d5ab2d0ad87 100644
>>> --- a/gcc/cp/decl.cc
>>> +++ b/gcc/cp/decl.cc
>>> @@ -6382,22 +6382,38 @@ struct reshape_iter
>>>
>>>    static tree reshape_init_r (tree, reshape_iter *, tree, tsubst_flags_t);
>>>
>>> +/* FIELD is an element of TYPE_FIELDS of an aggregate class, or NULL.  In the
>>> +   former case, the value returned is the next FIELD_DECL (possibly FIELD
>>> +   itself) that can be initialized.  If there are no more such fields, the
>>> +   return value will be NULL.  */
>>> +
>>> +tree
>>> +next_aggregate_field (tree field)
>>> +{
>>> +  while (field
>>> +      && (TREE_CODE (field) != FIELD_DECL
>>> +          || DECL_UNNAMED_BIT_FIELD (field)
>>> +          || (DECL_ARTIFICIAL (field)
>>> +              /* In C++17, aggregates can have base classes.  */
>>> +              && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field)))))
>>> +    field = DECL_CHAIN (field);
>>> +
>>> +  return field;
>>> +}
>>> +
>>>    /* FIELD is an element of TYPE_FIELDS or NULL.  In the former case, the value
>>> -   returned is the next FIELD_DECL (possibly FIELD itself) that can be
>>> -   initialized.  If there are no more such fields, the return value
>>> -   will be NULL.  */
>>> +   returned is the next FIELD_DECL (possibly FIELD itself) that corresponds
>>> +   to a subobject.  If there are no more such fields, the return value will be
>>> +   NULL.  */
>>>
>>>    tree
>>> -next_initializable_field (tree field)
>>> +next_subobject_field (tree field)
>>>    {
>>>      while (field
>>>         && (TREE_CODE (field) != FIELD_DECL
>>>             || DECL_UNNAMED_BIT_FIELD (field)
>>>             || (DECL_ARTIFICIAL (field)
>>> -              /* In C++17, don't skip base class fields.  */
>>> -              && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field))
>>> -              /* Don't skip vptr fields.  We might see them when we're
>>> -                 called from reduced_constant_expression_p.  */
>>> +              && !DECL_FIELD_IS_BASE (field)
>>>                 && !DECL_VIRTUAL_P (field))))
>>>        field = DECL_CHAIN (field);
>>>
>>> @@ -6595,7 +6611,7 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
>>>      if (base_binfo)
>>>        field = base_binfo;
>>>      else
>>> -    field = next_initializable_field (TYPE_FIELDS (type));
>>> +    field = next_aggregate_field (TYPE_FIELDS (type));
>>>
>>>      if (!field)
>>>        {
>>> @@ -6760,10 +6776,10 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
>>>          if (BINFO_BASE_ITERATE (binfo, ++binfo_idx, base_binfo))
>>>            field = base_binfo;
>>>          else
>>> -         field = next_initializable_field (TYPE_FIELDS (type));
>>> +         field = next_aggregate_field (TYPE_FIELDS (type));
>>>        }
>>>          else
>>> -     field = next_initializable_field (DECL_CHAIN (field));
>>> +     field = next_aggregate_field (DECL_CHAIN (field));
>>>        }
>>>
>>>      /* A trailing aggregate element that is a pack expansion is assumed to
>>> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
>>> index 75ab965a218..f1ed9336dc9 100644
>>> --- a/gcc/cp/init.cc
>>> +++ b/gcc/cp/init.cc
>>> @@ -422,7 +422,7 @@ build_value_init_noctor (tree type, tsubst_flags_t complain)
>>>                  && !COMPLETE_TYPE_P (ftype)
>>>                  && !TYPE_DOMAIN (ftype)
>>>                  && COMPLETE_TYPE_P (TREE_TYPE (ftype))
>>> -               && (next_initializable_field (DECL_CHAIN (field))
>>> +               && (next_aggregate_field (DECL_CHAIN (field))
>>>                      == NULL_TREE))
>>>                continue;
>>>
>>> @@ -1477,9 +1477,9 @@ emit_mem_initializers (tree mem_inits)
>>>
>>>      /* Initially that is all of them.  */
>>>      if (warn_uninitialized)
>>> -    for (tree f = next_initializable_field (TYPE_FIELDS (current_class_type));
>>> +    for (tree f = next_aggregate_field (TYPE_FIELDS (current_class_type));
>>>         f != NULL_TREE;
>>> -      f = next_initializable_field (DECL_CHAIN (f)))
>>> +      f = next_aggregate_field (DECL_CHAIN (f)))
>>>          if (!DECL_ARTIFICIAL (f)
>>>          && !is_really_empty_class (TREE_TYPE (f), /*ignore_vptr*/false))
>>>        uninitialized.add (f);
>>> diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
>>> index afac53b6d7c..27704a578af 100644
>>> --- a/gcc/cp/lambda.cc
>>> +++ b/gcc/cp/lambda.cc
>>> @@ -425,9 +425,9 @@ build_capture_proxy (tree member, tree init)
>>>      if (DECL_VLA_CAPTURE_P (member))
>>>        {
>>>          /* Rebuild the VLA type from the pointer and maxindex.  */
>>> -      tree field = next_initializable_field (TYPE_FIELDS (type));
>>> +      tree field = next_aggregate_field (TYPE_FIELDS (type));
>>>          tree ptr = build_simple_component_ref (object, field);
>>> -      field = next_initializable_field (DECL_CHAIN (field));
>>> +      field = next_aggregate_field (DECL_CHAIN (field));
>>>          tree max = build_simple_component_ref (object, field);
>>>          type = build_cplus_array_type (TREE_TYPE (TREE_TYPE (ptr)),
>>>                                     build_index_type (max));
>>> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
>>> index 903ee666ef3..0dffd648b0b 100644
>>> --- a/gcc/cp/method.cc
>>> +++ b/gcc/cp/method.cc
>>> @@ -1465,7 +1465,7 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
>>>      /* A defaulted comparison operator function for class C is defined as
>>>         deleted if ... C has variant members.  */
>>>      if (TREE_CODE (ctype) == UNION_TYPE
>>> -      && next_initializable_field (TYPE_FIELDS (ctype)))
>>> +      && next_aggregate_field (TYPE_FIELDS (ctype)))
>>>        {
>>>          if (complain & tf_error)
>>>        inform (info.loc, "cannot default compare union %qT", ctype);
>>> @@ -1518,9 +1518,9 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
>>>        }
>>>
>>>          /* Now compare the field subobjects.  */
>>> -      for (tree field = next_initializable_field (TYPE_FIELDS (ctype));
>>> +      for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
>>>           field;
>>> -        field = next_initializable_field (DECL_CHAIN (field)))
>>> +        field = next_aggregate_field (DECL_CHAIN (field)))
>>>        {
>>>          if (DECL_VIRTUAL_P (field) || DECL_FIELD_IS_BASE (field))
>>>            /* We ignore the vptr, and we already handled bases.  */
>>> @@ -1542,7 +1542,7 @@ build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
>>>              continue;
>>>            }
>>>          else if (ANON_UNION_TYPE_P (expr_type)
>>> -                && next_initializable_field (TYPE_FIELDS (expr_type)))
>>> +                && next_aggregate_field (TYPE_FIELDS (expr_type)))
>>>            {
>>>              if (complain & tf_error)
>>>                inform (field_loc, "cannot default compare "
>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>> index ac002907a41..86e7212daa5 100644
>>> --- a/gcc/cp/pt.cc
>>> +++ b/gcc/cp/pt.cc
>>> @@ -29578,7 +29578,7 @@ maybe_aggr_guide (tree tmpl, tree init, vec<tree,va_gc> *args)
>>>           len;
>>>           --len, field = DECL_CHAIN (field))
>>>        {
>>> -       field = next_initializable_field (field);
>>> +       field = next_aggregate_field (field);
>>>          if (!field)
>>>            return NULL_TREE;
>>>          tree ftype = finish_decltype_type (field, true, complain);
>>> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
>>> index ed0d0d22950..aba695116c5 100644
>>> --- a/gcc/cp/tree.cc
>>> +++ b/gcc/cp/tree.cc
>>> @@ -4852,8 +4852,8 @@ structural_type_p (tree t, bool explain)
>>>        explain_non_literal_class (t);
>>>          return false;
>>>        }
>>> -  for (tree m = next_initializable_field (TYPE_FIELDS (t)); m;
>>> -       m = next_initializable_field (DECL_CHAIN (m)))
>>> +  for (tree m = next_aggregate_field (TYPE_FIELDS (t)); m;
>>> +       m = next_aggregate_field (DECL_CHAIN (m)))
>>>        {
>>>          if (TREE_PRIVATE (m) || TREE_PROTECTED (m))
>>>        {
>>> diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
>>> index 63d95c1529a..1d92310edd0 100644
>>> --- a/gcc/cp/typeck2.cc
>>> +++ b/gcc/cp/typeck2.cc
>>> @@ -606,7 +606,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool last,
>>>                                  : TYPE_FIELDS (type));
>>>                     ; prev = DECL_CHAIN (prev))
>>>                  {
>>> -                 prev = next_initializable_field (prev);
>>> +                 prev = next_aggregate_field (prev);
>>>                    if (prev == field_index)
>>>                      break;
>>>                    tree ptype = TREE_TYPE (prev);
>>> @@ -1304,7 +1304,7 @@ digest_init_r (tree type, tree init, int nested, int flags,
>>>            the first element of d, which is the B base subobject.  The base
>>>            of type B is copy-initialized from the D temporary, causing
>>>            object slicing.  */
>>> -       tree field = next_initializable_field (TYPE_FIELDS (type));
>>> +       tree field = next_aggregate_field (TYPE_FIELDS (type));
>>>          if (field && DECL_FIELD_IS_BASE (field))
>>>            {
>>>              if (warning_at (loc, 0, "initializing a base class of type %qT "
>>> 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_;
>>
>
  

On Wed, 18 May 2022, Jason Merrill wrote:

> On 5/17/22 12:34, Patrick Palka wrote:
> > On Sat, May 7, 2022 at 5:18 PM Jason Merrill <jason@redhat.com> wrote:
> > > 
> > > On 5/6/22 16:46, Patrick Palka wrote:
> > > > On Fri, 6 May 2022, Jason Merrill wrote:
> > > > 
> > > > > On 5/6/22 16:10, Patrick Palka wrote:
> > > > > > 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.
> > > > > 
> > > > > Why clear the flag on the union before the inner ctor is fully
> > > > > initialized, if
> > > > > the intent is to prevent changing the active member during
> > > > > initialization?
> > > > > 
> > > > > In the loop over ctors in cxx_eval_store_expression, I'd think if we
> > > > > encounter
> > > > > a CONSTRUCTOR_NO_CLEARING ctor along the way, we shouldn't clear the
> > > > > flag on
> > > > > an outer ctor.
> > > > 
> > > > Wouldn't that prevent us from later activating a different member, which
> > > > is
> > > > allowed in C++20?  It would cause us to reject e.g.
> > > > 
> > > >       constexpr auto f() {
> > > >         union U {
> > > >           struct { int x, y; } a;
> > > >           int b;
> > > >         } u;
> > > >         u.a.x = 0;
> > > >         u.b = 0;
> > > >         return u;
> > > >       }
> > > > 
> > > >       static_assert(f().b == 0);
> > > > 
> > > > even in C++20 with
> > > > 
> > > > <source>:7:7: error: change of the active member of a union from
> > > > ‘f()::U::a’ to ‘f()::U::b’ during initialization
> > > >       7 |   u.b = 0;
> > > >         |   ~~~~^~~
> > > > 
> > > > because when evaluating the second assignment we'd see that the
> > > > CONSTRUCTOR_NO_CLEARING flag is still set on the union while it already
> > > > has an activated member.
> > > 
> > > Ah, makes sense.
> > > 
> > > > On a related note, here's the patch that mechanically renames
> > > > next_initializable_field to next_aggregate_field and makes it skip over
> > > > vptr
> > > > fields, and defines next_subobject_field alongside it for use by
> > > > r_c_e_p.
> > > > Bootstrap and regtesting in progress.
> > > 
> > > OK if successful.
> > 
> > Thanks, committed as r13-211-g0c7bce0ac184c0.  Would this patch be
> > suitable for backporting to 12?  It seems safe enough.  Alternatively
> > I guess we could backport the original workaround that tweaks
> > clear_no_implicit_zero.
> 
> Maybe for 12 just add next_subobject_field without touching any other users of
> next_initializable_field?

Ah makes sense, like so?

-- >8 --

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

...

NB: This minimal backport of r13-211-g0c7bce0ac184c0 for 12.2 just adds
next_subobject_field and makes reduced_constant_expression_p use it.

	PR c++/105491

gcc/cp/ChangeLog:

	* constexpr.cc (reduced_constant_expression_p): Use
	next_subobject_field instead.
	* cp-tree.h (next_subobject_field): Declare.
	* decl.cc (next_subobject_field): Define.

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.

(cherry picked from commit 0c7bce0ac184c057bacad9c8e615ce82923835fd)
---
 gcc/cp/constexpr.cc                           |  8 +++----
 gcc/cp/cp-tree.h                              |  1 +
 gcc/cp/decl.cc                                | 19 +++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C | 17 +++++++++++++
 .../g++.dg/cpp0x/constexpr-union7a.C          | 15 ++++++++++++
 gcc/testsuite/g++.dg/cpp2a/constinit17.C      | 24 +++++++++++++++++++
 6 files changed, 80 insertions(+), 4 deletions(-)
 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 47d5113ace2..1e3342adbb9 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3053,7 +3053,7 @@ reduced_constant_expression_p (tree t)
 	      field = NULL_TREE;
 	    }
 	  else
-	    field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
+	    field = next_subobject_field (TYPE_FIELDS (TREE_TYPE (t)));
 	}
       else
 	field = NULL_TREE;
@@ -3065,15 +3065,15 @@ reduced_constant_expression_p (tree t)
 	    return false;
 	  /* Empty class field may or may not have an initializer.  */
 	  for (; field && e.index != field;
-	       field = next_initializable_field (DECL_CHAIN (field)))
+	       field = next_subobject_field (DECL_CHAIN (field)))
 	    if (!is_really_empty_class (TREE_TYPE (field),
 					/*ignore_vptr*/false))
 	      return false;
 	  if (field)
-	    field = next_initializable_field (DECL_CHAIN (field));
+	    field = next_subobject_field (DECL_CHAIN (field));
 	}
       /* There could be a non-empty field at the end.  */
-      for (; field; field = next_initializable_field (DECL_CHAIN (field)))
+      for (; field; field = next_subobject_field (DECL_CHAIN (field)))
 	if (!is_really_empty_class (TREE_TYPE (field), /*ignore_vptr*/false))
 	  return false;
 ok:
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index e9a3d09ac4c..63437415296 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6884,6 +6884,7 @@ extern void initialize_artificial_var		(tree, vec<constructor_elt, va_gc> *);
 extern tree check_var_type			(tree, tree, location_t);
 extern tree reshape_init                        (tree, tree, tsubst_flags_t);
 extern tree next_initializable_field (tree);
+extern tree next_subobject_field		(tree);
 extern tree first_field				(const_tree);
 extern tree fndecl_declared_return_type		(tree);
 extern bool undeduced_auto_decl			(tree);
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 2852093d624..af5eca71ba1 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6409,6 +6409,25 @@ next_initializable_field (tree field)
   return field;
 }
 
+/* FIELD is an element of TYPE_FIELDS or NULL.  In the former case, the value
+   returned is the next FIELD_DECL (possibly FIELD itself) that corresponds
+   to a subobject.  If there are no more such fields, the return value will be
+   NULL.  */
+
+tree
+next_subobject_field (tree field)
+{
+  while (field
+	 && (TREE_CODE (field) != FIELD_DECL
+	     || DECL_UNNAMED_BIT_FIELD (field)
+	     || (DECL_ARTIFICIAL (field)
+		 && !DECL_FIELD_IS_BASE (field)
+		 && !DECL_VIRTUAL_P (field))))
+     field = DECL_CHAIN (field);
+
+  return field;
+}
+
 /* Return true for [dcl.init.list] direct-list-initialization from
    single element of enumeration with a fixed underlying type.  */
 
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_;
  

On 5/31/22 12:41, Patrick Palka wrote:
> On Wed, 18 May 2022, Jason Merrill wrote:
> 
>> On 5/17/22 12:34, Patrick Palka wrote:
>>> On Sat, May 7, 2022 at 5:18 PM Jason Merrill <jason@redhat.com> wrote:
>>>>
>>>> On 5/6/22 16:46, Patrick Palka wrote:
>>>>> On Fri, 6 May 2022, Jason Merrill wrote:
>>>>>
>>>>>> On 5/6/22 16:10, Patrick Palka wrote:
>>>>>>> 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.
>>>>>>
>>>>>> Why clear the flag on the union before the inner ctor is fully
>>>>>> initialized, if
>>>>>> the intent is to prevent changing the active member during
>>>>>> initialization?
>>>>>>
>>>>>> In the loop over ctors in cxx_eval_store_expression, I'd think if we
>>>>>> encounter
>>>>>> a CONSTRUCTOR_NO_CLEARING ctor along the way, we shouldn't clear the
>>>>>> flag on
>>>>>> an outer ctor.
>>>>>
>>>>> Wouldn't that prevent us from later activating a different member, which
>>>>> is
>>>>> allowed in C++20?  It would cause us to reject e.g.
>>>>>
>>>>>        constexpr auto f() {
>>>>>          union U {
>>>>>            struct { int x, y; } a;
>>>>>            int b;
>>>>>          } u;
>>>>>          u.a.x = 0;
>>>>>          u.b = 0;
>>>>>          return u;
>>>>>        }
>>>>>
>>>>>        static_assert(f().b == 0);
>>>>>
>>>>> even in C++20 with
>>>>>
>>>>> <source>:7:7: error: change of the active member of a union from
>>>>> ‘f()::U::a’ to ‘f()::U::b’ during initialization
>>>>>        7 |   u.b = 0;
>>>>>          |   ~~~~^~~
>>>>>
>>>>> because when evaluating the second assignment we'd see that the
>>>>> CONSTRUCTOR_NO_CLEARING flag is still set on the union while it already
>>>>> has an activated member.
>>>>
>>>> Ah, makes sense.
>>>>
>>>>> On a related note, here's the patch that mechanically renames
>>>>> next_initializable_field to next_aggregate_field and makes it skip over
>>>>> vptr
>>>>> fields, and defines next_subobject_field alongside it for use by
>>>>> r_c_e_p.
>>>>> Bootstrap and regtesting in progress.
>>>>
>>>> OK if successful.
>>>
>>> Thanks, committed as r13-211-g0c7bce0ac184c0.  Would this patch be
>>> suitable for backporting to 12?  It seems safe enough.  Alternatively
>>> I guess we could backport the original workaround that tweaks
>>> clear_no_implicit_zero.
>>
>> Maybe for 12 just add next_subobject_field without touching any other users of
>> next_initializable_field?
> 
> Ah makes sense, like so?

OK.

> -- >8 --
> 
> Subject: [PATCH] c++: constexpr init of union sub-aggr w/ base [PR105491]
> 
> ...
> 
> NB: This minimal backport of r13-211-g0c7bce0ac184c0 for 12.2 just adds
> next_subobject_field and makes reduced_constant_expression_p use it.
> 
> 	PR c++/105491
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (reduced_constant_expression_p): Use
> 	next_subobject_field instead.
> 	* cp-tree.h (next_subobject_field): Declare.
> 	* decl.cc (next_subobject_field): Define.
> 
> 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.
> 
> (cherry picked from commit 0c7bce0ac184c057bacad9c8e615ce82923835fd)
> ---
>   gcc/cp/constexpr.cc                           |  8 +++----
>   gcc/cp/cp-tree.h                              |  1 +
>   gcc/cp/decl.cc                                | 19 +++++++++++++++
>   gcc/testsuite/g++.dg/cpp0x/constexpr-union7.C | 17 +++++++++++++
>   .../g++.dg/cpp0x/constexpr-union7a.C          | 15 ++++++++++++
>   gcc/testsuite/g++.dg/cpp2a/constinit17.C      | 24 +++++++++++++++++++
>   6 files changed, 80 insertions(+), 4 deletions(-)
>   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 47d5113ace2..1e3342adbb9 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -3053,7 +3053,7 @@ reduced_constant_expression_p (tree t)
>   	      field = NULL_TREE;
>   	    }
>   	  else
> -	    field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
> +	    field = next_subobject_field (TYPE_FIELDS (TREE_TYPE (t)));
>   	}
>         else
>   	field = NULL_TREE;
> @@ -3065,15 +3065,15 @@ reduced_constant_expression_p (tree t)
>   	    return false;
>   	  /* Empty class field may or may not have an initializer.  */
>   	  for (; field && e.index != field;
> -	       field = next_initializable_field (DECL_CHAIN (field)))
> +	       field = next_subobject_field (DECL_CHAIN (field)))
>   	    if (!is_really_empty_class (TREE_TYPE (field),
>   					/*ignore_vptr*/false))
>   	      return false;
>   	  if (field)
> -	    field = next_initializable_field (DECL_CHAIN (field));
> +	    field = next_subobject_field (DECL_CHAIN (field));
>   	}
>         /* There could be a non-empty field at the end.  */
> -      for (; field; field = next_initializable_field (DECL_CHAIN (field)))
> +      for (; field; field = next_subobject_field (DECL_CHAIN (field)))
>   	if (!is_really_empty_class (TREE_TYPE (field), /*ignore_vptr*/false))
>   	  return false;
>   ok:
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index e9a3d09ac4c..63437415296 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6884,6 +6884,7 @@ extern void initialize_artificial_var		(tree, vec<constructor_elt, va_gc> *);
>   extern tree check_var_type			(tree, tree, location_t);
>   extern tree reshape_init                        (tree, tree, tsubst_flags_t);
>   extern tree next_initializable_field (tree);
> +extern tree next_subobject_field		(tree);
>   extern tree first_field				(const_tree);
>   extern tree fndecl_declared_return_type		(tree);
>   extern bool undeduced_auto_decl			(tree);
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 2852093d624..af5eca71ba1 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -6409,6 +6409,25 @@ next_initializable_field (tree field)
>     return field;
>   }
>   
> +/* FIELD is an element of TYPE_FIELDS or NULL.  In the former case, the value
> +   returned is the next FIELD_DECL (possibly FIELD itself) that corresponds
> +   to a subobject.  If there are no more such fields, the return value will be
> +   NULL.  */
> +
> +tree
> +next_subobject_field (tree field)
> +{
> +  while (field
> +	 && (TREE_CODE (field) != FIELD_DECL
> +	     || DECL_UNNAMED_BIT_FIELD (field)
> +	     || (DECL_ARTIFICIAL (field)
> +		 && !DECL_FIELD_IS_BASE (field)
> +		 && !DECL_VIRTUAL_P (field))))
> +     field = DECL_CHAIN (field);
> +
> +  return field;
> +}
> +
>   /* Return true for [dcl.init.list] direct-list-initialization from
>      single element of enumeration with a fixed underlying type.  */
>   
> 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_;