diff mbox series

[v6] c++: Check for indirect change of active union member in constexpr [PR101631,PR102286]

Message ID 6527b428.170a0220.810bd.457e@mx.google.com
State New
Headers show
Series [v6] c++: Check for indirect change of active union member in constexpr [PR101631,PR102286] | expand

Commit Message

Nathaniel Shead Oct. 12, 2023, 8:53 a.m. UTC 
On Wed, Oct 11, 2023 at 12:48:12AM +1100, Nathaniel Shead wrote:
> On Mon, Oct 09, 2023 at 04:46:46PM -0400, Jason Merrill wrote:
> > On 10/8/23 21:03, Nathaniel Shead wrote:
> > > Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631203.html
> > > 
> > > +	  && (TREE_CODE (t) == MODIFY_EXPR
> > > +	      /* Also check if initializations have implicit change of active
> > > +		 member earlier up the access chain.  */
> > > +	      || !refs->is_empty())
> > 
> > I'm not sure what the cumulative point of these two tests is.  TREE_CODE (t)
> > will be either MODIFY_EXPR or INIT_EXPR, and either should be OK.
> > 
> > As I understand it, the problematic case is something like
> > constexpr-union2.C, where we're also looking at a MODIFY_EXPR.  So what is
> > this check doing?
> 
> The reasoning was to correctly handle cases like the the following (in
> constexpr-union6.C):
> 
>   constexpr int test1() {
>     U u {};
>     std::construct_at(&u.s, S{ 1, 2 });
>     return u.s.b;
>   }
>   static_assert(test1() == 2);
> 
> The initialisation of &u.s here is not a member access expression within
> the call to std::construct_at, since it's just a pointer, but this code
> is still legal; in general, an INIT_EXPR to initialise a union member
> should always be OK (I believe?), hence constraining to just
> MODIFY_EXPR.
> 
> However, just that would then (incorrectly) allow all the following
> cases in that test to compile, such as
> 
>   constexpr int test2() {
>     U u {};
>     int* p = &u.s.b;
>     std::construct_at(p, 5);
>     return u.s.b;
>   }
>   constexpr int x2 = test2();
> 
> since the INIT_EXPR is really only initialising 'b', but the implicit
> "modification" of active member to 'u.s' is illegal.
> 
> Maybe a better way of expressing this condition would be something like
> this?
> 
>   /* An INIT_EXPR of the last member in an access chain is always OK,
>      but still check implicit change of members earlier on; see 
>      cpp2a/constexpr-union6.C.  */
>   && !(TREE_CODE (t) == INIT_EXPR && refs->is_empty ())
> 
> Otherwise I'll see if I can rework some of the other conditions instead.
> 
> > Incidentally, I think constexpr-union6.C could use a test where we pass &u.s
> > to a function other than construct_at, and then try (and fail) to assign to
> > the b member from that function.
> > 
> > Jason
> > 
> 
> Sounds good; I've added the following test:
> 
>   constexpr void foo(S* s) {
>     s->b = 10;  // { dg-error "accessing .U::s. member instead of initialized .U::k." }
>   }
>   constexpr int test3() {
>     U u {};
>     foo(&u.s);  // { dg-message "in .constexpr. expansion" }
>     return u.s.b;
>   }
>   constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }
> 
> Incidentally I found this particular example caused a very unhelpful
> error + ICE due to reporting that S could not be value-initialized in
> the current version of the patch. The updated version below fixes that
> by using 'build_zero_init' instead -- is this an appropriate choice
> here?
> 
> A similar (but unrelated) issue is with e.g.
>   
>   struct S { const int a; int b; };
>   union U { int k; S s; };
> 
>   constexpr int test() {
>     U u {};
>     return u.s.b;
>   }
>   constexpr int x = test();
> 
> giving me this pretty unhelpful error message:
> 
> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
>     6 |   return u.s.b;
>       |          ~~^
> /home/ns/main.cpp:1:8: note: ‘S::S()’ is implicitly deleted because the default definition would be ill-formed:
>     1 | struct S { const int a; int b; };
>       |        ^
> /home/ns/main.cpp:1:8: error: uninitialised const member in ‘struct S’
> /home/ns/main.cpp:1:22: note: ‘const int S::a’ should be initialised
>     1 | struct S { const int a; int b; };
>       |                      ^
> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
>     6 |   return u.s.b;
>       |          ~~^
> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
> 
> but I'll try and fix this separately (it exists on current trunk without
> this patch as well).

While attempting to fix this I found a much better way of handling
value-initialised unions. Here's a new version of the patch which also
includes the fix for accessing the wrong member of a value-initialised
union as well.

Additionally includes an `auto_diagnostic_group` for the `inform`
diagnostics as Marek helpfully informed me about in my other patch.

Bootstrapped and regtested on x86_64-pc-linux-gnu.

-- >8 --

This patch adds checks for attempting to change the active member of a
union by methods other than a member access expression.

To be able to properly distinguish `*(&u.a) = ` from `u.a = `, this
patch redoes the solution for c++/59950 to avoid extranneous *&; it
seems that the only case that needed the workaround was when copying
empty classes.

This patch also ensures that constructors for a union field mark that
field as the active member before entering the call itself; this ensures
that modifications of the field within the constructor's body don't
cause false positives (as these will not appear to be member access
expressions). This means that we no longer need to start the lifetime of
empty union members after the constructor body completes.

As a drive-by fix, this patch also ensures that value-initialised unions
are considered to have activated their initial member for the purpose of
checking stores and accesses, which catches some additional mistakes
pre-C++20.

	PR c++/101631
	PR c++/102286

gcc/cp/ChangeLog:

	* call.cc (build_over_call): Fold more indirect refs for trivial
	assignment op.
	* class.cc (type_has_non_deleted_trivial_default_ctor): Create.
	* constexpr.cc (cxx_eval_call_expression): Start lifetime of
	union member before entering constructor.
        (cxx_eval_component_reference): Check against first member of
        value-initialised union.
	(cxx_eval_store_expression): Activate member for
	value-initialised union. Check for accessing inactive union
	member indirectly.
	* cp-tree.h (type_has_non_deleted_trivial_default_ctor):
	Forward declare.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1y/constexpr-89336-3.C: Fix union initialisation.
	* g++.dg/cpp1y/constexpr-union6.C: New test.
	* g++.dg/cpp1y/constexpr-union7.C: New test.
	* g++.dg/cpp2a/constexpr-union2.C: New test.
	* g++.dg/cpp2a/constexpr-union3.C: New test.
	* g++.dg/cpp2a/constexpr-union4.C: New test.
	* g++.dg/cpp2a/constexpr-union5.C: New test.
	* g++.dg/cpp2a/constexpr-union6.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/call.cc                                |  11 +-
 gcc/cp/class.cc                               |   8 +
 gcc/cp/constexpr.cc                           | 164 +++++++++++++-----
 gcc/cp/cp-tree.h                              |   1 +
 .../g++.dg/cpp1y/constexpr-89336-3.C          |   2 +-
 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C |  13 ++
 gcc/testsuite/g++.dg/cpp1y/constexpr-union7.C |  18 ++
 gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C |  30 ++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C |  45 +++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C |  29 ++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C |  80 +++++++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C |  53 ++++++
 12 files changed, 402 insertions(+), 52 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union7.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C

Comments

Jason Merrill Oct. 12, 2023, 8:24 p.m. UTC | #1 
On 10/12/23 04:53, Nathaniel Shead wrote:
> On Wed, Oct 11, 2023 at 12:48:12AM +1100, Nathaniel Shead wrote:
>> On Mon, Oct 09, 2023 at 04:46:46PM -0400, Jason Merrill wrote:
>>> On 10/8/23 21:03, Nathaniel Shead wrote:
>>>> Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631203.html
>>>>
>>>> +	  && (TREE_CODE (t) == MODIFY_EXPR
>>>> +	      /* Also check if initializations have implicit change of active
>>>> +		 member earlier up the access chain.  */
>>>> +	      || !refs->is_empty())
>>>
>>> I'm not sure what the cumulative point of these two tests is.  TREE_CODE (t)
>>> will be either MODIFY_EXPR or INIT_EXPR, and either should be OK.
>>>
>>> As I understand it, the problematic case is something like
>>> constexpr-union2.C, where we're also looking at a MODIFY_EXPR.  So what is
>>> this check doing?
>>
>> The reasoning was to correctly handle cases like the the following (in
>> constexpr-union6.C):
>>
>>    constexpr int test1() {
>>      U u {};
>>      std::construct_at(&u.s, S{ 1, 2 });
>>      return u.s.b;
>>    }
>>    static_assert(test1() == 2);
>>
>> The initialisation of &u.s here is not a member access expression within
>> the call to std::construct_at, since it's just a pointer, but this code
>> is still legal; in general, an INIT_EXPR to initialise a union member
>> should always be OK (I believe?), hence constraining to just
>> MODIFY_EXPR.
>>
>> However, just that would then (incorrectly) allow all the following
>> cases in that test to compile, such as
>>
>>    constexpr int test2() {
>>      U u {};
>>      int* p = &u.s.b;
>>      std::construct_at(p, 5);
>>      return u.s.b;
>>    }
>>    constexpr int x2 = test2();
>>
>> since the INIT_EXPR is really only initialising 'b', but the implicit
>> "modification" of active member to 'u.s' is illegal.
>>
>> Maybe a better way of expressing this condition would be something like
>> this?
>>
>>    /* An INIT_EXPR of the last member in an access chain is always OK,
>>       but still check implicit change of members earlier on; see
>>       cpp2a/constexpr-union6.C.  */
>>    && !(TREE_CODE (t) == INIT_EXPR && refs->is_empty ())
>>
>> Otherwise I'll see if I can rework some of the other conditions instead.
>>
>>> Incidentally, I think constexpr-union6.C could use a test where we pass &u.s
>>> to a function other than construct_at, and then try (and fail) to assign to
>>> the b member from that function.
>>>
>>> Jason
>>>
>>
>> Sounds good; I've added the following test:
>>
>>    constexpr void foo(S* s) {
>>      s->b = 10;  // { dg-error "accessing .U::s. member instead of initialized .U::k." }
>>    }
>>    constexpr int test3() {
>>      U u {};
>>      foo(&u.s);  // { dg-message "in .constexpr. expansion" }
>>      return u.s.b;
>>    }
>>    constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }
>>
>> Incidentally I found this particular example caused a very unhelpful
>> error + ICE due to reporting that S could not be value-initialized in
>> the current version of the patch. The updated version below fixes that
>> by using 'build_zero_init' instead -- is this an appropriate choice
>> here?
>>
>> A similar (but unrelated) issue is with e.g.
>>    
>>    struct S { const int a; int b; };
>>    union U { int k; S s; };
>>
>>    constexpr int test() {
>>      U u {};
>>      return u.s.b;
>>    }
>>    constexpr int x = test();
>>
>> giving me this pretty unhelpful error message:
>>
>> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
>> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
>>      6 |   return u.s.b;
>>        |          ~~^
>> /home/ns/main.cpp:1:8: note: ‘S::S()’ is implicitly deleted because the default definition would be ill-formed:
>>      1 | struct S { const int a; int b; };
>>        |        ^
>> /home/ns/main.cpp:1:8: error: uninitialised const member in ‘struct S’
>> /home/ns/main.cpp:1:22: note: ‘const int S::a’ should be initialised
>>      1 | struct S { const int a; int b; };
>>        |                      ^
>> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
>> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
>>      6 |   return u.s.b;
>>        |          ~~^
>> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
>> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
>>
>> but I'll try and fix this separately (it exists on current trunk without
>> this patch as well).
> 
> While attempting to fix this I found a much better way of handling
> value-initialised unions. Here's a new version of the patch which also
> includes the fix for accessing the wrong member of a value-initialised
> union as well.
> 
> Additionally includes an `auto_diagnostic_group` for the `inform`
> diagnostics as Marek helpfully informed me about in my other patch.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu.
> 
>> @@ -4496,21 +4491,36 @@ cxx_eval_component_reference (const constexpr_ctx *ctx, tree t,
>   	    break;
>   	}
>       }
> -  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE
> -      && CONSTRUCTOR_NELTS (whole) > 0)
> +  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE)
>       {
> -      /* DR 1188 says we don't have to deal with this.  */
> -      if (!ctx->quiet)
> +      if (CONSTRUCTOR_NELTS (whole) > 0)
>   	{
> -	  constructor_elt *cep = CONSTRUCTOR_ELT (whole, 0);
> -	  if (cep->value == NULL_TREE)
> -	    error ("accessing uninitialized member %qD", part);
> -	  else
> -	    error ("accessing %qD member instead of initialized %qD member in "
> -		   "constant expression", part, cep->index);
> +	  /* DR 1188 says we don't have to deal with this.  */
> +	  if (!ctx->quiet)
> +	    {
> +	      constructor_elt *cep = CONSTRUCTOR_ELT (whole, 0);
> +	      if (cep->value == NULL_TREE)
> +		error ("accessing uninitialized member %qD", part);
> +	      else
> +		error ("accessing %qD member instead of initialized %qD member in "
> +		       "constant expression", part, cep->index);
> +	    }
> +	  *non_constant_p = true;
> +	  return t;
> +	}
> +      else if (!CONSTRUCTOR_NO_CLEARING (whole))
> +	{
> +	  /* Value-initialized union, check if looking at the first member.  */
> +	  tree first = next_aggregate_field (TYPE_FIELDS (TREE_TYPE (whole)));
> +	  if (pmf ? DECL_NAME (first) != DECL_NAME (part) : first != part)

You don't need to consider pmf here, since a PMF isn't UNION_TYPE.

> @@ -6267,23 +6288,74 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>   	  break;
>   	}
>   
> +      /* Process value-initialization of a union.  */
> +      if (code == UNION_TYPE
> +	  && !CONSTRUCTOR_NO_CLEARING (*valp)
> +	  && CONSTRUCTOR_NELTS (*valp) == 0)
> +	if (tree first = next_aggregate_field (TYPE_FIELDS (type)))
> +	  CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (*valp), first, NULL_TREE);

Isn't this adding an uninitialized member instead of value-initialized?

Jason
Nathaniel Shead Oct. 12, 2023, 10:05 p.m. UTC | #2 
On Thu, Oct 12, 2023 at 04:24:00PM -0400, Jason Merrill wrote:
> On 10/12/23 04:53, Nathaniel Shead wrote:
> > On Wed, Oct 11, 2023 at 12:48:12AM +1100, Nathaniel Shead wrote:
> > > On Mon, Oct 09, 2023 at 04:46:46PM -0400, Jason Merrill wrote:
> > > > On 10/8/23 21:03, Nathaniel Shead wrote:
> > > > > Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631203.html
> > > > > 
> > > > > +	  && (TREE_CODE (t) == MODIFY_EXPR
> > > > > +	      /* Also check if initializations have implicit change of active
> > > > > +		 member earlier up the access chain.  */
> > > > > +	      || !refs->is_empty())
> > > > 
> > > > I'm not sure what the cumulative point of these two tests is.  TREE_CODE (t)
> > > > will be either MODIFY_EXPR or INIT_EXPR, and either should be OK.
> > > > 
> > > > As I understand it, the problematic case is something like
> > > > constexpr-union2.C, where we're also looking at a MODIFY_EXPR.  So what is
> > > > this check doing?
> > > 
> > > The reasoning was to correctly handle cases like the the following (in
> > > constexpr-union6.C):
> > > 
> > >    constexpr int test1() {
> > >      U u {};
> > >      std::construct_at(&u.s, S{ 1, 2 });
> > >      return u.s.b;
> > >    }
> > >    static_assert(test1() == 2);
> > > 
> > > The initialisation of &u.s here is not a member access expression within
> > > the call to std::construct_at, since it's just a pointer, but this code
> > > is still legal; in general, an INIT_EXPR to initialise a union member
> > > should always be OK (I believe?), hence constraining to just
> > > MODIFY_EXPR.
> > > 
> > > However, just that would then (incorrectly) allow all the following
> > > cases in that test to compile, such as
> > > 
> > >    constexpr int test2() {
> > >      U u {};
> > >      int* p = &u.s.b;
> > >      std::construct_at(p, 5);
> > >      return u.s.b;
> > >    }
> > >    constexpr int x2 = test2();
> > > 
> > > since the INIT_EXPR is really only initialising 'b', but the implicit
> > > "modification" of active member to 'u.s' is illegal.
> > > 
> > > Maybe a better way of expressing this condition would be something like
> > > this?
> > > 
> > >    /* An INIT_EXPR of the last member in an access chain is always OK,
> > >       but still check implicit change of members earlier on; see
> > >       cpp2a/constexpr-union6.C.  */
> > >    && !(TREE_CODE (t) == INIT_EXPR && refs->is_empty ())
> > > 
> > > Otherwise I'll see if I can rework some of the other conditions instead.
> > > 
> > > > Incidentally, I think constexpr-union6.C could use a test where we pass &u.s
> > > > to a function other than construct_at, and then try (and fail) to assign to
> > > > the b member from that function.
> > > > 
> > > > Jason
> > > > 
> > > 
> > > Sounds good; I've added the following test:
> > > 
> > >    constexpr void foo(S* s) {
> > >      s->b = 10;  // { dg-error "accessing .U::s. member instead of initialized .U::k." }
> > >    }
> > >    constexpr int test3() {
> > >      U u {};
> > >      foo(&u.s);  // { dg-message "in .constexpr. expansion" }
> > >      return u.s.b;
> > >    }
> > >    constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }
> > > 
> > > Incidentally I found this particular example caused a very unhelpful
> > > error + ICE due to reporting that S could not be value-initialized in
> > > the current version of the patch. The updated version below fixes that
> > > by using 'build_zero_init' instead -- is this an appropriate choice
> > > here?
> > > 
> > > A similar (but unrelated) issue is with e.g.
> > >    struct S { const int a; int b; };
> > >    union U { int k; S s; };
> > > 
> > >    constexpr int test() {
> > >      U u {};
> > >      return u.s.b;
> > >    }
> > >    constexpr int x = test();
> > > 
> > > giving me this pretty unhelpful error message:
> > > 
> > > /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
> > > /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
> > >      6 |   return u.s.b;
> > >        |          ~~^
> > > /home/ns/main.cpp:1:8: note: ‘S::S()’ is implicitly deleted because the default definition would be ill-formed:
> > >      1 | struct S { const int a; int b; };
> > >        |        ^
> > > /home/ns/main.cpp:1:8: error: uninitialised const member in ‘struct S’
> > > /home/ns/main.cpp:1:22: note: ‘const int S::a’ should be initialised
> > >      1 | struct S { const int a; int b; };
> > >        |                      ^
> > > /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
> > > /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
> > >      6 |   return u.s.b;
> > >        |          ~~^
> > > /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
> > > /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
> > > 
> > > but I'll try and fix this separately (it exists on current trunk without
> > > this patch as well).
> > 
> > While attempting to fix this I found a much better way of handling
> > value-initialised unions. Here's a new version of the patch which also
> > includes the fix for accessing the wrong member of a value-initialised
> > union as well.
> > 
> > Additionally includes an `auto_diagnostic_group` for the `inform`
> > diagnostics as Marek helpfully informed me about in my other patch.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu.
> > 
> > > @@ -4496,21 +4491,36 @@ cxx_eval_component_reference (const constexpr_ctx *ctx, tree t,
> >   	    break;
> >   	}
> >       }
> > -  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE
> > -      && CONSTRUCTOR_NELTS (whole) > 0)
> > +  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE)
> >       {
> > -      /* DR 1188 says we don't have to deal with this.  */
> > -      if (!ctx->quiet)
> > +      if (CONSTRUCTOR_NELTS (whole) > 0)
> >   	{
> > -	  constructor_elt *cep = CONSTRUCTOR_ELT (whole, 0);
> > -	  if (cep->value == NULL_TREE)
> > -	    error ("accessing uninitialized member %qD", part);
> > -	  else
> > -	    error ("accessing %qD member instead of initialized %qD member in "
> > -		   "constant expression", part, cep->index);
> > +	  /* DR 1188 says we don't have to deal with this.  */
> > +	  if (!ctx->quiet)
> > +	    {
> > +	      constructor_elt *cep = CONSTRUCTOR_ELT (whole, 0);
> > +	      if (cep->value == NULL_TREE)
> > +		error ("accessing uninitialized member %qD", part);
> > +	      else
> > +		error ("accessing %qD member instead of initialized %qD member in "
> > +		       "constant expression", part, cep->index);
> > +	    }
> > +	  *non_constant_p = true;
> > +	  return t;
> > +	}
> > +      else if (!CONSTRUCTOR_NO_CLEARING (whole))
> > +	{
> > +	  /* Value-initialized union, check if looking at the first member.  */
> > +	  tree first = next_aggregate_field (TYPE_FIELDS (TREE_TYPE (whole)));
> > +	  if (pmf ? DECL_NAME (first) != DECL_NAME (part) : first != part)
> 
> You don't need to consider pmf here, since a PMF isn't UNION_TYPE.
> 

Ah right, thanks.

> > @@ -6267,23 +6288,74 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> >   	  break;
> >   	}
> > +      /* Process value-initialization of a union.  */
> > +      if (code == UNION_TYPE
> > +	  && !CONSTRUCTOR_NO_CLEARING (*valp)
> > +	  && CONSTRUCTOR_NELTS (*valp) == 0)
> > +	if (tree first = next_aggregate_field (TYPE_FIELDS (type)))
> > +	  CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (*valp), first, NULL_TREE);
> 
> Isn't this adding an uninitialized member instead of value-initialized?
> 
> Jason
> 

This is equivalent to what was going to happen at the end of the loop
when 'get_or_insert_ctor_field (*valp, index)' is called regardless.
A value-initialized subobject is then created at the start of the loop
(based on 'no_zero_init') replacing the NULL_TREE here. I can try
expanding the comment to clarify this perhaps. Here are two tests that
validate this is working as intended as well:

  union U1 { int a; int b; };
  union U2 { U1 u; };
  union U3 { U2 u; };

  constexpr int foo() {
    U3 u {};
    int* p = &u.u.u.a;
    *p = 10;
    return *p;
  }
  constexpr int x = foo();

  constexpr int bar() {
    U3 u {};
    int* p = &u.u.u.b;
    *p = 10;  // { dg-error "accessing" }
    return *p;
  }
  constexpr int y = bar();
Jason Merrill Oct. 20, 2023, 3:23 a.m. UTC | #3 
On 10/12/23 18:05, Nathaniel Shead wrote:
> On Thu, Oct 12, 2023 at 04:24:00PM -0400, Jason Merrill wrote:
>> On 10/12/23 04:53, Nathaniel Shead wrote:
>>> On Wed, Oct 11, 2023 at 12:48:12AM +1100, Nathaniel Shead wrote:
>>>> On Mon, Oct 09, 2023 at 04:46:46PM -0400, Jason Merrill wrote:
>>>>> On 10/8/23 21:03, Nathaniel Shead wrote:
>>>>>> Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631203.html
>>>>>>
>>>>>> +	  && (TREE_CODE (t) == MODIFY_EXPR
>>>>>> +	      /* Also check if initializations have implicit change of active
>>>>>> +		 member earlier up the access chain.  */
>>>>>> +	      || !refs->is_empty())
>>>>>
>>>>> I'm not sure what the cumulative point of these two tests is.  TREE_CODE (t)
>>>>> will be either MODIFY_EXPR or INIT_EXPR, and either should be OK.
>>>>>
>>>>> As I understand it, the problematic case is something like
>>>>> constexpr-union2.C, where we're also looking at a MODIFY_EXPR.  So what is
>>>>> this check doing?
>>>>
>>>> The reasoning was to correctly handle cases like the the following (in
>>>> constexpr-union6.C):
>>>>
>>>>     constexpr int test1() {
>>>>       U u {};
>>>>       std::construct_at(&u.s, S{ 1, 2 });
>>>>       return u.s.b;
>>>>     }
>>>>     static_assert(test1() == 2);
>>>>
>>>> The initialisation of &u.s here is not a member access expression within
>>>> the call to std::construct_at, since it's just a pointer, but this code
>>>> is still legal; in general, an INIT_EXPR to initialise a union member
>>>> should always be OK (I believe?), hence constraining to just
>>>> MODIFY_EXPR.
>>>>
>>>> However, just that would then (incorrectly) allow all the following
>>>> cases in that test to compile, such as
>>>>
>>>>     constexpr int test2() {
>>>>       U u {};
>>>>       int* p = &u.s.b;
>>>>       std::construct_at(p, 5);
>>>>       return u.s.b;
>>>>     }
>>>>     constexpr int x2 = test2();
>>>>
>>>> since the INIT_EXPR is really only initialising 'b', but the implicit
>>>> "modification" of active member to 'u.s' is illegal.
>>>>
>>>> Maybe a better way of expressing this condition would be something like
>>>> this?
>>>>
>>>>     /* An INIT_EXPR of the last member in an access chain is always OK,
>>>>        but still check implicit change of members earlier on; see
>>>>        cpp2a/constexpr-union6.C.  */
>>>>     && !(TREE_CODE (t) == INIT_EXPR && refs->is_empty ())
>>>>
>>>> Otherwise I'll see if I can rework some of the other conditions instead.
>>>>
>>>>> Incidentally, I think constexpr-union6.C could use a test where we pass &u.s
>>>>> to a function other than construct_at, and then try (and fail) to assign to
>>>>> the b member from that function.
>>>>>
>>>>> Jason
>>>>>
>>>>
>>>> Sounds good; I've added the following test:
>>>>
>>>>     constexpr void foo(S* s) {
>>>>       s->b = 10;  // { dg-error "accessing .U::s. member instead of initialized .U::k." }
>>>>     }
>>>>     constexpr int test3() {
>>>>       U u {};
>>>>       foo(&u.s);  // { dg-message "in .constexpr. expansion" }
>>>>       return u.s.b;
>>>>     }
>>>>     constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }
>>>>
>>>> Incidentally I found this particular example caused a very unhelpful
>>>> error + ICE due to reporting that S could not be value-initialized in
>>>> the current version of the patch. The updated version below fixes that
>>>> by using 'build_zero_init' instead -- is this an appropriate choice
>>>> here?
>>>>
>>>> A similar (but unrelated) issue is with e.g.
>>>>     struct S { const int a; int b; };
>>>>     union U { int k; S s; };
>>>>
>>>>     constexpr int test() {
>>>>       U u {};
>>>>       return u.s.b;
>>>>     }
>>>>     constexpr int x = test();
>>>>
>>>> giving me this pretty unhelpful error message:
>>>>
>>>> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
>>>> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
>>>>       6 |   return u.s.b;
>>>>         |          ~~^
>>>> /home/ns/main.cpp:1:8: note: ‘S::S()’ is implicitly deleted because the default definition would be ill-formed:
>>>>       1 | struct S { const int a; int b; };
>>>>         |        ^
>>>> /home/ns/main.cpp:1:8: error: uninitialised const member in ‘struct S’
>>>> /home/ns/main.cpp:1:22: note: ‘const int S::a’ should be initialised
>>>>       1 | struct S { const int a; int b; };
>>>>         |                      ^
>>>> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
>>>> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
>>>>       6 |   return u.s.b;
>>>>         |          ~~^
>>>> /home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
>>>> /home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
>>>>
>>>> but I'll try and fix this separately (it exists on current trunk without
>>>> this patch as well).
>>>
>>> While attempting to fix this I found a much better way of handling
>>> value-initialised unions. Here's a new version of the patch which also
>>> includes the fix for accessing the wrong member of a value-initialised
>>> union as well.
>>>
>>> Additionally includes an `auto_diagnostic_group` for the `inform`
>>> diagnostics as Marek helpfully informed me about in my other patch.
>>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu.
>>>
>>>> @@ -4496,21 +4491,36 @@ cxx_eval_component_reference (const constexpr_ctx *ctx, tree t,
>>>    	    break;
>>>    	}
>>>        }
>>> -  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE
>>> -      && CONSTRUCTOR_NELTS (whole) > 0)
>>> +  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE)
>>>        {
>>> -      /* DR 1188 says we don't have to deal with this.  */
>>> -      if (!ctx->quiet)
>>> +      if (CONSTRUCTOR_NELTS (whole) > 0)
>>>    	{
>>> -	  constructor_elt *cep = CONSTRUCTOR_ELT (whole, 0);
>>> -	  if (cep->value == NULL_TREE)
>>> -	    error ("accessing uninitialized member %qD", part);
>>> -	  else
>>> -	    error ("accessing %qD member instead of initialized %qD member in "
>>> -		   "constant expression", part, cep->index);
>>> +	  /* DR 1188 says we don't have to deal with this.  */
>>> +	  if (!ctx->quiet)
>>> +	    {
>>> +	      constructor_elt *cep = CONSTRUCTOR_ELT (whole, 0);
>>> +	      if (cep->value == NULL_TREE)
>>> +		error ("accessing uninitialized member %qD", part);
>>> +	      else
>>> +		error ("accessing %qD member instead of initialized %qD member in "
>>> +		       "constant expression", part, cep->index);
>>> +	    }
>>> +	  *non_constant_p = true;
>>> +	  return t;
>>> +	}
>>> +      else if (!CONSTRUCTOR_NO_CLEARING (whole))
>>> +	{
>>> +	  /* Value-initialized union, check if looking at the first member.  */
>>> +	  tree first = next_aggregate_field (TYPE_FIELDS (TREE_TYPE (whole)));
>>> +	  if (pmf ? DECL_NAME (first) != DECL_NAME (part) : first != part)
>>
>> You don't need to consider pmf here, since a PMF isn't UNION_TYPE.
> 
> Ah right, thanks.
> 
>>> @@ -6267,23 +6288,74 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>>    	  break;
>>>    	}
>>> +      /* Process value-initialization of a union.  */
>>> +      if (code == UNION_TYPE
>>> +	  && !CONSTRUCTOR_NO_CLEARING (*valp)
>>> +	  && CONSTRUCTOR_NELTS (*valp) == 0)
>>> +	if (tree first = next_aggregate_field (TYPE_FIELDS (type)))
>>> +	  CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (*valp), first, NULL_TREE);
>>
>> Isn't this adding an uninitialized member instead of value-initialized?
> 
> This is equivalent to what was going to happen at the end of the loop
> when 'get_or_insert_ctor_field (*valp, index)' is called regardless.
> A value-initialized subobject is then created at the start of the loop
> (based on 'no_zero_init') replacing the NULL_TREE here.

Ah, indeed.

I'm applying this tweaked patch, thanks:
diff mbox series

Patch

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index e8dafbd8ba6..c1fb8807d3f 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -10330,10 +10330,7 @@  build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
 	   && DECL_OVERLOADED_OPERATOR_IS (fn, NOP_EXPR)
 	   && trivial_fn_p (fn))
     {
-      /* Don't use cp_build_fold_indirect_ref, op= returns an lvalue even if
-	 the object argument isn't one.  */
-      tree to = cp_build_indirect_ref (input_location, argarray[0],
-				       RO_ARROW, complain);
+      tree to = cp_build_fold_indirect_ref (argarray[0]);
       tree type = TREE_TYPE (to);
       tree as_base = CLASSTYPE_AS_BASE (type);
       tree arg = argarray[1];
@@ -10341,7 +10338,11 @@  build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
 
       if (is_really_empty_class (type, /*ignore_vptr*/true))
 	{
-	  /* Avoid copying empty classes.  */
+	  /* Avoid copying empty classes, but ensure op= returns an lvalue even
+	     if the object argument isn't one. This isn't needed in other cases
+	     since MODIFY_EXPR is always considered an lvalue.  */
+	  to = cp_build_addr_expr (to, tf_none);
+	  to = cp_build_indirect_ref (input_location, to, RO_ARROW, complain);
 	  val = build2 (COMPOUND_EXPR, type, arg, to);
 	  suppress_warning (val, OPT_Wunused);
 	}
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index b71333af1f8..e31aeb8e68b 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -5688,6 +5688,14 @@  type_has_virtual_destructor (tree type)
   return (dtor && DECL_VIRTUAL_P (dtor));
 }
 
+/* True iff class TYPE has a non-deleted trivial default
+   constructor.  */
+
+bool type_has_non_deleted_trivial_default_ctor (tree type)
+{
+  return TYPE_HAS_TRIVIAL_DFLT (type) && locate_ctor (type);
+}
+
 /* Returns true iff T, a class, has a move-assignment or
    move-constructor.  Does not lazily declare either.
    If USER_P is false, any move function will do.  If it is true, the
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0f948db7c2d..99b8610df7a 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3146,40 +3146,34 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	    cxx_set_object_constness (ctx, new_obj, /*readonly_p=*/false,
 				      non_constant_p, overflow_p);
 
+	  /* If this is a constructor, we are beginning the lifetime of the
+	     object we are initializing.  */
+	  if (new_obj
+	      && DECL_CONSTRUCTOR_P (fun)
+	      && TREE_CODE (new_obj) == COMPONENT_REF
+	      && TREE_CODE (TREE_TYPE (TREE_OPERAND (new_obj, 0))) == UNION_TYPE)
+	    {
+	      tree activate = build2 (INIT_EXPR, TREE_TYPE (new_obj),
+				      new_obj,
+				      build_constructor (TREE_TYPE (new_obj),
+							 NULL));
+	      cxx_eval_constant_expression (ctx, activate,
+					    lval, non_constant_p, overflow_p);
+	      ggc_free (activate);
+	    }
+
 	  tree jump_target = NULL_TREE;
 	  cxx_eval_constant_expression (&call_ctx, body,
 					vc_discard, non_constant_p, overflow_p,
 					&jump_target);
 
 	  if (DECL_CONSTRUCTOR_P (fun))
-	    {
-	      /* 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;
-
-	      /* If we've just evaluated a subobject constructor call for an
-		 empty union member, it might not have produced a side effect
-		 that actually activated the union member.  So produce such a
-		 side effect now to ensure the union appears initialized.  */
-	      if (!result && new_obj
-		  && TREE_CODE (new_obj) == COMPONENT_REF
-		  && TREE_CODE (TREE_TYPE
-				(TREE_OPERAND (new_obj, 0))) == UNION_TYPE
-		  && is_really_empty_class (TREE_TYPE (new_obj),
-					    /*ignore_vptr*/false))
-		{
-		  tree activate = build2 (MODIFY_EXPR, TREE_TYPE (new_obj),
-					  new_obj,
-					  build_constructor (TREE_TYPE (new_obj),
-							     NULL));
-		  cxx_eval_constant_expression (ctx, activate, lval,
-						non_constant_p, overflow_p);
-		  ggc_free (activate);
-		}
-	    }
+	    /* 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;
 	  else if (VOID_TYPE_P (TREE_TYPE (res)))
 	    result = void_node;
 	  else
@@ -4478,6 +4472,7 @@  cxx_eval_component_reference (const constexpr_ctx *ctx, tree t,
       *non_constant_p = true;
       return t;
     }
+
   bool pmf = TYPE_PTRMEMFUNC_P (TREE_TYPE (whole));
   FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (whole), i, field, value)
     {
@@ -4496,21 +4491,36 @@  cxx_eval_component_reference (const constexpr_ctx *ctx, tree t,
 	    break;
 	}
     }
-  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE
-      && CONSTRUCTOR_NELTS (whole) > 0)
+  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE)
     {
-      /* DR 1188 says we don't have to deal with this.  */
-      if (!ctx->quiet)
+      if (CONSTRUCTOR_NELTS (whole) > 0)
 	{
-	  constructor_elt *cep = CONSTRUCTOR_ELT (whole, 0);
-	  if (cep->value == NULL_TREE)
-	    error ("accessing uninitialized member %qD", part);
-	  else
-	    error ("accessing %qD member instead of initialized %qD member in "
-		   "constant expression", part, cep->index);
+	  /* DR 1188 says we don't have to deal with this.  */
+	  if (!ctx->quiet)
+	    {
+	      constructor_elt *cep = CONSTRUCTOR_ELT (whole, 0);
+	      if (cep->value == NULL_TREE)
+		error ("accessing uninitialized member %qD", part);
+	      else
+		error ("accessing %qD member instead of initialized %qD member in "
+		       "constant expression", part, cep->index);
+	    }
+	  *non_constant_p = true;
+	  return t;
+	}
+      else if (!CONSTRUCTOR_NO_CLEARING (whole))
+	{
+	  /* Value-initialized union, check if looking at the first member.  */
+	  tree first = next_aggregate_field (TYPE_FIELDS (TREE_TYPE (whole)));
+	  if (pmf ? DECL_NAME (first) != DECL_NAME (part) : first != part)
+	    {
+	      if (!ctx->quiet)
+		error ("accessing %qD member instead of initialized %qD "
+		       "member in constant expression", part, first);
+	      *non_constant_p = true;
+	      return t;
+	    }
 	}
-      *non_constant_p = true;
-      return t;
     }
 
   /* We only create a CONSTRUCTOR for a subobject when we modify it, so empty
@@ -6126,6 +6136,14 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 						  mutable_p)
 		     && const_object_being_modified == NULL_TREE)
 	      const_object_being_modified = probe;
+
+	    /* Track named member accesses for unions to validate modifications
+	       that change active member.  */
+	    if (!evaluated && TREE_CODE (probe) == COMPONENT_REF)
+	      vec_safe_push (refs, probe);
+	    else
+	      vec_safe_push (refs, NULL_TREE);
+
 	    vec_safe_push (refs, elt);
 	    vec_safe_push (refs, TREE_TYPE (probe));
 	    probe = ob;
@@ -6134,6 +6152,7 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 
 	case REALPART_EXPR:
 	  gcc_assert (probe == target);
+	  vec_safe_push (refs, NULL_TREE);
 	  vec_safe_push (refs, probe);
 	  vec_safe_push (refs, TREE_TYPE (probe));
 	  probe = TREE_OPERAND (probe, 0);
@@ -6141,6 +6160,7 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 
 	case IMAGPART_EXPR:
 	  gcc_assert (probe == target);
+	  vec_safe_push (refs, NULL_TREE);
 	  vec_safe_push (refs, probe);
 	  vec_safe_push (refs, TREE_TYPE (probe));
 	  probe = TREE_OPERAND (probe, 0);
@@ -6229,6 +6249,7 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
       enum tree_code code = TREE_CODE (type);
       tree reftype = refs->pop();
       tree index = refs->pop();
+      bool is_access_expr = refs->pop() != NULL_TREE;
 
       if (code == COMPLEX_TYPE)
 	{
@@ -6267,23 +6288,74 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 	  break;
 	}
 
+      /* Process value-initialization of a union.  */
+      if (code == UNION_TYPE
+	  && !CONSTRUCTOR_NO_CLEARING (*valp)
+	  && CONSTRUCTOR_NELTS (*valp) == 0)
+	if (tree first = next_aggregate_field (TYPE_FIELDS (type)))
+	  CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (*valp), first, NULL_TREE);
+
       type = reftype;
 
-      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
-	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
+      /* Check for implicit change of active member for a union.  */
+      if (code == UNION_TYPE
+	  && (CONSTRUCTOR_NELTS (*valp) == 0
+	      || CONSTRUCTOR_ELT (*valp, 0)->index != index)
+	  /* An INIT_EXPR of the last member in an access chain is always OK,
+	     but still check implicit change of members earlier on; see
+	     cpp2a/constexpr-union6.C.  */
+	  && !(TREE_CODE (t) == INIT_EXPR && refs->is_empty ()))
 	{
-	  if (cxx_dialect < cxx20)
+	  bool has_active_member = CONSTRUCTOR_NELTS (*valp) != 0;
+	  tree inner = strip_array_types (reftype);
+
+	  if (has_active_member && cxx_dialect < cxx20)
 	    {
 	      if (!ctx->quiet)
 		error_at (cp_expr_loc_or_input_loc (t),
 			  "change of the active member of a union "
-			  "from %qD to %qD",
+			  "from %qD to %qD is not a constant expression "
+			  "before C++20",
 			  CONSTRUCTOR_ELT (*valp, 0)->index,
 			  index);
 	      *non_constant_p = true;
 	    }
-	  else if (TREE_CODE (t) == MODIFY_EXPR
-		   && CONSTRUCTOR_NO_CLEARING (*valp))
+	  else if (!is_access_expr
+		   || (TREE_CODE (t) == MODIFY_EXPR
+		       && CLASS_TYPE_P (inner)
+		       && !type_has_non_deleted_trivial_default_ctor (inner)))
+	    {
+	      /* Diagnose changing active union member after initialization
+		 without a valid member access expression, as described in
+		 [class.union.general] p5.  */
+	      if (!ctx->quiet)
+		{
+		  auto_diagnostic_group d;
+		  if (has_active_member)
+		    error_at (cp_expr_loc_or_input_loc (t),
+			      "accessing %qD member instead of initialized "
+			      "%qD member in constant expression",
+			      index, CONSTRUCTOR_ELT (*valp, 0)->index);
+		  else
+		    error_at (cp_expr_loc_or_input_loc (t),
+			      "accessing uninitialized member %qD",
+			      index);
+		  if (is_access_expr)
+		    inform (DECL_SOURCE_LOCATION (index),
+			    "%qD does not implicitly begin its lifetime "
+			    "because %qT does not have a non-deleted "
+			    "trivial default constructor, use "
+			    "%<std::construct_at%> instead",
+			    index, inner);
+		  else
+		    inform (DECL_SOURCE_LOCATION (index),
+			    "initializing %qD requires a member access "
+			    "expression as the left operand of the assignment",
+			    index);
+		}
+	      *non_constant_p = true;
+	    }
+	  else if (has_active_member && CONSTRUCTOR_NO_CLEARING (*valp))
 	    {
 	      /* Diagnose changing the active union member while the union
 		 is in the process of being initialized.  */
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 6e34952da99..b36db580818 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6815,6 +6815,7 @@  extern bool trivial_default_constructor_is_constexpr (tree);
 extern bool type_has_constexpr_default_constructor (tree);
 extern bool type_has_constexpr_destructor	(tree);
 extern bool type_has_virtual_destructor		(tree);
+extern bool type_has_non_deleted_trivial_default_ctor (tree);
 extern bool classtype_has_move_assign_or_move_ctor_p (tree, bool user_declared);
 extern bool classtype_has_non_deleted_move_ctor (tree);
 extern tree classtype_has_depr_implicit_copy	(tree);
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C
index 9d370ddefcf..6a60966f6f8 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C
@@ -18,7 +18,7 @@  constexpr int
 bar ()
 {
   union U { int a[5]; long b; };
-  union V { union U u; short v; };
+  union V { short v; union U u; };
   V w {};
   w.v = 5;
   w.u.a[3] = w.u.a[1] = w.v;		// { dg-error "change of the active member of a union from" "" { target c++17_down } }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C
new file mode 100644
index 00000000000..ff7ebf19f89
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C
@@ -0,0 +1,13 @@ 
+// { dg-do compile { target c++14 } }
+
+union U {
+  int a;
+  float b;
+};
+
+constexpr bool foo() {
+  U u {};
+  u.b = 1.0f;  // { dg-error "change of the active member" "" { target c++17_down } }
+  return u.b == 1.0f;
+}
+constexpr bool x = foo();
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-union7.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-union7.C
new file mode 100644
index 00000000000..6fc41f97354
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-union7.C
@@ -0,0 +1,18 @@ 
+// { dg-do compile { target c++14 } }
+
+// this type is not value-initialisable
+struct S { const int a; int b; };
+
+union U1 { int k; S s; };
+constexpr int test1() {
+  U1 u {};
+  return u.s.b;  // { dg-error "accessing .U1::s. member instead of initialized .U1::k. member" }
+}
+constexpr int x = test1();
+
+union U2 { int :0; static int s; void foo(); int k; };
+constexpr int test2() {
+  U2 u {};  // should skip zero-width bitfields, static members, and functions
+  return u.k;
+}
+static_assert(test2() == 0, "");
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C
new file mode 100644
index 00000000000..1712395d7e7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C
@@ -0,0 +1,30 @@ 
+// PR c++/101631
+// { dg-do compile { target c++20 } }
+
+struct sso {
+  union {
+    int buf[10];
+    int* alloc;
+  };
+};
+
+constexpr bool direct() {
+  sso val;
+  val.alloc = nullptr;
+  val.buf[5] = 42;
+  return true;
+}
+constexpr bool ok = direct();
+
+
+constexpr void perform_assignment(int& left, int right) noexcept {
+  left = right;  // { dg-error "accessing .+ member instead of initialized" }
+}
+
+constexpr bool indirect() {
+  sso val;
+  val.alloc = nullptr;
+  perform_assignment(val.buf[5], 42);  // { dg-message "in .constexpr. expansion" }
+  return true;
+}
+constexpr bool err = indirect();  // { dg-message "in .constexpr. expansion" }
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C
new file mode 100644
index 00000000000..6d30bb2498f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C
@@ -0,0 +1,45 @@ 
+// { dg-do compile { target c++20 } }
+
+struct S
+{
+  union {
+    char buf[8];
+    char* ptr;
+  };
+  unsigned len;
+
+  constexpr S(const char* s, unsigned n)
+  {
+    char* p;
+    if (n > 7)
+      p = ptr = new char[n+1];
+    else
+      p = buf;
+    for (len = 0; len < n; ++len)
+      p[len] = s[len];  // { dg-error "accessing uninitialized member" }
+    p[len] = '\0';
+  }
+
+  constexpr ~S()
+  {
+    if (len > 7)
+      delete[] ptr;
+  }
+};
+
+constexpr bool test1()
+{
+  S s("test", 4);  // { dg-message "in .constexpr. expansion" }
+  return true;
+}
+
+constexpr bool a = test1();  // { dg-message "in .constexpr. expansion" }
+
+
+constexpr bool test2()
+{
+  S s("hello world", 11);
+  return true;
+}
+
+constexpr bool b = test2();
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C
new file mode 100644
index 00000000000..429ab203ac2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C
@@ -0,0 +1,29 @@ 
+// { dg-do compile { target c++20 } }
+
+// from [class.union.general] p5
+
+union A { int x; int y[4]; };
+struct B { A a; };
+union C { B b; int k; };
+constexpr int f() {
+  C c;                  // does not start lifetime of any union member
+  c.b.a.y[3] = 4;       // OK, S(c.b.a.y[3]) contains c.b and c.b.a.y;
+                        // creates objects to hold union members c.b and c.b.a.y
+  return c.b.a.y[3];    // OK, c.b.a.y refers to newly created object (see [basic.life])
+}
+constexpr int a = f();
+
+struct X { const int a; int b; };
+union Y { X x; int k; };// { dg-message "does not implicitly begin its lifetime" }
+constexpr int g() {
+  Y y = { { 1, 2 } };   // OK, y.x is active union member ([class.mem])
+  int n = y.x.a;
+  y.k = 4;              // OK, ends lifetime of y.x, y.k is active member of union
+
+  y.x.b = n;            // { dg-error "accessing .* member instead of initialized .* member" }
+                        // undefined behavior: y.x.b modified outside its lifetime,
+                        // S(y.x.b) is empty because X's default constructor is deleted,
+                        // so union member y.x's lifetime does not implicitly start
+  return 0;
+}
+constexpr int b = g();  // { dg-message "in .constexpr. expansion" }
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C
new file mode 100644
index 00000000000..7e42522b4e1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C
@@ -0,0 +1,80 @@ 
+// { dg-do compile { target c++20 } }
+
+union U { int a; int b; int c[2]; };
+
+constexpr int test1() {
+  U u;
+  u.a = 10;
+  *&u.b = 20;  // { dg-error "accessing" }
+  return u.b;
+}
+constexpr int x1 = test1();  // { dg-message "in .constexpr. expansion" }
+
+constexpr int test2() {
+  U u;
+  u.a = 10;
+  (0, u.b) = 20;  // { dg-error "accessing" }
+  return u.b;
+}
+constexpr int x2 = test2();  // { dg-message "in .constexpr. expansion" }
+
+constexpr int test3() {
+  U u;
+  u.a = 0;
+  int* p = &u.b;
+  p[u.a] = 10;  // { dg-error "accessing" }
+  return u.b;
+}
+constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }
+
+constexpr int test4() {
+  U u;
+  u.a = 0;
+  int* p = &u.b;
+  u.a[p] = 10;  // { dg-error "accessing" }
+  return u.b;
+}
+constexpr int x4 = test4();  // { dg-message "in .constexpr. expansion" }
+
+struct S { U u[10]; };
+constexpr int test5() {
+  S s;
+  s.u[4].a = 10;
+  6[s.u].b = 15;
+  return 4[s.u].a + s.u[6].b;
+}
+static_assert(test5() == 25);
+
+constexpr int test6() {
+  U u;
+  u.a = 5;
+  u.c[0] = 3;
+  1[u.c] = 8;
+  return 1[u.c] + u.c[0];
+}
+static_assert(test6() == 11);
+
+constexpr int test7() {
+  U u;  // default initialisation leaves no member initialised
+  int* p = &u.a;
+  *p = 10;  // { dg-error "accessing" }
+  return *p;
+}
+constexpr int x7 = test7();  // { dg-message "in .constexpr. expansion" }
+
+constexpr int test8() {
+  U u {};  // value initialisation initialises first member
+  int* p = &u.a;
+  *p = 8;
+  return *p;
+}
+static_assert(test8() == 8);
+
+union V { int :0; static int x; void foo(); int a; };
+constexpr int test9() {
+  V v {}; // should skip zero-width bit fields, static members, and functions
+  int* p = &v.a;
+  *p = 9;
+  return *p;
+}
+static_assert(test9() == 9);
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C
new file mode 100644
index 00000000000..00bda531e59
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C
@@ -0,0 +1,53 @@ 
+// { dg-do compile { target c++20 } }
+// PR c++/102286
+
+#include "construct_at.h"
+
+struct S { const int a; int b; };
+union U { int k; S s; };
+
+constexpr int test1() {
+  U u {};
+  std::construct_at(&u.s, S{ 1, 2 });
+  return u.s.b;
+}
+static_assert(test1() == 2);
+
+constexpr int test2() {
+  U u {};
+  int* p = &u.s.b;
+  std::construct_at(p, 5);  // { dg-message "in .constexpr. expansion" }
+  return u.s.b;
+}
+constexpr int x2 = test2();  // { dg-message "in .constexpr. expansion" }
+
+constexpr void foo(S* s) {
+  s->b = 10;  // { dg-error "accessing .U::s. member instead of initialized .U::k." }
+}
+constexpr int test3() {
+  U u {};
+  foo(&u.s);  // { dg-message "in .constexpr. expansion" }
+  return u.s.b;
+}
+constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }
+
+struct S2 { int a; int b; };
+union U2 { int k; S2 s; };
+constexpr int test4() {
+  U2 u;
+  int* p = &u.s.b;
+  std::construct_at(p, 8);  // { dg-message "in .constexpr. expansion" }
+  return u.s.b;
+};
+constexpr int x4 = test4();  // { dg-message "in .constexpr. expansion" }
+
+constexpr int test5() {
+  union {
+    int data[1];
+  } u;
+  std::construct_at(u.data, 0);  // { dg-message "in .constexpr. expansion" }
+  return 0;
+}
+constexpr int x5 = test5();  // { dg-message "in .constexpr. expansion" }
+
+// { dg-error "accessing (uninitialized member|.* member instead of)" "" { target *-*-* } 0 }