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

Message ID ZQ2sQNoVz0rblzDN@Thaum.localdomain
State Accepted
Headers
Series [v4] c++: Check for indirect change of active union member in constexpr [PR101631,PR102286] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Nathaniel Shead Sept. 22, 2023, 3:01 p.m. UTC
  On Fri, Sep 22, 2023 at 02:21:15PM +0100, Jason Merrill wrote:
> On 9/21/23 09:41, Nathaniel Shead wrote:
> > I've updated the error messages, and also fixed another bug I found
> > while retesting (value-initialised unions weren't considered to have any
> > active member yet).
> > 
> > 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, which catches some additional mistakes pre-C++20.
> > 
> > 	PR c++/101631
> > 
> > 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_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/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.
> > 
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> >   gcc/cp/call.cc                                |  11 +-
> >   gcc/cp/class.cc                               |   8 ++
> >   gcc/cp/constexpr.cc                           | 129 +++++++++++++-----
> >   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/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 |  71 ++++++++++
> >   10 files changed, 296 insertions(+), 43 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.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
> > 
> > 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 a673a6022f1..5e50fc2c792 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
> > @@ -6127,6 +6121,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;
> > @@ -6135,6 +6137,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);
> > @@ -6142,6 +6145,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);
> > @@ -6230,6 +6234,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)
> >   	{
> > @@ -6270,21 +6275,72 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> >         type = reftype;
> > -      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> > -	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> > +      if (code == UNION_TYPE
> > +	  && TREE_CODE (t) == MODIFY_EXPR
> > +	  && (CONSTRUCTOR_NELTS (*valp) == 0
> > +	      || CONSTRUCTOR_ELT (*valp, 0)->index != index))
> >   	{
> > -	  if (cxx_dialect < cxx20)
> > +	  /* Probably a change of active member for a union. Ensure that
> > +	     this is valid.  */
> > +	  no_zero_init = true;
> > +
> > +	  tree init = *valp;
> > +	  if (!CONSTRUCTOR_NO_CLEARING (*valp) && CONSTRUCTOR_NELTS (*valp) == 0)
> > +	    /* This is a value-initialized union, process the initialization
> > +	       to ensure that the active member is properly set.  */
> > +	    init = build_value_init (TREE_TYPE (*valp), tf_warning_or_error);
> > +
> > +	  bool has_active_member = CONSTRUCTOR_NELTS (init) != 0;
> > +	  tree inner = strip_array_types (reftype);
> > +
> > +	  if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == index)
> > +	    /* After handling value-initialization, this wasn't actually a
> > +	       change of active member like we initially thought.  */
> > +	    no_zero_init = false;
> > +	  else 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",
> > -			  CONSTRUCTOR_ELT (*valp, 0)->index,
> > +			  "from %qD to %qD is not a constant expression "
> > +			  "before C++20",
> > +			  CONSTRUCTOR_ELT (init, 0)->index,
> >   			  index);
> >   	      *non_constant_p = true;
> >   	    }
> > -	  else if (TREE_CODE (t) == MODIFY_EXPR
> > -		   && CONSTRUCTOR_NO_CLEARING (*valp))
> > +	  else if (!is_access_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)
> > +		{
> > +		  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 (init, 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",
> > +			    index, inner);
> 
> It now occurs to me that this should suggest using placement new instead.
> And that we should test using placement new to begin the lifetime of such
> members.
> 
> Jason
> 

Good point. On testing placement new I found I wasn't checking for
indirect change of active member during initialisation either, but this
was pretty trivial to add. How does this look?

Bootstrap + regtest ongoing, but dg.exp completed with no errors. I'm
away from my computer next week so I thought I'd send this through now
anyway.

-- >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, 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_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/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                           | 135 +++++++++++++-----
 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/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 |  71 +++++++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C |  43 ++++++
 11 files changed, 345 insertions(+), 43 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.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

Nathaniel Shead Sept. 23, 2023, 12:38 a.m. UTC | #1
Now that bootstrap has finished, I have gotten regressions in the
following libstdc++ tests:

Running libstdc++:libstdc++-dg/conformance.exp ...
FAIL: 20_util/bitset/access/constexpr.cc -std=gnu++23 (test for excess errors)
FAIL: 20_util/bitset/access/constexpr.cc -std=gnu++26 (test for excess errors)
FAIL: 20_util/variant/constexpr.cc -std=gnu++20 (test for excess errors)
FAIL: 20_util/variant/constexpr.cc -std=gnu++26 (test for excess errors)
FAIL: 21_strings/basic_string/cons/char/constexpr.cc -std=gnu++20 (test for excess errors)
FAIL: 21_strings/basic_string/cons/char/constexpr.cc -std=gnu++26 (test for excess errors)
FAIL: 21_strings/basic_string/cons/wchar_t/constexpr.cc -std=gnu++20 (test for excess errors)
FAIL: 21_strings/basic_string/cons/wchar_t/constexpr.cc -std=gnu++26 (test for excess errors)
FAIL: 21_strings/basic_string/modifiers/swap/constexpr-wchar_t.cc -std=gnu++20 (test for excess errors)
FAIL: 21_strings/basic_string/modifiers/swap/constexpr-wchar_t.cc -std=gnu++26 (test for excess errors)
FAIL: 21_strings/basic_string/modifiers/swap/constexpr.cc -std=gnu++20 (test for excess errors)
FAIL: 21_strings/basic_string/modifiers/swap/constexpr.cc -std=gnu++26 (test for excess errors)
FAIL: std/ranges/adaptors/join_with/1.cc -std=gnu++23 (test for excess errors)
UNRESOLVED: std/ranges/adaptors/join_with/1.cc -std=gnu++23 compilation failed to produce executable
FAIL: std/ranges/adaptors/join_with/1.cc -std=gnu++26 (test for excess errors)
UNRESOLVED: std/ranges/adaptors/join_with/1.cc -std=gnu++26 compilation failed to produce executable

On investigation though it looks like the issue might be with libstdc++
rather than the patch itself; running the failing tests using clang with
libstdc++ also produces similar errors, and my reading of the code
suggests that this is correct.

What's the way forward here? Should I look at creating a patch to fix
the libstdc++ issues before resubmitting this patch for the C++
frontend? Or should I submit a version of this patch without the
`std::construct_at` changes and wait till libstdc++ gets fixed for that?

On Sat, Sep 23, 2023 at 01:01:20AM +1000, Nathaniel Shead wrote:
> On Fri, Sep 22, 2023 at 02:21:15PM +0100, Jason Merrill wrote:
> > On 9/21/23 09:41, Nathaniel Shead wrote:
> > > I've updated the error messages, and also fixed another bug I found
> > > while retesting (value-initialised unions weren't considered to have any
> > > active member yet).
> > > 
> > > 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, which catches some additional mistakes pre-C++20.
> > > 
> > > 	PR c++/101631
> > > 
> > > 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_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/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.
> > > 
> > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > ---
> > >   gcc/cp/call.cc                                |  11 +-
> > >   gcc/cp/class.cc                               |   8 ++
> > >   gcc/cp/constexpr.cc                           | 129 +++++++++++++-----
> > >   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/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 |  71 ++++++++++
> > >   10 files changed, 296 insertions(+), 43 deletions(-)
> > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.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
> > > 
> > > 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 a673a6022f1..5e50fc2c792 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
> > > @@ -6127,6 +6121,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;
> > > @@ -6135,6 +6137,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);
> > > @@ -6142,6 +6145,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);
> > > @@ -6230,6 +6234,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)
> > >   	{
> > > @@ -6270,21 +6275,72 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> > >         type = reftype;
> > > -      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> > > -	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> > > +      if (code == UNION_TYPE
> > > +	  && TREE_CODE (t) == MODIFY_EXPR
> > > +	  && (CONSTRUCTOR_NELTS (*valp) == 0
> > > +	      || CONSTRUCTOR_ELT (*valp, 0)->index != index))
> > >   	{
> > > -	  if (cxx_dialect < cxx20)
> > > +	  /* Probably a change of active member for a union. Ensure that
> > > +	     this is valid.  */
> > > +	  no_zero_init = true;
> > > +
> > > +	  tree init = *valp;
> > > +	  if (!CONSTRUCTOR_NO_CLEARING (*valp) && CONSTRUCTOR_NELTS (*valp) == 0)
> > > +	    /* This is a value-initialized union, process the initialization
> > > +	       to ensure that the active member is properly set.  */
> > > +	    init = build_value_init (TREE_TYPE (*valp), tf_warning_or_error);
> > > +
> > > +	  bool has_active_member = CONSTRUCTOR_NELTS (init) != 0;
> > > +	  tree inner = strip_array_types (reftype);
> > > +
> > > +	  if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == index)
> > > +	    /* After handling value-initialization, this wasn't actually a
> > > +	       change of active member like we initially thought.  */
> > > +	    no_zero_init = false;
> > > +	  else 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",
> > > -			  CONSTRUCTOR_ELT (*valp, 0)->index,
> > > +			  "from %qD to %qD is not a constant expression "
> > > +			  "before C++20",
> > > +			  CONSTRUCTOR_ELT (init, 0)->index,
> > >   			  index);
> > >   	      *non_constant_p = true;
> > >   	    }
> > > -	  else if (TREE_CODE (t) == MODIFY_EXPR
> > > -		   && CONSTRUCTOR_NO_CLEARING (*valp))
> > > +	  else if (!is_access_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)
> > > +		{
> > > +		  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 (init, 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",
> > > +			    index, inner);
> > 
> > It now occurs to me that this should suggest using placement new instead.
> > And that we should test using placement new to begin the lifetime of such
> > members.
> > 
> > Jason
> > 
> 
> Good point. On testing placement new I found I wasn't checking for
> indirect change of active member during initialisation either, but this
> was pretty trivial to add. How does this look?
> 
> Bootstrap + regtest ongoing, but dg.exp completed with no errors. I'm
> away from my computer next week so I thought I'd send this through now
> anyway.
> 
> -- >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, 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_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/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                           | 135 +++++++++++++-----
>  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/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 |  71 +++++++++
>  gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C |  43 ++++++
>  11 files changed, 345 insertions(+), 43 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.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
> 
> 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 a673a6022f1..8e6f60d03d2 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
> @@ -6127,6 +6121,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;
> @@ -6135,6 +6137,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);
> @@ -6142,6 +6145,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);
> @@ -6230,6 +6234,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)
>  	{
> @@ -6270,21 +6275,78 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>  
>        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
> +	  && (TREE_CODE (t) == MODIFY_EXPR
> +	      /* Also check if initializations have implicit change of active
> +		 member earlier up the access chain.  */
> +	      || !refs->is_empty())
> +	  && (CONSTRUCTOR_NELTS (*valp) == 0
> +	      || CONSTRUCTOR_ELT (*valp, 0)->index != index))
>  	{
> -	  if (cxx_dialect < cxx20)
> +	  no_zero_init = true;
> +
> +	  tree init = *valp;
> +	  if (TREE_CODE (t) == MODIFY_EXPR
> +	      && !CONSTRUCTOR_NO_CLEARING (*valp)
> +	      && CONSTRUCTOR_NELTS (*valp) == 0)
> +	    /* Modifying a value-initialized union, process the initialization
> +	       to ensure that the active member is properly set.  */
> +	    init = build_value_init (TREE_TYPE (*valp), tf_warning_or_error);
> +
> +	  bool has_active_member = CONSTRUCTOR_NELTS (init) != 0;
> +	  tree inner = strip_array_types (reftype);
> +
> +	  if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == index)
> +	    /* After handling value-initialization, this wasn't actually a
> +	       change of active member like we initially thought.  */
> +	    no_zero_init = false;
> +	  else 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",
> -			  CONSTRUCTOR_ELT (*valp, 0)->index,
> +			  "from %qD to %qD is not a constant expression "
> +			  "before C++20",
> +			  CONSTRUCTOR_ELT (init, 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)
> +		{
> +		  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 (init, 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 (init))
>  	    {
>  	      /* Diagnose changing the active union member while the union
>  		 is in the process of being initialized.  */
> @@ -6292,11 +6354,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>  		error_at (cp_expr_loc_or_input_loc (t),
>  			  "change of the active member of a union "
>  			  "from %qD to %qD during initialization",
> -			  CONSTRUCTOR_ELT (*valp, 0)->index,
> +			  CONSTRUCTOR_ELT (init, 0)->index,
>  			  index);
>  	      *non_constant_p = true;
>  	    }
> -	  no_zero_init = true;
>  	}
>  
>        ctors.safe_push (valp);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index fd6bf9fd4a0..40aa9f0be2b 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/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..c94d7cc0fc9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C
> @@ -0,0 +1,71 @@
> +// { 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 {};  // value initialisation initialises first member
> +  int* p = &u.a;
> +  *p = 10;
> +  return *p;
> +}
> +constexpr int x7 = test7();
> +
> +constexpr int test8() {
> +  U u;  // default initialisation leaves no member initialised
> +  int* p = &u.a;
> +  *p = 10;  // { dg-error "accessing" }
> +  return *p;
> +}
> +constexpr int x8 = test8();  // { dg-message "in .constexpr. expansion" }
> 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..7c37716892c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C
> @@ -0,0 +1,43 @@
> +// { 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" }
> +
> +struct S2 { int a; int b; };
> +union U2 { int k; S2 s; };
> +constexpr int test3() {
> +  U2 u;
> +  int* p = &u.s.b;
> +  std::construct_at(p, 8);  // { dg-message "in .constexpr. expansion" }
> +  return u.s.b;
> +};
> +constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }
> +
> +constexpr int test4() {
> +  union {
> +    int data[1];
> +  } u;
> +  std::construct_at(u.data, 0);  // { dg-message "in .constexpr. expansion" }
> +  return 0;
> +}
> +constexpr int x4 = test4();  // { dg-message "in .constexpr. expansion" }
> +
> +// { dg-error "accessing uninitialized member" "" { target *-*-* } 0 }
> -- 
> 2.41.0
>
  
Jonathan Wakely Sept. 23, 2023, 6:40 a.m. UTC | #2
On Sat, 23 Sept 2023, 01:39 Nathaniel Shead via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:

> Now that bootstrap has finished, I have gotten regressions in the
> following libstdc++ tests:
>
> Running libstdc++:libstdc++-dg/conformance.exp ...
> FAIL: 20_util/bitset/access/constexpr.cc -std=gnu++23 (test for excess
> errors)
> FAIL: 20_util/bitset/access/constexpr.cc -std=gnu++26 (test for excess
> errors)
> FAIL: 20_util/variant/constexpr.cc -std=gnu++20 (test for excess errors)
> FAIL: 20_util/variant/constexpr.cc -std=gnu++26 (test for excess errors)
> FAIL: 21_strings/basic_string/cons/char/constexpr.cc -std=gnu++20 (test
> for excess errors)
> FAIL: 21_strings/basic_string/cons/char/constexpr.cc -std=gnu++26 (test
> for excess errors)
> FAIL: 21_strings/basic_string/cons/wchar_t/constexpr.cc -std=gnu++20 (test
> for excess errors)
> FAIL: 21_strings/basic_string/cons/wchar_t/constexpr.cc -std=gnu++26 (test
> for excess errors)
> FAIL: 21_strings/basic_string/modifiers/swap/constexpr-wchar_t.cc
> -std=gnu++20 (test for excess errors)
> FAIL: 21_strings/basic_string/modifiers/swap/constexpr-wchar_t.cc
> -std=gnu++26 (test for excess errors)
> FAIL: 21_strings/basic_string/modifiers/swap/constexpr.cc -std=gnu++20
> (test for excess errors)
> FAIL: 21_strings/basic_string/modifiers/swap/constexpr.cc -std=gnu++26
> (test for excess errors)
> FAIL: std/ranges/adaptors/join_with/1.cc -std=gnu++23 (test for excess
> errors)
> UNRESOLVED: std/ranges/adaptors/join_with/1.cc -std=gnu++23 compilation
> failed to produce executable
> FAIL: std/ranges/adaptors/join_with/1.cc -std=gnu++26 (test for excess
> errors)
> UNRESOLVED: std/ranges/adaptors/join_with/1.cc -std=gnu++26 compilation
> failed to produce executable
>
> On investigation though it looks like the issue might be with libstdc++
> rather than the patch itself; running the failing tests using clang with
> libstdc++ also produces similar errors, and my reading of the code
> suggests that this is correct.
>
> What's the way forward here? Should I look at creating a patch to fix
> the libstdc++ issues before resubmitting this patch for the C++
> frontend? Or should I submit a version of this patch without the
> `std::construct_at` changes and wait till libstdc++ gets fixed for that?
>

I think we should fix libstdc++. There are probably only a few places that
need a fix, which cause all those failures.

I can help with those fixes. I'll look into it after the weekend.



> On Sat, Sep 23, 2023 at 01:01:20AM +1000, Nathaniel Shead wrote:
> > On Fri, Sep 22, 2023 at 02:21:15PM +0100, Jason Merrill wrote:
> > > On 9/21/23 09:41, Nathaniel Shead wrote:
> > > > I've updated the error messages, and also fixed another bug I found
> > > > while retesting (value-initialised unions weren't considered to have
> any
> > > > active member yet).
> > > >
> > > > 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, which catches some additional mistakes pre-C++20.
> > > >
> > > >   PR c++/101631
> > > >
> > > > 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_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/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.
> > > >
> > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > > ---
> > > >   gcc/cp/call.cc                                |  11 +-
> > > >   gcc/cp/class.cc                               |   8 ++
> > > >   gcc/cp/constexpr.cc                           | 129
> +++++++++++++-----
> > > >   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/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 |  71 ++++++++++
> > > >   10 files changed, 296 insertions(+), 43 deletions(-)
> > > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.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
> > > >
> > > > 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 a673a6022f1..5e50fc2c792 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
> > > > @@ -6127,6 +6121,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;
> > > > @@ -6135,6 +6137,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);
> > > > @@ -6142,6 +6145,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);
> > > > @@ -6230,6 +6234,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)
> > > >           {
> > > > @@ -6270,21 +6275,72 @@ cxx_eval_store_expression (const
> constexpr_ctx *ctx, tree t,
> > > >         type = reftype;
> > > > -      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> > > > -   && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> > > > +      if (code == UNION_TYPE
> > > > +   && TREE_CODE (t) == MODIFY_EXPR
> > > > +   && (CONSTRUCTOR_NELTS (*valp) == 0
> > > > +       || CONSTRUCTOR_ELT (*valp, 0)->index != index))
> > > >           {
> > > > -   if (cxx_dialect < cxx20)
> > > > +   /* Probably a change of active member for a union. Ensure that
> > > > +      this is valid.  */
> > > > +   no_zero_init = true;
> > > > +
> > > > +   tree init = *valp;
> > > > +   if (!CONSTRUCTOR_NO_CLEARING (*valp) && CONSTRUCTOR_NELTS
> (*valp) == 0)
> > > > +     /* This is a value-initialized union, process the
> initialization
> > > > +        to ensure that the active member is properly set.  */
> > > > +     init = build_value_init (TREE_TYPE (*valp),
> tf_warning_or_error);
> > > > +
> > > > +   bool has_active_member = CONSTRUCTOR_NELTS (init) != 0;
> > > > +   tree inner = strip_array_types (reftype);
> > > > +
> > > > +   if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index ==
> index)
> > > > +     /* After handling value-initialization, this wasn't actually a
> > > > +        change of active member like we initially thought.  */
> > > > +     no_zero_init = false;
> > > > +   else 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",
> > > > -                   CONSTRUCTOR_ELT (*valp, 0)->index,
> > > > +                   "from %qD to %qD is not a constant expression "
> > > > +                   "before C++20",
> > > > +                   CONSTRUCTOR_ELT (init, 0)->index,
> > > >                             index);
> > > >                 *non_constant_p = true;
> > > >               }
> > > > -   else if (TREE_CODE (t) == MODIFY_EXPR
> > > > -            && CONSTRUCTOR_NO_CLEARING (*valp))
> > > > +   else if (!is_access_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)
> > > > +         {
> > > > +           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 (init, 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",
> > > > +                     index, inner);
> > >
> > > It now occurs to me that this should suggest using placement new
> instead.
> > > And that we should test using placement new to begin the lifetime of
> such
> > > members.
> > >
> > > Jason
> > >
> >
> > Good point. On testing placement new I found I wasn't checking for
> > indirect change of active member during initialisation either, but this
> > was pretty trivial to add. How does this look?
> >
> > Bootstrap + regtest ongoing, but dg.exp completed with no errors. I'm
> > away from my computer next week so I thought I'd send this through now
> > anyway.
> >
> > -- >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, 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_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/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                           | 135 +++++++++++++-----
> >  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/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 |  71 +++++++++
> >  gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C |  43 ++++++
> >  11 files changed, 345 insertions(+), 43 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.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
> >
> > 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 a673a6022f1..8e6f60d03d2 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
> > @@ -6127,6 +6121,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;
> > @@ -6135,6 +6137,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);
> > @@ -6142,6 +6145,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);
> > @@ -6230,6 +6234,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)
> >       {
> > @@ -6270,21 +6275,78 @@ cxx_eval_store_expression (const constexpr_ctx
> *ctx, tree t,
> >
> >        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
> > +       && (TREE_CODE (t) == MODIFY_EXPR
> > +           /* Also check if initializations have implicit change of
> active
> > +              member earlier up the access chain.  */
> > +           || !refs->is_empty())
> > +       && (CONSTRUCTOR_NELTS (*valp) == 0
> > +           || CONSTRUCTOR_ELT (*valp, 0)->index != index))
> >       {
> > -       if (cxx_dialect < cxx20)
> > +       no_zero_init = true;
> > +
> > +       tree init = *valp;
> > +       if (TREE_CODE (t) == MODIFY_EXPR
> > +           && !CONSTRUCTOR_NO_CLEARING (*valp)
> > +           && CONSTRUCTOR_NELTS (*valp) == 0)
> > +         /* Modifying a value-initialized union, process the
> initialization
> > +            to ensure that the active member is properly set.  */
> > +         init = build_value_init (TREE_TYPE (*valp),
> tf_warning_or_error);
> > +
> > +       bool has_active_member = CONSTRUCTOR_NELTS (init) != 0;
> > +       tree inner = strip_array_types (reftype);
> > +
> > +       if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index ==
> index)
> > +         /* After handling value-initialization, this wasn't actually a
> > +            change of active member like we initially thought.  */
> > +         no_zero_init = false;
> > +       else 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",
> > -                       CONSTRUCTOR_ELT (*valp, 0)->index,
> > +                       "from %qD to %qD is not a constant expression "
> > +                       "before C++20",
> > +                       CONSTRUCTOR_ELT (init, 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)
> > +             {
> > +               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 (init, 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 (init))
> >           {
> >             /* Diagnose changing the active union member while the union
> >                is in the process of being initialized.  */
> > @@ -6292,11 +6354,10 @@ cxx_eval_store_expression (const constexpr_ctx
> *ctx, tree t,
> >               error_at (cp_expr_loc_or_input_loc (t),
> >                         "change of the active member of a union "
> >                         "from %qD to %qD during initialization",
> > -                       CONSTRUCTOR_ELT (*valp, 0)->index,
> > +                       CONSTRUCTOR_ELT (init, 0)->index,
> >                         index);
> >             *non_constant_p = true;
> >           }
> > -       no_zero_init = true;
> >       }
> >
> >        ctors.safe_push (valp);
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index fd6bf9fd4a0..40aa9f0be2b 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/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..c94d7cc0fc9
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C
> > @@ -0,0 +1,71 @@
> > +// { 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 {};  // value initialisation initialises first member
> > +  int* p = &u.a;
> > +  *p = 10;
> > +  return *p;
> > +}
> > +constexpr int x7 = test7();
> > +
> > +constexpr int test8() {
> > +  U u;  // default initialisation leaves no member initialised
> > +  int* p = &u.a;
> > +  *p = 10;  // { dg-error "accessing" }
> > +  return *p;
> > +}
> > +constexpr int x8 = test8();  // { dg-message "in .constexpr. expansion"
> }
> > 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..7c37716892c
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C
> > @@ -0,0 +1,43 @@
> > +// { 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"
> }
> > +
> > +struct S2 { int a; int b; };
> > +union U2 { int k; S2 s; };
> > +constexpr int test3() {
> > +  U2 u;
> > +  int* p = &u.s.b;
> > +  std::construct_at(p, 8);  // { dg-message "in .constexpr. expansion" }
> > +  return u.s.b;
> > +};
> > +constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion"
> }
> > +
> > +constexpr int test4() {
> > +  union {
> > +    int data[1];
> > +  } u;
> > +  std::construct_at(u.data, 0);  // { dg-message "in .constexpr.
> expansion" }
> > +  return 0;
> > +}
> > +constexpr int x4 = test4();  // { dg-message "in .constexpr. expansion"
> }
> > +
> > +// { dg-error "accessing uninitialized member" "" { target *-*-* } 0 }
> > --
> > 2.41.0
> >
>
  

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 a673a6022f1..8e6f60d03d2 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
@@ -6127,6 +6121,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;
@@ -6135,6 +6137,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);
@@ -6142,6 +6145,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);
@@ -6230,6 +6234,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)
 	{
@@ -6270,21 +6275,78 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 
       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
+	  && (TREE_CODE (t) == MODIFY_EXPR
+	      /* Also check if initializations have implicit change of active
+		 member earlier up the access chain.  */
+	      || !refs->is_empty())
+	  && (CONSTRUCTOR_NELTS (*valp) == 0
+	      || CONSTRUCTOR_ELT (*valp, 0)->index != index))
 	{
-	  if (cxx_dialect < cxx20)
+	  no_zero_init = true;
+
+	  tree init = *valp;
+	  if (TREE_CODE (t) == MODIFY_EXPR
+	      && !CONSTRUCTOR_NO_CLEARING (*valp)
+	      && CONSTRUCTOR_NELTS (*valp) == 0)
+	    /* Modifying a value-initialized union, process the initialization
+	       to ensure that the active member is properly set.  */
+	    init = build_value_init (TREE_TYPE (*valp), tf_warning_or_error);
+
+	  bool has_active_member = CONSTRUCTOR_NELTS (init) != 0;
+	  tree inner = strip_array_types (reftype);
+
+	  if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == index)
+	    /* After handling value-initialization, this wasn't actually a
+	       change of active member like we initially thought.  */
+	    no_zero_init = false;
+	  else 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",
-			  CONSTRUCTOR_ELT (*valp, 0)->index,
+			  "from %qD to %qD is not a constant expression "
+			  "before C++20",
+			  CONSTRUCTOR_ELT (init, 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)
+		{
+		  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 (init, 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 (init))
 	    {
 	      /* Diagnose changing the active union member while the union
 		 is in the process of being initialized.  */
@@ -6292,11 +6354,10 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 		error_at (cp_expr_loc_or_input_loc (t),
 			  "change of the active member of a union "
 			  "from %qD to %qD during initialization",
-			  CONSTRUCTOR_ELT (*valp, 0)->index,
+			  CONSTRUCTOR_ELT (init, 0)->index,
 			  index);
 	      *non_constant_p = true;
 	    }
-	  no_zero_init = true;
 	}
 
       ctors.safe_push (valp);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index fd6bf9fd4a0..40aa9f0be2b 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/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..c94d7cc0fc9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C
@@ -0,0 +1,71 @@ 
+// { 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 {};  // value initialisation initialises first member
+  int* p = &u.a;
+  *p = 10;
+  return *p;
+}
+constexpr int x7 = test7();
+
+constexpr int test8() {
+  U u;  // default initialisation leaves no member initialised
+  int* p = &u.a;
+  *p = 10;  // { dg-error "accessing" }
+  return *p;
+}
+constexpr int x8 = test8();  // { dg-message "in .constexpr. expansion" }
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..7c37716892c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C
@@ -0,0 +1,43 @@ 
+// { 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" }
+
+struct S2 { int a; int b; };
+union U2 { int k; S2 s; };
+constexpr int test3() {
+  U2 u;
+  int* p = &u.s.b;
+  std::construct_at(p, 8);  // { dg-message "in .constexpr. expansion" }
+  return u.s.b;
+};
+constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }
+
+constexpr int test4() {
+  union {
+    int data[1];
+  } u;
+  std::construct_at(u.data, 0);  // { dg-message "in .constexpr. expansion" }
+  return 0;
+}
+constexpr int x4 = test4();  // { dg-message "in .constexpr. expansion" }
+
+// { dg-error "accessing uninitialized member" "" { target *-*-* } 0 }