[PATCH] c++: Fix constexpr evaluation of self-modifying CONSTRUCTORs [PR94219]

[PATCH] c++: Fix constexpr evaluation of self-modifying CONSTRUCTORs [PR94219]

[Patrick Palka]

This PR reveals that cxx_eval_bare_aggregate and cxx_eval_store_expression do
not anticipate that a constructor element's initializer could mutate the
underlying CONSTRUCTOR.  Evaluation of the initializer could add new elements to
the underlying CONSTRUCTOR, thereby potentially invalidating any pointers to
or assumptions about the CONSTRUCTOR's elements, and so these routines should be
prepared for that.

To fix this problem, this patch makes cxx_eval_bare_aggregate and
cxx_eval_store_expression recompute the pointer to the constructor_elt's through
which we're assigning, after it evaluates the initializer.  Care is taken to
to make the common case where the initializer does not modify the underlying
CONSTRUCTOR as fast as before.

Does this look OK to commit after testing?

gcc/cp/ChangeLog:

	PR c++/94205
	PR c++/94219
	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
	support for VECTOR_TYPEs, and optimizations for the common case)
	from ...
	(cxx_eval_store_expression): ... here.  Rename local variable
	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
	the sequence of indexes into 'indexes' that yields the subobject we're
	assigning to.  Record the integer offsets of the constructor indexes
	we're assigning through into 'index_pos_hints'.  After evaluating the
	initializer of the store expression, recompute 'valp' using 'indexes'
	and 'index_pos_hints' as hints.
	(cxx_eval_bare_aggregate): Tweak comments.  Use get_or_insert_ctor_field
	to recompute the pointer to the constructor_elt we're assigning through
	after evaluating each initializer.

gcc/testsuite/ChangeLog:

	PR c++/94205
	PR c++/94219
	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
	* g++.dg/cpp1z/lambda-this5.C: New test.
---
 gcc/cp/constexpr.c                            | 252 +++++++++++-------
 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
 5 files changed, 228 insertions(+), 97 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 91f0c3ba269..b4173c595f0 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3151,6 +3151,97 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert)
   return -1;
 }
 
+/* Return a pointer to the constructor_elt of CTOR which matches INDEX.  If no
+   matching constructor_elt exists, then add one to CTOR.
+
+   As an optimization, if POS_HINT is non-negative then it is used as a guess
+   for the (integer) index of the matching constructor_elt within CTOR.  */
+
+static constructor_elt *
+get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
+{
+  tree type = TREE_TYPE (ctor);
+  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
+    {
+      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
+      return &CONSTRUCTOR_ELTS (ctor)->last();
+    }
+  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
+    {
+      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
+      gcc_assert (i >= 0);
+      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
+      gcc_assert (cep->index == NULL_TREE
+		  || TREE_CODE (cep->index) != RANGE_EXPR);
+      return cep;
+    }
+  else
+    {
+      gcc_assert (TREE_CODE (index) == FIELD_DECL);
+
+      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
+	 Usually we meet initializers in that order, but it is
+	 possible for base types to be placed not in program
+	 order.  */
+      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
+      unsigned HOST_WIDE_INT idx = 0;
+      constructor_elt *cep = NULL;
+
+      /* First, check if we're changing the active member of a union.  */
+      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
+	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
+	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
+      /* Next, check the hint.  */
+      else if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor)
+	       && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
+	{
+	  cep = CONSTRUCTOR_ELT (ctor, pos_hint);
+	  goto found;
+	}
+      /* If the hint was wrong, and if the bit offset of INDEX is larger than
+	 that of the last constructor_elt, then we can just immediately append a
+	 new constructor_elt to the end of CTOR.  */
+      else if (CONSTRUCTOR_NELTS (ctor)
+	       && tree_int_cst_compare (bit_position (index),
+					bit_position (CONSTRUCTOR_ELTS (ctor)
+						      ->last().index)) > 0)
+	{
+	  idx = CONSTRUCTOR_NELTS (ctor);
+	  goto insert;
+	}
+
+      /* Otherwise, we need to iterator over CTOR to find or insert INDEX
+	 appropriately.  */
+
+      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
+	   idx++, fields = DECL_CHAIN (fields))
+	{
+	  if (index == cep->index)
+	    goto found;
+
+	  /* The field we're initializing must be on the field
+	     list.  Look to see if it is present before the
+	     field the current ELT initializes.  */
+	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
+	    if (index == fields)
+	      goto insert;
+	}
+      /* We fell off the end of the CONSTRUCTOR, so insert a new
+	 entry at the end.  */
+
+    insert:
+      {
+	constructor_elt ce = { index, NULL_TREE };
+
+	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
+	cep = CONSTRUCTOR_ELT (ctor, idx);
+      }
+    found:;
+
+      return cep;
+    }
+}
+
 /* Under the control of CTX, issue a detailed diagnostic for
    an out-of-bounds subscript INDEX into the expression ARRAY.  */
 
@@ -3760,14 +3851,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
     {
       tree orig_value = value;
       init_subob_ctx (ctx, new_ctx, index, value);
+      int pos_hint = -1;
       if (new_ctx.ctor != ctx->ctor)
-	/* If we built a new CONSTRUCTOR, attach it now so that other
-	   initializers can refer to it.  */
-	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
+	{
+	  /* If we built a new CONSTRUCTOR, attach it now so that other
+	     initializers can refer to it.  */
+	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
+	  pos_hint = vec_safe_length (*p) - 1;
+	}
       else if (TREE_CODE (type) == UNION_TYPE)
-	/* Otherwise if we're constructing a union, set the active union member
-	   anyway so that we can later detect if the initializer attempts to
-	   activate another member.  */
+	/* Otherwise if we're constructing a non-aggregate union member, set
+	   the active union member now so that we can later detect and diagnose
+	   if its initializer attempts to activate another member.  */
 	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
       tree elt = cxx_eval_constant_expression (&new_ctx, value,
 					       lval,
@@ -3804,18 +3899,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
 	{
 	  if (TREE_CODE (type) == UNION_TYPE
 	      && (*p)->last().index != index)
-	    /* The initializer may have erroneously changed the active union
-	       member that we're initializing.  */
+	    /* The initializer erroneously changed the active union member that
+	       we're initializing.  */
 	    gcc_assert (*non_constant_p);
-	  else if (new_ctx.ctor != ctx->ctor
-		   || TREE_CODE (type) == UNION_TYPE)
+	  else
 	    {
-	      /* We appended this element above; update the value.  */
-	      gcc_assert ((*p)->last().index == index);
-	      (*p)->last().value = elt;
+	      /* The initializer might have mutated the underlying CONSTRUCTOR,
+		 so recompute the location of the target constructer_elt.  */
+	      constructor_elt *cep
+		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
+	      cep->value = elt;
 	    }
-	  else
-	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
+
 	  /* Adding or replacing an element might change the ctor's flags.  */
 	  TREE_CONSTANT (ctx->ctor) = constant_p;
 	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
@@ -4590,8 +4685,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
   type = TREE_TYPE (object);
   bool no_zero_init = true;
 
-  releasing_vec ctors;
-  bool changed_active_union_member_p = false;
+  releasing_vec ctors, indexes;
+  auto_vec<int> index_pos_hints;
+  bool activated_union_member_p = false;
   while (!refs->is_empty ())
     {
       if (*valp == NULL_TREE)
@@ -4632,94 +4728,49 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 	 subobjects will also be zero-initialized.  */
       no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
 
-      vec_safe_push (ctors, *valp);
-
       enum tree_code code = TREE_CODE (type);
       type = refs->pop();
       tree index = refs->pop();
 
-      constructor_elt *cep = NULL;
-      if (code == ARRAY_TYPE)
+      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
+	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
 	{
-	  HOST_WIDE_INT i
-	    = find_array_ctor_elt (*valp, index, /*insert*/true);
-	  gcc_assert (i >= 0);
-	  cep = CONSTRUCTOR_ELT (*valp, i);
-	  gcc_assert (cep->index == NULL_TREE
-		      || TREE_CODE (cep->index) != RANGE_EXPR);
-	}
-      else
-	{
-	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
-
-	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
-	     Usually we meet initializers in that order, but it is
-	     possible for base types to be placed not in program
-	     order.  */
-	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
-	  unsigned HOST_WIDE_INT idx;
-
-	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
-	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
+	  if (cxx_dialect < cxx2a)
 	    {
-	      if (cxx_dialect < cxx2a)
-		{
-		  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,
-			      index);
-		  *non_constant_p = true;
-		}
-	      else if (TREE_CODE (t) == MODIFY_EXPR
-		       && CONSTRUCTOR_NO_CLEARING (*valp))
-		{
-		  /* Diagnose changing the active union member while the union
-		     is in the process of being initialized.  */
-		  if (!ctx->quiet)
-		    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,
-			      index);
-		  *non_constant_p = true;
-		}
-	      /* Changing active member.  */
-	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
-	      no_zero_init = true;
+	      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,
+			  index);
+	      *non_constant_p = true;
 	    }
-
-	  for (idx = 0;
-	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
-	       idx++, fields = DECL_CHAIN (fields))
+	  else if (TREE_CODE (t) == MODIFY_EXPR
+		   && CONSTRUCTOR_NO_CLEARING (*valp))
 	    {
-	      if (index == cep->index)
-		goto found;
-
-	      /* The field we're initializing must be on the field
-		 list.  Look to see if it is present before the
-		 field the current ELT initializes.  */
-	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
-		if (index == fields)
-		  goto insert;
+	      /* Diagnose changing the active union member while the union
+		 is in the process of being initialized.  */
+	      if (!ctx->quiet)
+		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,
+			  index);
+	      *non_constant_p = true;
 	    }
+	  no_zero_init = true;
+	}
 
-	  /* We fell off the end of the CONSTRUCTOR, so insert a new
-	     entry at the end.  */
-	insert:
-	  {
-	    constructor_elt ce = { index, NULL_TREE };
+      vec_safe_push (ctors, *valp);
+      vec_safe_push (indexes, index);
 
-	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
-	    cep = CONSTRUCTOR_ELT (*valp, idx);
+      constructor_elt *cep
+	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
+      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELT (*valp, 0));
+
+      if (code == UNION_TYPE)
+	activated_union_member_p = true;
 
-	    if (code == UNION_TYPE)
-	      /* Record that we've changed an active union member.  */
-	      changed_active_union_member_p = true;
-	  }
-	found:;
-	}
       valp = &cep->value;
     }
 
@@ -4800,9 +4851,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 	  init = tinit;
       init = cxx_eval_constant_expression (&new_ctx, init, false,
 					   non_constant_p, overflow_p);
-      if (ctors->is_empty())
-	/* The hash table might have moved since the get earlier.  */
-	valp = ctx->global->values.get (object);
+      /* The hash table might have moved since the get earlier.  */
+      valp = ctx->global->values.get (object);
+      /* The initializer might have mutated the underlying CONSTRUCTORs, so we
+	 must recompute VALP.  */
+      for (unsigned i = 0; i < vec_safe_length (indexes); i++)
+	{
+	  constructor_elt *cep
+	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
+	  valp = &cep->value;
+	}
     }
 
   /* Don't share a CONSTRUCTOR that might be changed later.  */
@@ -4847,7 +4905,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
   unsigned i;
   bool c = TREE_CONSTANT (init);
   bool s = TREE_SIDE_EFFECTS (init);
-  if (!c || s || changed_active_union_member_p)
+  if (!c || s || activated_union_member_p)
     FOR_EACH_VEC_ELT (*ctors, i, elt)
       {
 	if (!c)
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
new file mode 100644
index 00000000000..0f91e93ca3f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
@@ -0,0 +1,19 @@
+// PR c++/94205
+// { dg-do compile { target c++14 } }
+
+struct S
+{
+  struct A
+  {
+    S *p;
+    constexpr A(S* p): p(p) {}
+    constexpr operator int() { p->a = 5; return 6; }
+  };
+  int a = A(this);
+};
+
+
+constexpr S s = {};
+
+#define SA(X) static_assert((X),#X)
+SA(s.a == 6);
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
new file mode 100644
index 00000000000..0ceef1bb29f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
@@ -0,0 +1,21 @@
+// PR c++/94219
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+struct U;
+constexpr A foo(U *up);
+
+struct U {
+  A a = foo(this); int y;
+};
+
+constexpr A foo(U *up) {
+  up->y = 11;
+  return {42};
+}
+
+extern constexpr U u = {};
+
+static_assert(u.y == 11, "");
+static_assert(u.a.x == 42, "");
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
new file mode 100644
index 00000000000..59e7a10d6e8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
@@ -0,0 +1,22 @@
+// PR c++/94219
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+struct U;
+constexpr A foo(U *up);
+
+struct U {
+  U() = default;
+  int y; A a = foo(this);
+};
+
+constexpr A foo(U *up) {
+  up->y = 11;
+  return {42};
+}
+
+extern constexpr U u = {};
+
+static_assert(u.y == 11, "");
+static_assert(u.a.x == 42, "");
diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
new file mode 100644
index 00000000000..c6d44d7fd0b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
@@ -0,0 +1,11 @@
+// PR c++/94205
+// { dg-do compile { target c++17 } }
+
+struct S
+{
+  int a = [this] { this->a = 5; return 6; } ();
+};
+
+constexpr S s = {};
+
+static_assert(s.a == 6);
-- 
2.26.0.106.g9fadedd637

Re: [PATCH] c++: Fix constexpr evaluation of self-modifying CONSTRUCTORs [PR94219]

[Patrick Palka]

On Thu, 2 Apr 2020, Patrick Palka wrote:

> This PR reveals that cxx_eval_bare_aggregate and cxx_eval_store_expression do
> not anticipate that a constructor element's initializer could mutate the
> underlying CONSTRUCTOR.  Evaluation of the initializer could add new elements to
> the underlying CONSTRUCTOR, thereby potentially invalidating any pointers to
> or assumptions about the CONSTRUCTOR's elements, and so these routines should be
> prepared for that.
> 
> To fix this problem, this patch makes cxx_eval_bare_aggregate and
> cxx_eval_store_expression recompute the pointer to the constructor_elt's through
> which we're assigning, after it evaluates the initializer.  Care is taken to
> to make the common case where the initializer does not modify the underlying
> CONSTRUCTOR as fast as before.

Also, with this patch, I'm not totally sure but I think we might not
need the special preeval handling in cxx_eval_store_expression anymore.
I could try to remove it in a subsequent patch.

> 
> Does this look OK to commit after testing?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/94205
> 	PR c++/94219
> 	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
> 	support for VECTOR_TYPEs, and optimizations for the common case)
> 	from ...
> 	(cxx_eval_store_expression): ... here.  Rename local variable
> 	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
> 	the sequence of indexes into 'indexes' that yields the subobject we're
> 	assigning to.  Record the integer offsets of the constructor indexes
> 	we're assigning through into 'index_pos_hints'.  After evaluating the
> 	initializer of the store expression, recompute 'valp' using 'indexes'
> 	and 'index_pos_hints' as hints.
> 	(cxx_eval_bare_aggregate): Tweak comments.  Use get_or_insert_ctor_field
> 	to recompute the pointer to the constructor_elt we're assigning through
> 	after evaluating each initializer.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/94205
> 	PR c++/94219
> 	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
> 	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
> 	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
> 	* g++.dg/cpp1z/lambda-this5.C: New test.
> ---
>  gcc/cp/constexpr.c                            | 252 +++++++++++-------
>  gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
>  gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
>  gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
>  gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
>  5 files changed, 228 insertions(+), 97 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 91f0c3ba269..b4173c595f0 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -3151,6 +3151,97 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert)
>    return -1;
>  }
>  
> +/* Return a pointer to the constructor_elt of CTOR which matches INDEX.  If no
> +   matching constructor_elt exists, then add one to CTOR.
> +
> +   As an optimization, if POS_HINT is non-negative then it is used as a guess
> +   for the (integer) index of the matching constructor_elt within CTOR.  */
> +
> +static constructor_elt *
> +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
> +{
> +  tree type = TREE_TYPE (ctor);
> +  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
> +    {
> +      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
> +      return &CONSTRUCTOR_ELTS (ctor)->last();
> +    }
> +  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
> +    {
> +      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
> +      gcc_assert (i >= 0);
> +      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
> +      gcc_assert (cep->index == NULL_TREE
> +		  || TREE_CODE (cep->index) != RANGE_EXPR);
> +      return cep;
> +    }
> +  else
> +    {
> +      gcc_assert (TREE_CODE (index) == FIELD_DECL);
> +
> +      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
> +	 Usually we meet initializers in that order, but it is
> +	 possible for base types to be placed not in program
> +	 order.  */
> +      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
> +      unsigned HOST_WIDE_INT idx = 0;
> +      constructor_elt *cep = NULL;
> +
> +      /* First, check if we're changing the active member of a union.  */
> +      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
> +	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
> +	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
> +      /* Next, check the hint.  */
> +      else if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor)
> +	       && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
> +	{
> +	  cep = CONSTRUCTOR_ELT (ctor, pos_hint);
> +	  goto found;
> +	}
> +      /* If the hint was wrong, and if the bit offset of INDEX is larger than
> +	 that of the last constructor_elt, then we can just immediately append a
> +	 new constructor_elt to the end of CTOR.  */
> +      else if (CONSTRUCTOR_NELTS (ctor)
> +	       && tree_int_cst_compare (bit_position (index),
> +					bit_position (CONSTRUCTOR_ELTS (ctor)
> +						      ->last().index)) > 0)
> +	{
> +	  idx = CONSTRUCTOR_NELTS (ctor);
> +	  goto insert;
> +	}
> +
> +      /* Otherwise, we need to iterator over CTOR to find or insert INDEX
> +	 appropriately.  */
> +
> +      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
> +	   idx++, fields = DECL_CHAIN (fields))
> +	{
> +	  if (index == cep->index)
> +	    goto found;
> +
> +	  /* The field we're initializing must be on the field
> +	     list.  Look to see if it is present before the
> +	     field the current ELT initializes.  */
> +	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
> +	    if (index == fields)
> +	      goto insert;
> +	}
> +      /* We fell off the end of the CONSTRUCTOR, so insert a new
> +	 entry at the end.  */
> +
> +    insert:
> +      {
> +	constructor_elt ce = { index, NULL_TREE };
> +
> +	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
> +	cep = CONSTRUCTOR_ELT (ctor, idx);
> +      }
> +    found:;
> +
> +      return cep;
> +    }
> +}
> +
>  /* Under the control of CTX, issue a detailed diagnostic for
>     an out-of-bounds subscript INDEX into the expression ARRAY.  */
>  
> @@ -3760,14 +3851,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>      {
>        tree orig_value = value;
>        init_subob_ctx (ctx, new_ctx, index, value);
> +      int pos_hint = -1;
>        if (new_ctx.ctor != ctx->ctor)
> -	/* If we built a new CONSTRUCTOR, attach it now so that other
> -	   initializers can refer to it.  */
> -	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> +	{
> +	  /* If we built a new CONSTRUCTOR, attach it now so that other
> +	     initializers can refer to it.  */
> +	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> +	  pos_hint = vec_safe_length (*p) - 1;
> +	}
>        else if (TREE_CODE (type) == UNION_TYPE)
> -	/* Otherwise if we're constructing a union, set the active union member
> -	   anyway so that we can later detect if the initializer attempts to
> -	   activate another member.  */
> +	/* Otherwise if we're constructing a non-aggregate union member, set
> +	   the active union member now so that we can later detect and diagnose
> +	   if its initializer attempts to activate another member.  */
>  	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
>        tree elt = cxx_eval_constant_expression (&new_ctx, value,
>  					       lval,
> @@ -3804,18 +3899,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>  	{
>  	  if (TREE_CODE (type) == UNION_TYPE
>  	      && (*p)->last().index != index)
> -	    /* The initializer may have erroneously changed the active union
> -	       member that we're initializing.  */
> +	    /* The initializer erroneously changed the active union member that
> +	       we're initializing.  */
>  	    gcc_assert (*non_constant_p);
> -	  else if (new_ctx.ctor != ctx->ctor
> -		   || TREE_CODE (type) == UNION_TYPE)
> +	  else
>  	    {
> -	      /* We appended this element above; update the value.  */
> -	      gcc_assert ((*p)->last().index == index);
> -	      (*p)->last().value = elt;
> +	      /* The initializer might have mutated the underlying CONSTRUCTOR,
> +		 so recompute the location of the target constructer_elt.  */
> +	      constructor_elt *cep
> +		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
> +	      cep->value = elt;
>  	    }
> -	  else
> -	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
> +
>  	  /* Adding or replacing an element might change the ctor's flags.  */
>  	  TREE_CONSTANT (ctx->ctor) = constant_p;
>  	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
> @@ -4590,8 +4685,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>    type = TREE_TYPE (object);
>    bool no_zero_init = true;
>  
> -  releasing_vec ctors;
> -  bool changed_active_union_member_p = false;
> +  releasing_vec ctors, indexes;
> +  auto_vec<int> index_pos_hints;
> +  bool activated_union_member_p = false;
>    while (!refs->is_empty ())
>      {
>        if (*valp == NULL_TREE)
> @@ -4632,94 +4728,49 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>  	 subobjects will also be zero-initialized.  */
>        no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
>  
> -      vec_safe_push (ctors, *valp);
> -
>        enum tree_code code = TREE_CODE (type);
>        type = refs->pop();
>        tree index = refs->pop();
>  
> -      constructor_elt *cep = NULL;
> -      if (code == ARRAY_TYPE)
> +      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> +	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>  	{
> -	  HOST_WIDE_INT i
> -	    = find_array_ctor_elt (*valp, index, /*insert*/true);
> -	  gcc_assert (i >= 0);
> -	  cep = CONSTRUCTOR_ELT (*valp, i);
> -	  gcc_assert (cep->index == NULL_TREE
> -		      || TREE_CODE (cep->index) != RANGE_EXPR);
> -	}
> -      else
> -	{
> -	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
> -
> -	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
> -	     Usually we meet initializers in that order, but it is
> -	     possible for base types to be placed not in program
> -	     order.  */
> -	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
> -	  unsigned HOST_WIDE_INT idx;
> -
> -	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> -	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> +	  if (cxx_dialect < cxx2a)
>  	    {
> -	      if (cxx_dialect < cxx2a)
> -		{
> -		  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,
> -			      index);
> -		  *non_constant_p = true;
> -		}
> -	      else if (TREE_CODE (t) == MODIFY_EXPR
> -		       && CONSTRUCTOR_NO_CLEARING (*valp))
> -		{
> -		  /* Diagnose changing the active union member while the union
> -		     is in the process of being initialized.  */
> -		  if (!ctx->quiet)
> -		    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,
> -			      index);
> -		  *non_constant_p = true;
> -		}
> -	      /* Changing active member.  */
> -	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
> -	      no_zero_init = true;
> +	      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,
> +			  index);
> +	      *non_constant_p = true;
>  	    }
> -
> -	  for (idx = 0;
> -	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
> -	       idx++, fields = DECL_CHAIN (fields))
> +	  else if (TREE_CODE (t) == MODIFY_EXPR
> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
>  	    {
> -	      if (index == cep->index)
> -		goto found;
> -
> -	      /* The field we're initializing must be on the field
> -		 list.  Look to see if it is present before the
> -		 field the current ELT initializes.  */
> -	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
> -		if (index == fields)
> -		  goto insert;
> +	      /* Diagnose changing the active union member while the union
> +		 is in the process of being initialized.  */
> +	      if (!ctx->quiet)
> +		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,
> +			  index);
> +	      *non_constant_p = true;
>  	    }
> +	  no_zero_init = true;
> +	}
>  
> -	  /* We fell off the end of the CONSTRUCTOR, so insert a new
> -	     entry at the end.  */
> -	insert:
> -	  {
> -	    constructor_elt ce = { index, NULL_TREE };
> +      vec_safe_push (ctors, *valp);
> +      vec_safe_push (indexes, index);
>  
> -	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
> -	    cep = CONSTRUCTOR_ELT (*valp, idx);
> +      constructor_elt *cep
> +	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
> +      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELT (*valp, 0));
> +
> +      if (code == UNION_TYPE)
> +	activated_union_member_p = true;
>  
> -	    if (code == UNION_TYPE)
> -	      /* Record that we've changed an active union member.  */
> -	      changed_active_union_member_p = true;
> -	  }
> -	found:;
> -	}
>        valp = &cep->value;
>      }
>  
> @@ -4800,9 +4851,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>  	  init = tinit;
>        init = cxx_eval_constant_expression (&new_ctx, init, false,
>  					   non_constant_p, overflow_p);
> -      if (ctors->is_empty())
> -	/* The hash table might have moved since the get earlier.  */
> -	valp = ctx->global->values.get (object);
> +      /* The hash table might have moved since the get earlier.  */
> +      valp = ctx->global->values.get (object);
> +      /* The initializer might have mutated the underlying CONSTRUCTORs, so we
> +	 must recompute VALP.  */
> +      for (unsigned i = 0; i < vec_safe_length (indexes); i++)
> +	{
> +	  constructor_elt *cep
> +	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
> +	  valp = &cep->value;
> +	}
>      }
>  
>    /* Don't share a CONSTRUCTOR that might be changed later.  */
> @@ -4847,7 +4905,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>    unsigned i;
>    bool c = TREE_CONSTANT (init);
>    bool s = TREE_SIDE_EFFECTS (init);
> -  if (!c || s || changed_active_union_member_p)
> +  if (!c || s || activated_union_member_p)
>      FOR_EACH_VEC_ELT (*ctors, i, elt)
>        {
>  	if (!c)
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
> new file mode 100644
> index 00000000000..0f91e93ca3f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
> @@ -0,0 +1,19 @@
> +// PR c++/94205
> +// { dg-do compile { target c++14 } }
> +
> +struct S
> +{
> +  struct A
> +  {
> +    S *p;
> +    constexpr A(S* p): p(p) {}
> +    constexpr operator int() { p->a = 5; return 6; }
> +  };
> +  int a = A(this);
> +};
> +
> +
> +constexpr S s = {};
> +
> +#define SA(X) static_assert((X),#X)
> +SA(s.a == 6);
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
> new file mode 100644
> index 00000000000..0ceef1bb29f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
> @@ -0,0 +1,21 @@
> +// PR c++/94219
> +// { dg-do compile { target c++14 } }
> +
> +struct A { long x; };
> +
> +struct U;
> +constexpr A foo(U *up);
> +
> +struct U {
> +  A a = foo(this); int y;
> +};
> +
> +constexpr A foo(U *up) {
> +  up->y = 11;
> +  return {42};
> +}
> +
> +extern constexpr U u = {};
> +
> +static_assert(u.y == 11, "");
> +static_assert(u.a.x == 42, "");
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
> new file mode 100644
> index 00000000000..59e7a10d6e8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
> @@ -0,0 +1,22 @@
> +// PR c++/94219
> +// { dg-do compile { target c++14 } }
> +
> +struct A { long x; };
> +
> +struct U;
> +constexpr A foo(U *up);
> +
> +struct U {
> +  U() = default;
> +  int y; A a = foo(this);
> +};
> +
> +constexpr A foo(U *up) {
> +  up->y = 11;
> +  return {42};
> +}
> +
> +extern constexpr U u = {};
> +
> +static_assert(u.y == 11, "");
> +static_assert(u.a.x == 42, "");
> diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
> new file mode 100644
> index 00000000000..c6d44d7fd0b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
> @@ -0,0 +1,11 @@
> +// PR c++/94205
> +// { dg-do compile { target c++17 } }
> +
> +struct S
> +{
> +  int a = [this] { this->a = 5; return 6; } ();
> +};
> +
> +constexpr S s = {};
> +
> +static_assert(s.a == 6);
> -- 
> 2.26.0.106.g9fadedd637
> 
> 

Re: [PATCH] c++: Fix constexpr evaluation of self-modifying CONSTRUCTORs [PR94219]

[Jason Merrill]

On 4/2/20 2:19 PM, Patrick Palka wrote:
> On Thu, 2 Apr 2020, Patrick Palka wrote:
> 
>> This PR reveals that cxx_eval_bare_aggregate and cxx_eval_store_expression do
>> not anticipate that a constructor element's initializer could mutate the
>> underlying CONSTRUCTOR.  Evaluation of the initializer could add new elements to
>> the underlying CONSTRUCTOR, thereby potentially invalidating any pointers to
>> or assumptions about the CONSTRUCTOR's elements, and so these routines should be
>> prepared for that.
>>
>> To fix this problem, this patch makes cxx_eval_bare_aggregate and
>> cxx_eval_store_expression recompute the pointer to the constructor_elt's through
>> which we're assigning, after it evaluates the initializer.  Care is taken to
>> to make the common case where the initializer does not modify the underlying
>> CONSTRUCTOR as fast as before.
> 
> Also, with this patch, I'm not totally sure but I think we might not
> need the special preeval handling in cxx_eval_store_expression anymore.
> I could try to remove it in a subsequent patch.

I think for C++17 order of evaluation it's better to keep preevaluating 
when we can.

>> gcc/cp/ChangeLog:
>>
>> 	PR c++/94205
>> 	PR c++/94219
>> 	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
>> 	support for VECTOR_TYPEs, and optimizations for the common case)
>> 	from ...
>> 	(cxx_eval_store_expression): ... here.  Rename local variable
>> 	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
>> 	the sequence of indexes into 'indexes' that yields the subobject we're
>> 	assigning to.  Record the integer offsets of the constructor indexes
>> 	we're assigning through into 'index_pos_hints'.  After evaluating the
>> 	initializer of the store expression, recompute 'valp' using 'indexes'
>> 	and 'index_pos_hints' as hints.
>> 	(cxx_eval_bare_aggregate): Tweak comments.  Use get_or_insert_ctor_field
>> 	to recompute the pointer to the constructor_elt we're assigning through
>> 	after evaluating each initializer.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR c++/94205
>> 	PR c++/94219
>> 	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
>> 	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
>> 	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
>> 	* g++.dg/cpp1z/lambda-this5.C: New test.
>> ---
>>   gcc/cp/constexpr.c                            | 252 +++++++++++-------
>>   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
>>   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
>>   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
>>   gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
>>   5 files changed, 228 insertions(+), 97 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>>
>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>> index 91f0c3ba269..b4173c595f0 100644
>> --- a/gcc/cp/constexpr.c
>> +++ b/gcc/cp/constexpr.c
>> @@ -3151,6 +3151,97 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert)
>>     return -1;
>>   }
>>   
>> +/* Return a pointer to the constructor_elt of CTOR which matches INDEX.  If no
>> +   matching constructor_elt exists, then add one to CTOR.
>> +
>> +   As an optimization, if POS_HINT is non-negative then it is used as a guess
>> +   for the (integer) index of the matching constructor_elt within CTOR.  */
>> +
>> +static constructor_elt *
>> +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
>> +{
>> +  tree type = TREE_TYPE (ctor);
>> +  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
>> +    {
>> +      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
>> +      return &CONSTRUCTOR_ELTS (ctor)->last();
>> +    }
>> +  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
>> +    {
>> +      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
>> +      gcc_assert (i >= 0);
>> +      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
>> +      gcc_assert (cep->index == NULL_TREE
>> +		  || TREE_CODE (cep->index) != RANGE_EXPR);
>> +      return cep;
>> +    }
>> +  else
>> +    {
>> +      gcc_assert (TREE_CODE (index) == FIELD_DECL);
>> +
>> +      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>> +	 Usually we meet initializers in that order, but it is
>> +	 possible for base types to be placed not in program
>> +	 order.  */
>> +      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>> +      unsigned HOST_WIDE_INT idx = 0;
>> +      constructor_elt *cep = NULL;
>> +
>> +      /* First, check if we're changing the active member of a union.  */
>> +      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
>> +	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
>> +	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
>> +      /* Next, check the hint.  */
>> +      else if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor)
>> +	       && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
>> +	{
>> +	  cep = CONSTRUCTOR_ELT (ctor, pos_hint);
>> +	  goto found;
>> +	}

Don't we want to use pos_hint for all aggregate types?

>> +      /* If the hint was wrong, and if the bit offset of INDEX is larger than
>> +	 that of the last constructor_elt, then we can just immediately append a
>> +	 new constructor_elt to the end of CTOR.  */
>> +      else if (CONSTRUCTOR_NELTS (ctor)
>> +	       && tree_int_cst_compare (bit_position (index),
>> +					bit_position (CONSTRUCTOR_ELTS (ctor)
>> +						      ->last().index)) > 0)
>> +	{
>> +	  idx = CONSTRUCTOR_NELTS (ctor);
>> +	  goto insert;
>> +	}
>> +
>> +      /* Otherwise, we need to iterator over CTOR to find or insert INDEX

s/iterator/iterate/

>> +	 appropriately.  */
>> +
>> +      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
>> +	   idx++, fields = DECL_CHAIN (fields))
>> +	{
>> +	  if (index == cep->index)
>> +	    goto found;
>> +
>> +	  /* The field we're initializing must be on the field
>> +	     list.  Look to see if it is present before the
>> +	     field the current ELT initializes.  */
>> +	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
>> +	    if (index == fields)
>> +	      goto insert;
>> +	}
>> +      /* We fell off the end of the CONSTRUCTOR, so insert a new
>> +	 entry at the end.  */
>> +
>> +    insert:
>> +      {
>> +	constructor_elt ce = { index, NULL_TREE };
>> +
>> +	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
>> +	cep = CONSTRUCTOR_ELT (ctor, idx);
>> +      }
>> +    found:;
>> +
>> +      return cep;
>> +    }
>> +}
>> +
>>   /* Under the control of CTX, issue a detailed diagnostic for
>>      an out-of-bounds subscript INDEX into the expression ARRAY.  */
>>   
>> @@ -3760,14 +3851,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>>       {
>>         tree orig_value = value;
>>         init_subob_ctx (ctx, new_ctx, index, value);
>> +      int pos_hint = -1;
>>         if (new_ctx.ctor != ctx->ctor)
>> -	/* If we built a new CONSTRUCTOR, attach it now so that other
>> -	   initializers can refer to it.  */
>> -	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>> +	{
>> +	  /* If we built a new CONSTRUCTOR, attach it now so that other
>> +	     initializers can refer to it.  */
>> +	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>> +	  pos_hint = vec_safe_length (*p) - 1;
>> +	}
>>         else if (TREE_CODE (type) == UNION_TYPE)
>> -	/* Otherwise if we're constructing a union, set the active union member
>> -	   anyway so that we can later detect if the initializer attempts to
>> -	   activate another member.  */
>> +	/* Otherwise if we're constructing a non-aggregate union member, set
>> +	   the active union member now so that we can later detect and diagnose
>> +	   if its initializer attempts to activate another member.  */
>>   	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
>>         tree elt = cxx_eval_constant_expression (&new_ctx, value,
>>   					       lval,
>> @@ -3804,18 +3899,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>>   	{
>>   	  if (TREE_CODE (type) == UNION_TYPE
>>   	      && (*p)->last().index != index)
>> -	    /* The initializer may have erroneously changed the active union
>> -	       member that we're initializing.  */
>> +	    /* The initializer erroneously changed the active union member that
>> +	       we're initializing.  */
>>   	    gcc_assert (*non_constant_p);
>> -	  else if (new_ctx.ctor != ctx->ctor
>> -		   || TREE_CODE (type) == UNION_TYPE)
>> +	  else
>>   	    {
>> -	      /* We appended this element above; update the value.  */
>> -	      gcc_assert ((*p)->last().index == index);
>> -	      (*p)->last().value = elt;
>> +	      /* The initializer might have mutated the underlying CONSTRUCTOR,
>> +		 so recompute the location of the target constructer_elt.  */
>> +	      constructor_elt *cep
>> +		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
>> +	      cep->value = elt;
>>   	    }
>> -	  else
>> -	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
>> +
>>   	  /* Adding or replacing an element might change the ctor's flags.  */
>>   	  TREE_CONSTANT (ctx->ctor) = constant_p;
>>   	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
>> @@ -4590,8 +4685,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>     type = TREE_TYPE (object);
>>     bool no_zero_init = true;
>>   
>> -  releasing_vec ctors;
>> -  bool changed_active_union_member_p = false;
>> +  releasing_vec ctors, indexes;
>> +  auto_vec<int> index_pos_hints;
>> +  bool activated_union_member_p = false;
>>     while (!refs->is_empty ())
>>       {
>>         if (*valp == NULL_TREE)
>> @@ -4632,94 +4728,49 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>   	 subobjects will also be zero-initialized.  */
>>         no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
>>   
>> -      vec_safe_push (ctors, *valp);
>> -
>>         enum tree_code code = TREE_CODE (type);
>>         type = refs->pop();
>>         tree index = refs->pop();
>>   
>> -      constructor_elt *cep = NULL;
>> -      if (code == ARRAY_TYPE)
>> +      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>> +	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>>   	{
>> -	  HOST_WIDE_INT i
>> -	    = find_array_ctor_elt (*valp, index, /*insert*/true);
>> -	  gcc_assert (i >= 0);
>> -	  cep = CONSTRUCTOR_ELT (*valp, i);
>> -	  gcc_assert (cep->index == NULL_TREE
>> -		      || TREE_CODE (cep->index) != RANGE_EXPR);
>> -	}
>> -      else
>> -	{
>> -	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
>> -
>> -	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>> -	     Usually we meet initializers in that order, but it is
>> -	     possible for base types to be placed not in program
>> -	     order.  */
>> -	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>> -	  unsigned HOST_WIDE_INT idx;
>> -
>> -	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>> -	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>> +	  if (cxx_dialect < cxx2a)
>>   	    {
>> -	      if (cxx_dialect < cxx2a)
>> -		{
>> -		  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,
>> -			      index);
>> -		  *non_constant_p = true;
>> -		}
>> -	      else if (TREE_CODE (t) == MODIFY_EXPR
>> -		       && CONSTRUCTOR_NO_CLEARING (*valp))
>> -		{
>> -		  /* Diagnose changing the active union member while the union
>> -		     is in the process of being initialized.  */
>> -		  if (!ctx->quiet)
>> -		    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,
>> -			      index);
>> -		  *non_constant_p = true;
>> -		}
>> -	      /* Changing active member.  */
>> -	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
>> -	      no_zero_init = true;
>> +	      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,
>> +			  index);
>> +	      *non_constant_p = true;
>>   	    }
>> -
>> -	  for (idx = 0;
>> -	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
>> -	       idx++, fields = DECL_CHAIN (fields))
>> +	  else if (TREE_CODE (t) == MODIFY_EXPR
>> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
>>   	    {
>> -	      if (index == cep->index)
>> -		goto found;
>> -
>> -	      /* The field we're initializing must be on the field
>> -		 list.  Look to see if it is present before the
>> -		 field the current ELT initializes.  */
>> -	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
>> -		if (index == fields)
>> -		  goto insert;
>> +	      /* Diagnose changing the active union member while the union
>> +		 is in the process of being initialized.  */
>> +	      if (!ctx->quiet)
>> +		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,
>> +			  index);
>> +	      *non_constant_p = true;
>>   	    }
>> +	  no_zero_init = true;
>> +	}
>>   
>> -	  /* We fell off the end of the CONSTRUCTOR, so insert a new
>> -	     entry at the end.  */
>> -	insert:
>> -	  {
>> -	    constructor_elt ce = { index, NULL_TREE };
>> +      vec_safe_push (ctors, *valp);
>> +      vec_safe_push (indexes, index);
>>   
>> -	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
>> -	    cep = CONSTRUCTOR_ELT (*valp, idx);
>> +      constructor_elt *cep
>> +	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
>> +      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELT (*valp, 0));

I wonder if you want to use valp->begin() rather than CONSTRUCTOR_ELT?

>> +
>> +      if (code == UNION_TYPE)
>> +	activated_union_member_p = true;
>>   
>> -	    if (code == UNION_TYPE)
>> -	      /* Record that we've changed an active union member.  */
>> -	      changed_active_union_member_p = true;
>> -	  }
>> -	found:;
>> -	}
>>         valp = &cep->value;
>>       }
>>   
>> @@ -4800,9 +4851,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>   	  init = tinit;
>>         init = cxx_eval_constant_expression (&new_ctx, init, false,
>>   					   non_constant_p, overflow_p);
>> -      if (ctors->is_empty())
>> -	/* The hash table might have moved since the get earlier.  */
>> -	valp = ctx->global->values.get (object);
>> +      /* The hash table might have moved since the get earlier.  */
>> +      valp = ctx->global->values.get (object);
>> +      /* The initializer might have mutated the underlying CONSTRUCTORs, so we
>> +	 must recompute VALP.  */
>> +      for (unsigned i = 0; i < vec_safe_length (indexes); i++)
>> +	{
>> +	  constructor_elt *cep
>> +	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
>> +	  valp = &cep->value;
>> +	}
>>       }
>>   
>>     /* Don't share a CONSTRUCTOR that might be changed later.  */
>> @@ -4847,7 +4905,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>     unsigned i;
>>     bool c = TREE_CONSTANT (init);
>>     bool s = TREE_SIDE_EFFECTS (init);
>> -  if (!c || s || changed_active_union_member_p)
>> +  if (!c || s || activated_union_member_p)
>>       FOR_EACH_VEC_ELT (*ctors, i, elt)
>>         {
>>   	if (!c)
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>> new file mode 100644
>> index 00000000000..0f91e93ca3f
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>> @@ -0,0 +1,19 @@
>> +// PR c++/94205
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct S
>> +{
>> +  struct A
>> +  {
>> +    S *p;
>> +    constexpr A(S* p): p(p) {}
>> +    constexpr operator int() { p->a = 5; return 6; }
>> +  };
>> +  int a = A(this);
>> +};
>> +
>> +
>> +constexpr S s = {};
>> +
>> +#define SA(X) static_assert((X),#X)
>> +SA(s.a == 6);
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>> new file mode 100644
>> index 00000000000..0ceef1bb29f
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>> @@ -0,0 +1,21 @@
>> +// PR c++/94219
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct A { long x; };
>> +
>> +struct U;
>> +constexpr A foo(U *up);
>> +
>> +struct U {
>> +  A a = foo(this); int y;
>> +};
>> +
>> +constexpr A foo(U *up) {
>> +  up->y = 11;
>> +  return {42};
>> +}
>> +
>> +extern constexpr U u = {};
>> +
>> +static_assert(u.y == 11, "");
>> +static_assert(u.a.x == 42, "");
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>> new file mode 100644
>> index 00000000000..59e7a10d6e8
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>> @@ -0,0 +1,22 @@
>> +// PR c++/94219
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct A { long x; };
>> +
>> +struct U;
>> +constexpr A foo(U *up);
>> +
>> +struct U {
>> +  U() = default;
>> +  int y; A a = foo(this);
>> +};
>> +
>> +constexpr A foo(U *up) {
>> +  up->y = 11;
>> +  return {42};
>> +}
>> +
>> +extern constexpr U u = {};
>> +
>> +static_assert(u.y == 11, "");
>> +static_assert(u.a.x == 42, "");
>> diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>> new file mode 100644
>> index 00000000000..c6d44d7fd0b
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>> @@ -0,0 +1,11 @@
>> +// PR c++/94205
>> +// { dg-do compile { target c++17 } }
>> +
>> +struct S
>> +{
>> +  int a = [this] { this->a = 5; return 6; } ();
>> +};
>> +
>> +constexpr S s = {};
>> +
>> +static_assert(s.a == 6);
>> -- 
>> 2.26.0.106.g9fadedd637
>>
>>
> 

Re: [PATCH] c++: Fix constexpr evaluation of self-modifying CONSTRUCTORs [PR94219]

[Patrick Palka]

On Fri, 3 Apr 2020, Jason Merrill wrote:

> On 4/2/20 2:19 PM, Patrick Palka wrote:
> > On Thu, 2 Apr 2020, Patrick Palka wrote:
> > 
> > > This PR reveals that cxx_eval_bare_aggregate and cxx_eval_store_expression
> > > do
> > > not anticipate that a constructor element's initializer could mutate the
> > > underlying CONSTRUCTOR.  Evaluation of the initializer could add new
> > > elements to
> > > the underlying CONSTRUCTOR, thereby potentially invalidating any pointers
> > > to
> > > or assumptions about the CONSTRUCTOR's elements, and so these routines
> > > should be
> > > prepared for that.
> > > 
> > > To fix this problem, this patch makes cxx_eval_bare_aggregate and
> > > cxx_eval_store_expression recompute the pointer to the constructor_elt's
> > > through
> > > which we're assigning, after it evaluates the initializer.  Care is taken
> > > to
> > > to make the common case where the initializer does not modify the
> > > underlying
> > > CONSTRUCTOR as fast as before.
> > 
> > Also, with this patch, I'm not totally sure but I think we might not
> > need the special preeval handling in cxx_eval_store_expression anymore.
> > I could try to remove it in a subsequent patch.
> 
> I think for C++17 order of evaluation it's better to keep preevaluating when
> we can.

Sounds good.

> 
> > > gcc/cp/ChangeLog:
> > > 
> > > 	PR c++/94205
> > > 	PR c++/94219
> > > 	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
> > > 	support for VECTOR_TYPEs, and optimizations for the common case)
> > > 	from ...
> > > 	(cxx_eval_store_expression): ... here.  Rename local variable
> > > 	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
> > > 	the sequence of indexes into 'indexes' that yields the subobject we're
> > > 	assigning to.  Record the integer offsets of the constructor indexes
> > > 	we're assigning through into 'index_pos_hints'.  After evaluating the
> > > 	initializer of the store expression, recompute 'valp' using 'indexes'
> > > 	and 'index_pos_hints' as hints.
> > > 	(cxx_eval_bare_aggregate): Tweak comments.  Use
> > > get_or_insert_ctor_field
> > > 	to recompute the pointer to the constructor_elt we're assigning
> > > through
> > > 	after evaluating each initializer.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	PR c++/94205
> > > 	PR c++/94219
> > > 	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
> > > 	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
> > > 	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
> > > 	* g++.dg/cpp1z/lambda-this5.C: New test.
> > > ---
> > >   gcc/cp/constexpr.c                            | 252 +++++++++++-------
> > >   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
> > >   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
> > >   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
> > >   gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
> > >   5 files changed, 228 insertions(+), 97 deletions(-)
> > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
> > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
> > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
> > >   create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
> > > 
> > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > > index 91f0c3ba269..b4173c595f0 100644
> > > --- a/gcc/cp/constexpr.c
> > > +++ b/gcc/cp/constexpr.c
> > > @@ -3151,6 +3151,97 @@ find_array_ctor_elt (tree ary, tree dindex, bool
> > > insert)
> > >     return -1;
> > >   }
> > >   +/* Return a pointer to the constructor_elt of CTOR which matches INDEX.
> > > If no
> > > +   matching constructor_elt exists, then add one to CTOR.
> > > +
> > > +   As an optimization, if POS_HINT is non-negative then it is used as a
> > > guess
> > > +   for the (integer) index of the matching constructor_elt within CTOR.
> > > */
> > > +
> > > +static constructor_elt *
> > > +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
> > > +{
> > > +  tree type = TREE_TYPE (ctor);
> > > +  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
> > > +    {
> > > +      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
> > > +      return &CONSTRUCTOR_ELTS (ctor)->last();
> > > +    }
> > > +  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) ==
> > > VECTOR_TYPE)
> > > +    {
> > > +      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index,
> > > /*insert*/true);
> > > +      gcc_assert (i >= 0);
> > > +      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
> > > +      gcc_assert (cep->index == NULL_TREE
> > > +		  || TREE_CODE (cep->index) != RANGE_EXPR);
> > > +      return cep;
> > > +    }
> > > +  else
> > > +    {
> > > +      gcc_assert (TREE_CODE (index) == FIELD_DECL);
> > > +
> > > +      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
> > > +	 Usually we meet initializers in that order, but it is
> > > +	 possible for base types to be placed not in program
> > > +	 order.  */
> > > +      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
> > > +      unsigned HOST_WIDE_INT idx = 0;
> > > +      constructor_elt *cep = NULL;
> > > +
> > > +      /* First, check if we're changing the active member of a union.  */
> > > +      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
> > > +	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
> > > +	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
> > > +      /* Next, check the hint.  */
> > > +      else if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS
> > > (ctor)
> > > +	       && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
> > > +	{
> > > +	  cep = CONSTRUCTOR_ELT (ctor, pos_hint);
> > > +	  goto found;
> > > +	}
> 
> Don't we want to use pos_hint for all aggregate types?

Oops, yes.  I moved the hint check up to happen first and
unconditionally.

> 
> > > +      /* If the hint was wrong, and if the bit offset of INDEX is larger
> > > than
> > > +	 that of the last constructor_elt, then we can just immediately append
> > > a
> > > +	 new constructor_elt to the end of CTOR.  */
> > > +      else if (CONSTRUCTOR_NELTS (ctor)
> > > +	       && tree_int_cst_compare (bit_position (index),
> > > +					bit_position (CONSTRUCTOR_ELTS (ctor)
> > > +						      ->last().index)) > 0)
> > > +	{
> > > +	  idx = CONSTRUCTOR_NELTS (ctor);
> > > +	  goto insert;
> > > +	}
> > > +
> > > +      /* Otherwise, we need to iterator over CTOR to find or insert INDEX
> 
> s/iterator/iterate/

Fixed.

> 
> > > +	 appropriately.  */
> > > +
> > > +      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
> > > +	   idx++, fields = DECL_CHAIN (fields))
> > > +	{
> > > +	  if (index == cep->index)
> > > +	    goto found;
> > > +
> > > +	  /* The field we're initializing must be on the field
> > > +	     list.  Look to see if it is present before the
> > > +	     field the current ELT initializes.  */
> > > +	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
> > > +	    if (index == fields)
> > > +	      goto insert;
> > > +	}
> > > +      /* We fell off the end of the CONSTRUCTOR, so insert a new
> > > +	 entry at the end.  */
> > > +
> > > +    insert:
> > > +      {
> > > +	constructor_elt ce = { index, NULL_TREE };
> > > +
> > > +	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
> > > +	cep = CONSTRUCTOR_ELT (ctor, idx);
> > > +      }
> > > +    found:;
> > > +
> > > +      return cep;
> > > +    }
> > > +}
> > > +
> > >   /* Under the control of CTX, issue a detailed diagnostic for
> > >      an out-of-bounds subscript INDEX into the expression ARRAY.  */
> > >   @@ -3760,14 +3851,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx
> > > *ctx, tree t,
> > >       {
> > >         tree orig_value = value;
> > >         init_subob_ctx (ctx, new_ctx, index, value);
> > > +      int pos_hint = -1;
> > >         if (new_ctx.ctor != ctx->ctor)
> > > -	/* If we built a new CONSTRUCTOR, attach it now so that other
> > > -	   initializers can refer to it.  */
> > > -	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> > > +	{
> > > +	  /* If we built a new CONSTRUCTOR, attach it now so that other
> > > +	     initializers can refer to it.  */
> > > +	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> > > +	  pos_hint = vec_safe_length (*p) - 1;
> > > +	}
> > >         else if (TREE_CODE (type) == UNION_TYPE)
> > > -	/* Otherwise if we're constructing a union, set the active union
> > > member
> > > -	   anyway so that we can later detect if the initializer attempts to
> > > -	   activate another member.  */
> > > +	/* Otherwise if we're constructing a non-aggregate union member, set
> > > +	   the active union member now so that we can later detect and
> > > diagnose
> > > +	   if its initializer attempts to activate another member.  */
> > >   	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
> > >         tree elt = cxx_eval_constant_expression (&new_ctx, value,
> > >   					       lval,
> > > @@ -3804,18 +3899,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx,
> > > tree t,
> > >   	{
> > >   	  if (TREE_CODE (type) == UNION_TYPE
> > >   	      && (*p)->last().index != index)
> > > -	    /* The initializer may have erroneously changed the active union
> > > -	       member that we're initializing.  */
> > > +	    /* The initializer erroneously changed the active union member
> > > that
> > > +	       we're initializing.  */
> > >   	    gcc_assert (*non_constant_p);
> > > -	  else if (new_ctx.ctor != ctx->ctor
> > > -		   || TREE_CODE (type) == UNION_TYPE)
> > > +	  else
> > >   	    {
> > > -	      /* We appended this element above; update the value.  */
> > > -	      gcc_assert ((*p)->last().index == index);
> > > -	      (*p)->last().value = elt;
> > > +	      /* The initializer might have mutated the underlying
> > > CONSTRUCTOR,
> > > +		 so recompute the location of the target constructer_elt.  */
> > > +	      constructor_elt *cep
> > > +		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
> > > +	      cep->value = elt;
> > >   	    }
> > > -	  else
> > > -	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
> > > +
> > >   	  /* Adding or replacing an element might change the ctor's flags.  */
> > >   	  TREE_CONSTANT (ctx->ctor) = constant_p;
> > >   	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
> > > @@ -4590,8 +4685,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
> > > tree t,
> > >     type = TREE_TYPE (object);
> > >     bool no_zero_init = true;
> > >   -  releasing_vec ctors;
> > > -  bool changed_active_union_member_p = false;
> > > +  releasing_vec ctors, indexes;
> > > +  auto_vec<int> index_pos_hints;
> > > +  bool activated_union_member_p = false;
> > >     while (!refs->is_empty ())
> > >       {
> > >         if (*valp == NULL_TREE)
> > > @@ -4632,94 +4728,49 @@ cxx_eval_store_expression (const constexpr_ctx
> > > *ctx, tree t,
> > >   	 subobjects will also be zero-initialized.  */
> > >         no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> > >   -      vec_safe_push (ctors, *valp);
> > > -
> > >         enum tree_code code = TREE_CODE (type);
> > >         type = refs->pop();
> > >         tree index = refs->pop();
> > >   -      constructor_elt *cep = NULL;
> > > -      if (code == ARRAY_TYPE)
> > > +      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> > > +	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> > >   	{
> > > -	  HOST_WIDE_INT i
> > > -	    = find_array_ctor_elt (*valp, index, /*insert*/true);
> > > -	  gcc_assert (i >= 0);
> > > -	  cep = CONSTRUCTOR_ELT (*valp, i);
> > > -	  gcc_assert (cep->index == NULL_TREE
> > > -		      || TREE_CODE (cep->index) != RANGE_EXPR);
> > > -	}
> > > -      else
> > > -	{
> > > -	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
> > > -
> > > -	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
> > > -	     Usually we meet initializers in that order, but it is
> > > -	     possible for base types to be placed not in program
> > > -	     order.  */
> > > -	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
> > > -	  unsigned HOST_WIDE_INT idx;
> > > -
> > > -	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> > > -	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> > > +	  if (cxx_dialect < cxx2a)
> > >   	    {
> > > -	      if (cxx_dialect < cxx2a)
> > > -		{
> > > -		  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,
> > > -			      index);
> > > -		  *non_constant_p = true;
> > > -		}
> > > -	      else if (TREE_CODE (t) == MODIFY_EXPR
> > > -		       && CONSTRUCTOR_NO_CLEARING (*valp))
> > > -		{
> > > -		  /* Diagnose changing the active union member while the union
> > > -		     is in the process of being initialized.  */
> > > -		  if (!ctx->quiet)
> > > -		    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,
> > > -			      index);
> > > -		  *non_constant_p = true;
> > > -		}
> > > -	      /* Changing active member.  */
> > > -	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
> > > -	      no_zero_init = true;
> > > +	      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,
> > > +			  index);
> > > +	      *non_constant_p = true;
> > >   	    }
> > > -
> > > -	  for (idx = 0;
> > > -	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
> > > -	       idx++, fields = DECL_CHAIN (fields))
> > > +	  else if (TREE_CODE (t) == MODIFY_EXPR
> > > +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> > >   	    {
> > > -	      if (index == cep->index)
> > > -		goto found;
> > > -
> > > -	      /* The field we're initializing must be on the field
> > > -		 list.  Look to see if it is present before the
> > > -		 field the current ELT initializes.  */
> > > -	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
> > > -		if (index == fields)
> > > -		  goto insert;
> > > +	      /* Diagnose changing the active union member while the union
> > > +		 is in the process of being initialized.  */
> > > +	      if (!ctx->quiet)
> > > +		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,
> > > +			  index);
> > > +	      *non_constant_p = true;
> > >   	    }
> > > +	  no_zero_init = true;
> > > +	}
> > >   -	  /* We fell off the end of the CONSTRUCTOR, so insert a new
> > > -	     entry at the end.  */
> > > -	insert:
> > > -	  {
> > > -	    constructor_elt ce = { index, NULL_TREE };
> > > +      vec_safe_push (ctors, *valp);
> > > +      vec_safe_push (indexes, index);
> > >   -	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
> > > -	    cep = CONSTRUCTOR_ELT (*valp, idx);
> > > +      constructor_elt *cep
> > > +	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
> > > +      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELT (*valp, 0));
> 
> I wonder if you want to use valp->begin() rather than CONSTRUCTOR_ELT?

Fixed to do CONSTRUCTOR_ELTS (*valp)->begin().

Here's the updated patch with the above changes incorporated:

-- >8 --

gcc/cp/ChangeLog:

	PR c++/94219
	PR c++/94205
	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
	support for VECTOR_TYPEs, and optimizations for the common case)
	from ...
	(cxx_eval_store_expression): ... here.  Rename local variable
	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
	the sequence of indexes into 'indexes' that yields the subobject we're
	assigning to.  Record the integer offsets of the constructor indexes
	we're assigning through into 'index_pos_hints'.  After evaluating the
	initializer of the store expression, recompute 'valp' using 'indexes'
	and using 'index_pos_hints' as hints.
	(cxx_eval_bare_aggregate): Tweak comments.  Use get_or_insert_ctor_field
	to recompute the pointer to the constructor_elt we're assigning through
	after evaluating each initializer.

gcc/testsuite/ChangeLog:

	PR c++/94219
	PR c++/94205
	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
	* g++.dg/cpp1z/lambda-this5.C: New test.
---
 gcc/cp/constexpr.c                            | 250 +++++++++++-------
 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
 5 files changed, 226 insertions(+), 97 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 91f0c3ba269..3c5137bc3b0 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3151,6 +3151,95 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert)
   return -1;
 }
 
+/* Return a pointer to the constructor_elt of CTOR which matches INDEX.  If no
+   matching constructor_elt exists, then add one to CTOR.
+
+   As an optimization, if POS_HINT is non-negative then it is used as a guess
+   for the (integer) index of the matching constructor_elt within CTOR.  */
+
+static constructor_elt *
+get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
+{
+  /* Check the hint first.  */
+  if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor)
+      && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
+    return CONSTRUCTOR_ELT (ctor, pos_hint);
+
+  tree type = TREE_TYPE (ctor);
+  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
+    {
+      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
+      return &CONSTRUCTOR_ELTS (ctor)->last();
+    }
+  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
+    {
+      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
+      gcc_assert (i >= 0);
+      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
+      gcc_assert (cep->index == NULL_TREE
+		  || TREE_CODE (cep->index) != RANGE_EXPR);
+      return cep;
+    }
+  else
+    {
+      gcc_assert (TREE_CODE (index) == FIELD_DECL);
+
+      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
+	 Usually we meet initializers in that order, but it is
+	 possible for base types to be placed not in program
+	 order.  */
+      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
+      unsigned HOST_WIDE_INT idx = 0;
+      constructor_elt *cep = NULL;
+
+      /* Check if we're changing the active member of a union.  */
+      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
+	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
+	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
+      /* If the bit offset of INDEX is larger than that of the last
+	 constructor_elt, then we can just immediately append a new
+	 constructor_elt to the end of CTOR.  */
+      else if (CONSTRUCTOR_NELTS (ctor)
+	       && tree_int_cst_compare (bit_position (index),
+					bit_position (CONSTRUCTOR_ELTS (ctor)
+						      ->last().index)) > 0)
+	{
+	  idx = CONSTRUCTOR_NELTS (ctor);
+	  goto insert;
+	}
+
+      /* Otherwise, we need to iterate over CTOR to find or insert INDEX
+	 appropriately.  */
+
+      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
+	   idx++, fields = DECL_CHAIN (fields))
+	{
+	  if (index == cep->index)
+	    goto found;
+
+	  /* The field we're initializing must be on the field
+	     list.  Look to see if it is present before the
+	     field the current ELT initializes.  */
+	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
+	    if (index == fields)
+	      goto insert;
+	}
+      /* We fell off the end of the CONSTRUCTOR, so insert a new
+	 entry at the end.  */
+
+    insert:
+      {
+	constructor_elt ce = { index, NULL_TREE };
+
+	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
+	cep = CONSTRUCTOR_ELT (ctor, idx);
+      }
+    found:;
+
+      return cep;
+    }
+}
+
 /* Under the control of CTX, issue a detailed diagnostic for
    an out-of-bounds subscript INDEX into the expression ARRAY.  */
 
@@ -3760,14 +3849,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
     {
       tree orig_value = value;
       init_subob_ctx (ctx, new_ctx, index, value);
+      int pos_hint = -1;
       if (new_ctx.ctor != ctx->ctor)
-	/* If we built a new CONSTRUCTOR, attach it now so that other
-	   initializers can refer to it.  */
-	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
+	{
+	  /* If we built a new CONSTRUCTOR, attach it now so that other
+	     initializers can refer to it.  */
+	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
+	  pos_hint = vec_safe_length (*p) - 1;
+	}
       else if (TREE_CODE (type) == UNION_TYPE)
-	/* Otherwise if we're constructing a union, set the active union member
-	   anyway so that we can later detect if the initializer attempts to
-	   activate another member.  */
+	/* Otherwise if we're constructing a non-aggregate union member, set
+	   the active union member now so that we can later detect and diagnose
+	   if its initializer attempts to activate another member.  */
 	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
       tree elt = cxx_eval_constant_expression (&new_ctx, value,
 					       lval,
@@ -3804,18 +3897,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
 	{
 	  if (TREE_CODE (type) == UNION_TYPE
 	      && (*p)->last().index != index)
-	    /* The initializer may have erroneously changed the active union
-	       member that we're initializing.  */
+	    /* The initializer erroneously changed the active union member that
+	       we're initializing.  */
 	    gcc_assert (*non_constant_p);
-	  else if (new_ctx.ctor != ctx->ctor
-		   || TREE_CODE (type) == UNION_TYPE)
+	  else
 	    {
-	      /* We appended this element above; update the value.  */
-	      gcc_assert ((*p)->last().index == index);
-	      (*p)->last().value = elt;
+	      /* The initializer might have mutated the underlying CONSTRUCTOR,
+		 so recompute the location of the target constructer_elt.  */
+	      constructor_elt *cep
+		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
+	      cep->value = elt;
 	    }
-	  else
-	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
+
 	  /* Adding or replacing an element might change the ctor's flags.  */
 	  TREE_CONSTANT (ctx->ctor) = constant_p;
 	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
@@ -4590,8 +4683,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
   type = TREE_TYPE (object);
   bool no_zero_init = true;
 
-  releasing_vec ctors;
-  bool changed_active_union_member_p = false;
+  releasing_vec ctors, indexes;
+  auto_vec<int> index_pos_hints;
+  bool activated_union_member_p = false;
   while (!refs->is_empty ())
     {
       if (*valp == NULL_TREE)
@@ -4632,94 +4726,49 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 	 subobjects will also be zero-initialized.  */
       no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
 
-      vec_safe_push (ctors, *valp);
-
       enum tree_code code = TREE_CODE (type);
       type = refs->pop();
       tree index = refs->pop();
 
-      constructor_elt *cep = NULL;
-      if (code == ARRAY_TYPE)
+      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
+	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
 	{
-	  HOST_WIDE_INT i
-	    = find_array_ctor_elt (*valp, index, /*insert*/true);
-	  gcc_assert (i >= 0);
-	  cep = CONSTRUCTOR_ELT (*valp, i);
-	  gcc_assert (cep->index == NULL_TREE
-		      || TREE_CODE (cep->index) != RANGE_EXPR);
-	}
-      else
-	{
-	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
-
-	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
-	     Usually we meet initializers in that order, but it is
-	     possible for base types to be placed not in program
-	     order.  */
-	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
-	  unsigned HOST_WIDE_INT idx;
-
-	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
-	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
+	  if (cxx_dialect < cxx2a)
 	    {
-	      if (cxx_dialect < cxx2a)
-		{
-		  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,
-			      index);
-		  *non_constant_p = true;
-		}
-	      else if (TREE_CODE (t) == MODIFY_EXPR
-		       && CONSTRUCTOR_NO_CLEARING (*valp))
-		{
-		  /* Diagnose changing the active union member while the union
-		     is in the process of being initialized.  */
-		  if (!ctx->quiet)
-		    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,
-			      index);
-		  *non_constant_p = true;
-		}
-	      /* Changing active member.  */
-	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
-	      no_zero_init = true;
+	      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,
+			  index);
+	      *non_constant_p = true;
 	    }
-
-	  for (idx = 0;
-	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
-	       idx++, fields = DECL_CHAIN (fields))
+	  else if (TREE_CODE (t) == MODIFY_EXPR
+		   && CONSTRUCTOR_NO_CLEARING (*valp))
 	    {
-	      if (index == cep->index)
-		goto found;
-
-	      /* The field we're initializing must be on the field
-		 list.  Look to see if it is present before the
-		 field the current ELT initializes.  */
-	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
-		if (index == fields)
-		  goto insert;
+	      /* Diagnose changing the active union member while the union
+		 is in the process of being initialized.  */
+	      if (!ctx->quiet)
+		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,
+			  index);
+	      *non_constant_p = true;
 	    }
+	  no_zero_init = true;
+	}
 
-	  /* We fell off the end of the CONSTRUCTOR, so insert a new
-	     entry at the end.  */
-	insert:
-	  {
-	    constructor_elt ce = { index, NULL_TREE };
+      vec_safe_push (ctors, *valp);
+      vec_safe_push (indexes, index);
 
-	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
-	    cep = CONSTRUCTOR_ELT (*valp, idx);
+      constructor_elt *cep
+	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
+      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELTS (*valp)->begin());
+
+      if (code == UNION_TYPE)
+	activated_union_member_p = true;
 
-	    if (code == UNION_TYPE)
-	      /* Record that we've changed an active union member.  */
-	      changed_active_union_member_p = true;
-	  }
-	found:;
-	}
       valp = &cep->value;
     }
 
@@ -4800,9 +4849,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 	  init = tinit;
       init = cxx_eval_constant_expression (&new_ctx, init, false,
 					   non_constant_p, overflow_p);
-      if (ctors->is_empty())
-	/* The hash table might have moved since the get earlier.  */
-	valp = ctx->global->values.get (object);
+      /* The hash table might have moved since the get earlier, and the
+	 initializer might have mutated the underlying CONSTRUCTORs, so we must
+	 recompute VALP. */
+      valp = ctx->global->values.get (object);
+      for (unsigned i = 0; i < vec_safe_length (indexes); i++)
+	{
+	  constructor_elt *cep
+	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
+	  valp = &cep->value;
+	}
     }
 
   /* Don't share a CONSTRUCTOR that might be changed later.  */
@@ -4847,7 +4903,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
   unsigned i;
   bool c = TREE_CONSTANT (init);
   bool s = TREE_SIDE_EFFECTS (init);
-  if (!c || s || changed_active_union_member_p)
+  if (!c || s || activated_union_member_p)
     FOR_EACH_VEC_ELT (*ctors, i, elt)
       {
 	if (!c)
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
new file mode 100644
index 00000000000..0f91e93ca3f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
@@ -0,0 +1,19 @@
+// PR c++/94205
+// { dg-do compile { target c++14 } }
+
+struct S
+{
+  struct A
+  {
+    S *p;
+    constexpr A(S* p): p(p) {}
+    constexpr operator int() { p->a = 5; return 6; }
+  };
+  int a = A(this);
+};
+
+
+constexpr S s = {};
+
+#define SA(X) static_assert((X),#X)
+SA(s.a == 6);
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
new file mode 100644
index 00000000000..0ceef1bb29f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
@@ -0,0 +1,21 @@
+// PR c++/94219
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+struct U;
+constexpr A foo(U *up);
+
+struct U {
+  A a = foo(this); int y;
+};
+
+constexpr A foo(U *up) {
+  up->y = 11;
+  return {42};
+}
+
+extern constexpr U u = {};
+
+static_assert(u.y == 11, "");
+static_assert(u.a.x == 42, "");
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
new file mode 100644
index 00000000000..59e7a10d6e8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
@@ -0,0 +1,22 @@
+// PR c++/94219
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+struct U;
+constexpr A foo(U *up);
+
+struct U {
+  U() = default;
+  int y; A a = foo(this);
+};
+
+constexpr A foo(U *up) {
+  up->y = 11;
+  return {42};
+}
+
+extern constexpr U u = {};
+
+static_assert(u.y == 11, "");
+static_assert(u.a.x == 42, "");
diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
new file mode 100644
index 00000000000..c6d44d7fd0b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
@@ -0,0 +1,11 @@
+// PR c++/94205
+// { dg-do compile { target c++17 } }
+
+struct S
+{
+  int a = [this] { this->a = 5; return 6; } ();
+};
+
+constexpr S s = {};
+
+static_assert(s.a == 6);
-- 
2.26.0.106.g9fadedd637

Re: [PATCH] c++: Fix constexpr evaluation of self-modifying CONSTRUCTORs [PR94219]

[Patrick Palka]

On Fri, 3 Apr 2020, Patrick Palka wrote:

> On Fri, 3 Apr 2020, Jason Merrill wrote:
> 
> > On 4/2/20 2:19 PM, Patrick Palka wrote:
> > > On Thu, 2 Apr 2020, Patrick Palka wrote:
> > > 
> > > > This PR reveals that cxx_eval_bare_aggregate and cxx_eval_store_expression
> > > > do
> > > > not anticipate that a constructor element's initializer could mutate the
> > > > underlying CONSTRUCTOR.  Evaluation of the initializer could add new
> > > > elements to
> > > > the underlying CONSTRUCTOR, thereby potentially invalidating any pointers
> > > > to
> > > > or assumptions about the CONSTRUCTOR's elements, and so these routines
> > > > should be
> > > > prepared for that.
> > > > 
> > > > To fix this problem, this patch makes cxx_eval_bare_aggregate and
> > > > cxx_eval_store_expression recompute the pointer to the constructor_elt's
> > > > through
> > > > which we're assigning, after it evaluates the initializer.  Care is taken
> > > > to
> > > > to make the common case where the initializer does not modify the
> > > > underlying
> > > > CONSTRUCTOR as fast as before.
> > > 
> > > Also, with this patch, I'm not totally sure but I think we might not
> > > need the special preeval handling in cxx_eval_store_expression anymore.
> > > I could try to remove it in a subsequent patch.
> > 
> > I think for C++17 order of evaluation it's better to keep preevaluating when
> > we can.
> 
> Sounds good.
> 
> > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > 	PR c++/94205
> > > > 	PR c++/94219
> > > > 	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
> > > > 	support for VECTOR_TYPEs, and optimizations for the common case)
> > > > 	from ...
> > > > 	(cxx_eval_store_expression): ... here.  Rename local variable
> > > > 	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
> > > > 	the sequence of indexes into 'indexes' that yields the subobject we're
> > > > 	assigning to.  Record the integer offsets of the constructor indexes
> > > > 	we're assigning through into 'index_pos_hints'.  After evaluating the
> > > > 	initializer of the store expression, recompute 'valp' using 'indexes'
> > > > 	and 'index_pos_hints' as hints.
> > > > 	(cxx_eval_bare_aggregate): Tweak comments.  Use
> > > > get_or_insert_ctor_field
> > > > 	to recompute the pointer to the constructor_elt we're assigning
> > > > through
> > > > 	after evaluating each initializer.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 	PR c++/94205
> > > > 	PR c++/94219
> > > > 	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
> > > > 	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
> > > > 	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
> > > > 	* g++.dg/cpp1z/lambda-this5.C: New test.
> > > > ---
> > > >   gcc/cp/constexpr.c                            | 252 +++++++++++-------
> > > >   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
> > > >   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
> > > >   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
> > > >   gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
> > > >   5 files changed, 228 insertions(+), 97 deletions(-)
> > > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
> > > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
> > > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
> > > >   create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
> > > > 
> > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > > > index 91f0c3ba269..b4173c595f0 100644
> > > > --- a/gcc/cp/constexpr.c
> > > > +++ b/gcc/cp/constexpr.c
> > > > @@ -3151,6 +3151,97 @@ find_array_ctor_elt (tree ary, tree dindex, bool
> > > > insert)
> > > >     return -1;
> > > >   }
> > > >   +/* Return a pointer to the constructor_elt of CTOR which matches INDEX.
> > > > If no
> > > > +   matching constructor_elt exists, then add one to CTOR.
> > > > +
> > > > +   As an optimization, if POS_HINT is non-negative then it is used as a
> > > > guess
> > > > +   for the (integer) index of the matching constructor_elt within CTOR.
> > > > */
> > > > +
> > > > +static constructor_elt *
> > > > +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
> > > > +{
> > > > +  tree type = TREE_TYPE (ctor);
> > > > +  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
> > > > +    {
> > > > +      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
> > > > +      return &CONSTRUCTOR_ELTS (ctor)->last();
> > > > +    }
> > > > +  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) ==
> > > > VECTOR_TYPE)
> > > > +    {
> > > > +      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index,
> > > > /*insert*/true);
> > > > +      gcc_assert (i >= 0);
> > > > +      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
> > > > +      gcc_assert (cep->index == NULL_TREE
> > > > +		  || TREE_CODE (cep->index) != RANGE_EXPR);
> > > > +      return cep;
> > > > +    }
> > > > +  else
> > > > +    {
> > > > +      gcc_assert (TREE_CODE (index) == FIELD_DECL);
> > > > +
> > > > +      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
> > > > +	 Usually we meet initializers in that order, but it is
> > > > +	 possible for base types to be placed not in program
> > > > +	 order.  */
> > > > +      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
> > > > +      unsigned HOST_WIDE_INT idx = 0;
> > > > +      constructor_elt *cep = NULL;
> > > > +
> > > > +      /* First, check if we're changing the active member of a union.  */
> > > > +      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
> > > > +	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
> > > > +	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
> > > > +      /* Next, check the hint.  */
> > > > +      else if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS
> > > > (ctor)
> > > > +	       && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
> > > > +	{
> > > > +	  cep = CONSTRUCTOR_ELT (ctor, pos_hint);
> > > > +	  goto found;
> > > > +	}
> > 
> > Don't we want to use pos_hint for all aggregate types?
> 
> Oops, yes.  I moved the hint check up to happen first and
> unconditionally.
> 
> > 
> > > > +      /* If the hint was wrong, and if the bit offset of INDEX is larger
> > > > than
> > > > +	 that of the last constructor_elt, then we can just immediately append
> > > > a
> > > > +	 new constructor_elt to the end of CTOR.  */
> > > > +      else if (CONSTRUCTOR_NELTS (ctor)
> > > > +	       && tree_int_cst_compare (bit_position (index),
> > > > +					bit_position (CONSTRUCTOR_ELTS (ctor)
> > > > +						      ->last().index)) > 0)
> > > > +	{
> > > > +	  idx = CONSTRUCTOR_NELTS (ctor);
> > > > +	  goto insert;
> > > > +	}
> > > > +
> > > > +      /* Otherwise, we need to iterator over CTOR to find or insert INDEX
> > 
> > s/iterator/iterate/
> 
> Fixed.
> 
> > 
> > > > +	 appropriately.  */
> > > > +
> > > > +      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
> > > > +	   idx++, fields = DECL_CHAIN (fields))
> > > > +	{
> > > > +	  if (index == cep->index)
> > > > +	    goto found;
> > > > +
> > > > +	  /* The field we're initializing must be on the field
> > > > +	     list.  Look to see if it is present before the
> > > > +	     field the current ELT initializes.  */
> > > > +	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
> > > > +	    if (index == fields)
> > > > +	      goto insert;
> > > > +	}
> > > > +      /* We fell off the end of the CONSTRUCTOR, so insert a new
> > > > +	 entry at the end.  */
> > > > +
> > > > +    insert:
> > > > +      {
> > > > +	constructor_elt ce = { index, NULL_TREE };
> > > > +
> > > > +	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
> > > > +	cep = CONSTRUCTOR_ELT (ctor, idx);
> > > > +      }
> > > > +    found:;
> > > > +
> > > > +      return cep;
> > > > +    }
> > > > +}
> > > > +
> > > >   /* Under the control of CTX, issue a detailed diagnostic for
> > > >      an out-of-bounds subscript INDEX into the expression ARRAY.  */
> > > >   @@ -3760,14 +3851,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx
> > > > *ctx, tree t,
> > > >       {
> > > >         tree orig_value = value;
> > > >         init_subob_ctx (ctx, new_ctx, index, value);
> > > > +      int pos_hint = -1;
> > > >         if (new_ctx.ctor != ctx->ctor)
> > > > -	/* If we built a new CONSTRUCTOR, attach it now so that other
> > > > -	   initializers can refer to it.  */
> > > > -	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> > > > +	{
> > > > +	  /* If we built a new CONSTRUCTOR, attach it now so that other
> > > > +	     initializers can refer to it.  */
> > > > +	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> > > > +	  pos_hint = vec_safe_length (*p) - 1;
> > > > +	}
> > > >         else if (TREE_CODE (type) == UNION_TYPE)
> > > > -	/* Otherwise if we're constructing a union, set the active union
> > > > member
> > > > -	   anyway so that we can later detect if the initializer attempts to
> > > > -	   activate another member.  */
> > > > +	/* Otherwise if we're constructing a non-aggregate union member, set
> > > > +	   the active union member now so that we can later detect and
> > > > diagnose
> > > > +	   if its initializer attempts to activate another member.  */
> > > >   	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
> > > >         tree elt = cxx_eval_constant_expression (&new_ctx, value,
> > > >   					       lval,
> > > > @@ -3804,18 +3899,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx,
> > > > tree t,
> > > >   	{
> > > >   	  if (TREE_CODE (type) == UNION_TYPE
> > > >   	      && (*p)->last().index != index)
> > > > -	    /* The initializer may have erroneously changed the active union
> > > > -	       member that we're initializing.  */
> > > > +	    /* The initializer erroneously changed the active union member
> > > > that
> > > > +	       we're initializing.  */
> > > >   	    gcc_assert (*non_constant_p);
> > > > -	  else if (new_ctx.ctor != ctx->ctor
> > > > -		   || TREE_CODE (type) == UNION_TYPE)
> > > > +	  else
> > > >   	    {
> > > > -	      /* We appended this element above; update the value.  */
> > > > -	      gcc_assert ((*p)->last().index == index);
> > > > -	      (*p)->last().value = elt;
> > > > +	      /* The initializer might have mutated the underlying
> > > > CONSTRUCTOR,
> > > > +		 so recompute the location of the target constructer_elt.  */
> > > > +	      constructor_elt *cep
> > > > +		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
> > > > +	      cep->value = elt;
> > > >   	    }
> > > > -	  else
> > > > -	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
> > > > +
> > > >   	  /* Adding or replacing an element might change the ctor's flags.  */
> > > >   	  TREE_CONSTANT (ctx->ctor) = constant_p;
> > > >   	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
> > > > @@ -4590,8 +4685,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
> > > > tree t,
> > > >     type = TREE_TYPE (object);
> > > >     bool no_zero_init = true;
> > > >   -  releasing_vec ctors;
> > > > -  bool changed_active_union_member_p = false;
> > > > +  releasing_vec ctors, indexes;
> > > > +  auto_vec<int> index_pos_hints;
> > > > +  bool activated_union_member_p = false;
> > > >     while (!refs->is_empty ())
> > > >       {
> > > >         if (*valp == NULL_TREE)
> > > > @@ -4632,94 +4728,49 @@ cxx_eval_store_expression (const constexpr_ctx
> > > > *ctx, tree t,
> > > >   	 subobjects will also be zero-initialized.  */
> > > >         no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> > > >   -      vec_safe_push (ctors, *valp);
> > > > -
> > > >         enum tree_code code = TREE_CODE (type);
> > > >         type = refs->pop();
> > > >         tree index = refs->pop();
> > > >   -      constructor_elt *cep = NULL;
> > > > -      if (code == ARRAY_TYPE)
> > > > +      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> > > > +	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> > > >   	{
> > > > -	  HOST_WIDE_INT i
> > > > -	    = find_array_ctor_elt (*valp, index, /*insert*/true);
> > > > -	  gcc_assert (i >= 0);
> > > > -	  cep = CONSTRUCTOR_ELT (*valp, i);
> > > > -	  gcc_assert (cep->index == NULL_TREE
> > > > -		      || TREE_CODE (cep->index) != RANGE_EXPR);
> > > > -	}
> > > > -      else
> > > > -	{
> > > > -	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
> > > > -
> > > > -	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
> > > > -	     Usually we meet initializers in that order, but it is
> > > > -	     possible for base types to be placed not in program
> > > > -	     order.  */
> > > > -	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
> > > > -	  unsigned HOST_WIDE_INT idx;
> > > > -
> > > > -	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> > > > -	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> > > > +	  if (cxx_dialect < cxx2a)
> > > >   	    {
> > > > -	      if (cxx_dialect < cxx2a)
> > > > -		{
> > > > -		  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,
> > > > -			      index);
> > > > -		  *non_constant_p = true;
> > > > -		}
> > > > -	      else if (TREE_CODE (t) == MODIFY_EXPR
> > > > -		       && CONSTRUCTOR_NO_CLEARING (*valp))
> > > > -		{
> > > > -		  /* Diagnose changing the active union member while the union
> > > > -		     is in the process of being initialized.  */
> > > > -		  if (!ctx->quiet)
> > > > -		    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,
> > > > -			      index);
> > > > -		  *non_constant_p = true;
> > > > -		}
> > > > -	      /* Changing active member.  */
> > > > -	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
> > > > -	      no_zero_init = true;
> > > > +	      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,
> > > > +			  index);
> > > > +	      *non_constant_p = true;
> > > >   	    }
> > > > -
> > > > -	  for (idx = 0;
> > > > -	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
> > > > -	       idx++, fields = DECL_CHAIN (fields))
> > > > +	  else if (TREE_CODE (t) == MODIFY_EXPR
> > > > +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> > > >   	    {
> > > > -	      if (index == cep->index)
> > > > -		goto found;
> > > > -
> > > > -	      /* The field we're initializing must be on the field
> > > > -		 list.  Look to see if it is present before the
> > > > -		 field the current ELT initializes.  */
> > > > -	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
> > > > -		if (index == fields)
> > > > -		  goto insert;
> > > > +	      /* Diagnose changing the active union member while the union
> > > > +		 is in the process of being initialized.  */
> > > > +	      if (!ctx->quiet)
> > > > +		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,
> > > > +			  index);
> > > > +	      *non_constant_p = true;
> > > >   	    }
> > > > +	  no_zero_init = true;
> > > > +	}
> > > >   -	  /* We fell off the end of the CONSTRUCTOR, so insert a new
> > > > -	     entry at the end.  */
> > > > -	insert:
> > > > -	  {
> > > > -	    constructor_elt ce = { index, NULL_TREE };
> > > > +      vec_safe_push (ctors, *valp);
> > > > +      vec_safe_push (indexes, index);
> > > >   -	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
> > > > -	    cep = CONSTRUCTOR_ELT (*valp, idx);
> > > > +      constructor_elt *cep
> > > > +	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
> > > > +      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELT (*valp, 0));
> > 
> > I wonder if you want to use valp->begin() rather than CONSTRUCTOR_ELT?
> 
> Fixed to do CONSTRUCTOR_ELTS (*valp)->begin().

Hmm, but I don't think this is necessary, because after the call to
get_or_insert_ctor_field, *valp will surely have at least one
constructor_elt.

> 
> Here's the updated patch with the above changes incorporated:
> 
> -- >8 --
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/94219
> 	PR c++/94205
> 	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
> 	support for VECTOR_TYPEs, and optimizations for the common case)
> 	from ...
> 	(cxx_eval_store_expression): ... here.  Rename local variable
> 	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
> 	the sequence of indexes into 'indexes' that yields the subobject we're
> 	assigning to.  Record the integer offsets of the constructor indexes
> 	we're assigning through into 'index_pos_hints'.  After evaluating the
> 	initializer of the store expression, recompute 'valp' using 'indexes'
> 	and using 'index_pos_hints' as hints.
> 	(cxx_eval_bare_aggregate): Tweak comments.  Use get_or_insert_ctor_field
> 	to recompute the pointer to the constructor_elt we're assigning through
> 	after evaluating each initializer.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/94219
> 	PR c++/94205
> 	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
> 	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
> 	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
> 	* g++.dg/cpp1z/lambda-this5.C: New test.
> ---
>  gcc/cp/constexpr.c                            | 250 +++++++++++-------
>  gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
>  gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
>  gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
>  gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
>  5 files changed, 226 insertions(+), 97 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 91f0c3ba269..3c5137bc3b0 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -3151,6 +3151,95 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert)
>    return -1;
>  }
>  
> +/* Return a pointer to the constructor_elt of CTOR which matches INDEX.  If no
> +   matching constructor_elt exists, then add one to CTOR.
> +
> +   As an optimization, if POS_HINT is non-negative then it is used as a guess
> +   for the (integer) index of the matching constructor_elt within CTOR.  */
> +
> +static constructor_elt *
> +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
> +{
> +  /* Check the hint first.  */
> +  if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor)
> +      && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
> +    return CONSTRUCTOR_ELT (ctor, pos_hint);
> +
> +  tree type = TREE_TYPE (ctor);
> +  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
> +    {
> +      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
> +      return &CONSTRUCTOR_ELTS (ctor)->last();
> +    }
> +  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
> +    {
> +      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
> +      gcc_assert (i >= 0);
> +      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
> +      gcc_assert (cep->index == NULL_TREE
> +		  || TREE_CODE (cep->index) != RANGE_EXPR);
> +      return cep;
> +    }
> +  else
> +    {
> +      gcc_assert (TREE_CODE (index) == FIELD_DECL);
> +
> +      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
> +	 Usually we meet initializers in that order, but it is
> +	 possible for base types to be placed not in program
> +	 order.  */
> +      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
> +      unsigned HOST_WIDE_INT idx = 0;
> +      constructor_elt *cep = NULL;
> +
> +      /* Check if we're changing the active member of a union.  */
> +      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
> +	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
> +	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
> +      /* If the bit offset of INDEX is larger than that of the last
> +	 constructor_elt, then we can just immediately append a new
> +	 constructor_elt to the end of CTOR.  */
> +      else if (CONSTRUCTOR_NELTS (ctor)
> +	       && tree_int_cst_compare (bit_position (index),
> +					bit_position (CONSTRUCTOR_ELTS (ctor)
> +						      ->last().index)) > 0)
> +	{
> +	  idx = CONSTRUCTOR_NELTS (ctor);
> +	  goto insert;
> +	}
> +
> +      /* Otherwise, we need to iterate over CTOR to find or insert INDEX
> +	 appropriately.  */
> +
> +      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
> +	   idx++, fields = DECL_CHAIN (fields))
> +	{
> +	  if (index == cep->index)
> +	    goto found;
> +
> +	  /* The field we're initializing must be on the field
> +	     list.  Look to see if it is present before the
> +	     field the current ELT initializes.  */
> +	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
> +	    if (index == fields)
> +	      goto insert;
> +	}
> +      /* We fell off the end of the CONSTRUCTOR, so insert a new
> +	 entry at the end.  */
> +
> +    insert:
> +      {
> +	constructor_elt ce = { index, NULL_TREE };
> +
> +	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
> +	cep = CONSTRUCTOR_ELT (ctor, idx);
> +      }
> +    found:;
> +
> +      return cep;
> +    }
> +}
> +
>  /* Under the control of CTX, issue a detailed diagnostic for
>     an out-of-bounds subscript INDEX into the expression ARRAY.  */
>  
> @@ -3760,14 +3849,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>      {
>        tree orig_value = value;
>        init_subob_ctx (ctx, new_ctx, index, value);
> +      int pos_hint = -1;
>        if (new_ctx.ctor != ctx->ctor)
> -	/* If we built a new CONSTRUCTOR, attach it now so that other
> -	   initializers can refer to it.  */
> -	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> +	{
> +	  /* If we built a new CONSTRUCTOR, attach it now so that other
> +	     initializers can refer to it.  */
> +	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> +	  pos_hint = vec_safe_length (*p) - 1;
> +	}
>        else if (TREE_CODE (type) == UNION_TYPE)
> -	/* Otherwise if we're constructing a union, set the active union member
> -	   anyway so that we can later detect if the initializer attempts to
> -	   activate another member.  */
> +	/* Otherwise if we're constructing a non-aggregate union member, set
> +	   the active union member now so that we can later detect and diagnose
> +	   if its initializer attempts to activate another member.  */
>  	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
>        tree elt = cxx_eval_constant_expression (&new_ctx, value,
>  					       lval,
> @@ -3804,18 +3897,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>  	{
>  	  if (TREE_CODE (type) == UNION_TYPE
>  	      && (*p)->last().index != index)
> -	    /* The initializer may have erroneously changed the active union
> -	       member that we're initializing.  */
> +	    /* The initializer erroneously changed the active union member that
> +	       we're initializing.  */
>  	    gcc_assert (*non_constant_p);
> -	  else if (new_ctx.ctor != ctx->ctor
> -		   || TREE_CODE (type) == UNION_TYPE)
> +	  else
>  	    {
> -	      /* We appended this element above; update the value.  */
> -	      gcc_assert ((*p)->last().index == index);
> -	      (*p)->last().value = elt;
> +	      /* The initializer might have mutated the underlying CONSTRUCTOR,
> +		 so recompute the location of the target constructer_elt.  */
> +	      constructor_elt *cep
> +		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
> +	      cep->value = elt;
>  	    }
> -	  else
> -	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
> +
>  	  /* Adding or replacing an element might change the ctor's flags.  */
>  	  TREE_CONSTANT (ctx->ctor) = constant_p;
>  	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
> @@ -4590,8 +4683,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>    type = TREE_TYPE (object);
>    bool no_zero_init = true;
>  
> -  releasing_vec ctors;
> -  bool changed_active_union_member_p = false;
> +  releasing_vec ctors, indexes;
> +  auto_vec<int> index_pos_hints;
> +  bool activated_union_member_p = false;
>    while (!refs->is_empty ())
>      {
>        if (*valp == NULL_TREE)
> @@ -4632,94 +4726,49 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>  	 subobjects will also be zero-initialized.  */
>        no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
>  
> -      vec_safe_push (ctors, *valp);
> -
>        enum tree_code code = TREE_CODE (type);
>        type = refs->pop();
>        tree index = refs->pop();
>  
> -      constructor_elt *cep = NULL;
> -      if (code == ARRAY_TYPE)
> +      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> +	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>  	{
> -	  HOST_WIDE_INT i
> -	    = find_array_ctor_elt (*valp, index, /*insert*/true);
> -	  gcc_assert (i >= 0);
> -	  cep = CONSTRUCTOR_ELT (*valp, i);
> -	  gcc_assert (cep->index == NULL_TREE
> -		      || TREE_CODE (cep->index) != RANGE_EXPR);
> -	}
> -      else
> -	{
> -	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
> -
> -	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
> -	     Usually we meet initializers in that order, but it is
> -	     possible for base types to be placed not in program
> -	     order.  */
> -	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
> -	  unsigned HOST_WIDE_INT idx;
> -
> -	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
> -	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
> +	  if (cxx_dialect < cxx2a)
>  	    {
> -	      if (cxx_dialect < cxx2a)
> -		{
> -		  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,
> -			      index);
> -		  *non_constant_p = true;
> -		}
> -	      else if (TREE_CODE (t) == MODIFY_EXPR
> -		       && CONSTRUCTOR_NO_CLEARING (*valp))
> -		{
> -		  /* Diagnose changing the active union member while the union
> -		     is in the process of being initialized.  */
> -		  if (!ctx->quiet)
> -		    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,
> -			      index);
> -		  *non_constant_p = true;
> -		}
> -	      /* Changing active member.  */
> -	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
> -	      no_zero_init = true;
> +	      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,
> +			  index);
> +	      *non_constant_p = true;
>  	    }
> -
> -	  for (idx = 0;
> -	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
> -	       idx++, fields = DECL_CHAIN (fields))
> +	  else if (TREE_CODE (t) == MODIFY_EXPR
> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
>  	    {
> -	      if (index == cep->index)
> -		goto found;
> -
> -	      /* The field we're initializing must be on the field
> -		 list.  Look to see if it is present before the
> -		 field the current ELT initializes.  */
> -	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
> -		if (index == fields)
> -		  goto insert;
> +	      /* Diagnose changing the active union member while the union
> +		 is in the process of being initialized.  */
> +	      if (!ctx->quiet)
> +		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,
> +			  index);
> +	      *non_constant_p = true;
>  	    }
> +	  no_zero_init = true;
> +	}
>  
> -	  /* We fell off the end of the CONSTRUCTOR, so insert a new
> -	     entry at the end.  */
> -	insert:
> -	  {
> -	    constructor_elt ce = { index, NULL_TREE };
> +      vec_safe_push (ctors, *valp);
> +      vec_safe_push (indexes, index);
>  
> -	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
> -	    cep = CONSTRUCTOR_ELT (*valp, idx);
> +      constructor_elt *cep
> +	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
> +      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELTS (*valp)->begin());
> +
> +      if (code == UNION_TYPE)
> +	activated_union_member_p = true;
>  
> -	    if (code == UNION_TYPE)
> -	      /* Record that we've changed an active union member.  */
> -	      changed_active_union_member_p = true;
> -	  }
> -	found:;
> -	}
>        valp = &cep->value;
>      }
>  
> @@ -4800,9 +4849,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>  	  init = tinit;
>        init = cxx_eval_constant_expression (&new_ctx, init, false,
>  					   non_constant_p, overflow_p);
> -      if (ctors->is_empty())
> -	/* The hash table might have moved since the get earlier.  */
> -	valp = ctx->global->values.get (object);
> +      /* The hash table might have moved since the get earlier, and the
> +	 initializer might have mutated the underlying CONSTRUCTORs, so we must
> +	 recompute VALP. */
> +      valp = ctx->global->values.get (object);
> +      for (unsigned i = 0; i < vec_safe_length (indexes); i++)
> +	{
> +	  constructor_elt *cep
> +	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
> +	  valp = &cep->value;
> +	}
>      }
>  
>    /* Don't share a CONSTRUCTOR that might be changed later.  */
> @@ -4847,7 +4903,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>    unsigned i;
>    bool c = TREE_CONSTANT (init);
>    bool s = TREE_SIDE_EFFECTS (init);
> -  if (!c || s || changed_active_union_member_p)
> +  if (!c || s || activated_union_member_p)
>      FOR_EACH_VEC_ELT (*ctors, i, elt)
>        {
>  	if (!c)
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
> new file mode 100644
> index 00000000000..0f91e93ca3f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
> @@ -0,0 +1,19 @@
> +// PR c++/94205
> +// { dg-do compile { target c++14 } }
> +
> +struct S
> +{
> +  struct A
> +  {
> +    S *p;
> +    constexpr A(S* p): p(p) {}
> +    constexpr operator int() { p->a = 5; return 6; }
> +  };
> +  int a = A(this);
> +};
> +
> +
> +constexpr S s = {};
> +
> +#define SA(X) static_assert((X),#X)
> +SA(s.a == 6);
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
> new file mode 100644
> index 00000000000..0ceef1bb29f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
> @@ -0,0 +1,21 @@
> +// PR c++/94219
> +// { dg-do compile { target c++14 } }
> +
> +struct A { long x; };
> +
> +struct U;
> +constexpr A foo(U *up);
> +
> +struct U {
> +  A a = foo(this); int y;
> +};
> +
> +constexpr A foo(U *up) {
> +  up->y = 11;
> +  return {42};
> +}
> +
> +extern constexpr U u = {};
> +
> +static_assert(u.y == 11, "");
> +static_assert(u.a.x == 42, "");
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
> new file mode 100644
> index 00000000000..59e7a10d6e8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
> @@ -0,0 +1,22 @@
> +// PR c++/94219
> +// { dg-do compile { target c++14 } }
> +
> +struct A { long x; };
> +
> +struct U;
> +constexpr A foo(U *up);
> +
> +struct U {
> +  U() = default;
> +  int y; A a = foo(this);
> +};
> +
> +constexpr A foo(U *up) {
> +  up->y = 11;
> +  return {42};
> +}
> +
> +extern constexpr U u = {};
> +
> +static_assert(u.y == 11, "");
> +static_assert(u.a.x == 42, "");
> diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
> new file mode 100644
> index 00000000000..c6d44d7fd0b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
> @@ -0,0 +1,11 @@
> +// PR c++/94205
> +// { dg-do compile { target c++17 } }
> +
> +struct S
> +{
> +  int a = [this] { this->a = 5; return 6; } ();
> +};
> +
> +constexpr S s = {};
> +
> +static_assert(s.a == 6);
> -- 
> 2.26.0.106.g9fadedd637
> 
> 

Re: [PATCH] c++: Fix constexpr evaluation of self-modifying CONSTRUCTORs [PR94219]

[Jason Merrill]

On 4/3/20 4:57 PM, Patrick Palka wrote:
> On Fri, 3 Apr 2020, Patrick Palka wrote:
> 
>> On Fri, 3 Apr 2020, Jason Merrill wrote:
>>
>>> On 4/2/20 2:19 PM, Patrick Palka wrote:
>>>> On Thu, 2 Apr 2020, Patrick Palka wrote:
>>>>
>>>>> This PR reveals that cxx_eval_bare_aggregate and cxx_eval_store_expression
>>>>> do
>>>>> not anticipate that a constructor element's initializer could mutate the
>>>>> underlying CONSTRUCTOR.  Evaluation of the initializer could add new
>>>>> elements to
>>>>> the underlying CONSTRUCTOR, thereby potentially invalidating any pointers
>>>>> to
>>>>> or assumptions about the CONSTRUCTOR's elements, and so these routines
>>>>> should be
>>>>> prepared for that.
>>>>>
>>>>> To fix this problem, this patch makes cxx_eval_bare_aggregate and
>>>>> cxx_eval_store_expression recompute the pointer to the constructor_elt's
>>>>> through
>>>>> which we're assigning, after it evaluates the initializer.  Care is taken
>>>>> to
>>>>> to make the common case where the initializer does not modify the
>>>>> underlying
>>>>> CONSTRUCTOR as fast as before.
>>>>
>>>> Also, with this patch, I'm not totally sure but I think we might not
>>>> need the special preeval handling in cxx_eval_store_expression anymore.
>>>> I could try to remove it in a subsequent patch.
>>>
>>> I think for C++17 order of evaluation it's better to keep preevaluating when
>>> we can.
>>
>> Sounds good.
>>
>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 	PR c++/94205
>>>>> 	PR c++/94219
>>>>> 	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
>>>>> 	support for VECTOR_TYPEs, and optimizations for the common case)
>>>>> 	from ...
>>>>> 	(cxx_eval_store_expression): ... here.  Rename local variable
>>>>> 	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
>>>>> 	the sequence of indexes into 'indexes' that yields the subobject we're
>>>>> 	assigning to.  Record the integer offsets of the constructor indexes
>>>>> 	we're assigning through into 'index_pos_hints'.  After evaluating the
>>>>> 	initializer of the store expression, recompute 'valp' using 'indexes'
>>>>> 	and 'index_pos_hints' as hints.
>>>>> 	(cxx_eval_bare_aggregate): Tweak comments.  Use
>>>>> get_or_insert_ctor_field
>>>>> 	to recompute the pointer to the constructor_elt we're assigning
>>>>> through
>>>>> 	after evaluating each initializer.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 	PR c++/94205
>>>>> 	PR c++/94219
>>>>> 	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
>>>>> 	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
>>>>> 	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
>>>>> 	* g++.dg/cpp1z/lambda-this5.C: New test.
>>>>> ---
>>>>>    gcc/cp/constexpr.c                            | 252 +++++++++++-------
>>>>>    gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
>>>>>    gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
>>>>>    gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
>>>>>    gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
>>>>>    5 files changed, 228 insertions(+), 97 deletions(-)
>>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>>>>>
>>>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>>>>> index 91f0c3ba269..b4173c595f0 100644
>>>>> --- a/gcc/cp/constexpr.c
>>>>> +++ b/gcc/cp/constexpr.c
>>>>> @@ -3151,6 +3151,97 @@ find_array_ctor_elt (tree ary, tree dindex, bool
>>>>> insert)
>>>>>      return -1;
>>>>>    }
>>>>>    +/* Return a pointer to the constructor_elt of CTOR which matches INDEX.
>>>>> If no
>>>>> +   matching constructor_elt exists, then add one to CTOR.
>>>>> +
>>>>> +   As an optimization, if POS_HINT is non-negative then it is used as a
>>>>> guess
>>>>> +   for the (integer) index of the matching constructor_elt within CTOR.
>>>>> */
>>>>> +
>>>>> +static constructor_elt *
>>>>> +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
>>>>> +{
>>>>> +  tree type = TREE_TYPE (ctor);
>>>>> +  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
>>>>> +    {
>>>>> +      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
>>>>> +      return &CONSTRUCTOR_ELTS (ctor)->last();
>>>>> +    }
>>>>> +  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) ==
>>>>> VECTOR_TYPE)
>>>>> +    {
>>>>> +      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index,
>>>>> /*insert*/true);
>>>>> +      gcc_assert (i >= 0);
>>>>> +      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
>>>>> +      gcc_assert (cep->index == NULL_TREE
>>>>> +		  || TREE_CODE (cep->index) != RANGE_EXPR);
>>>>> +      return cep;
>>>>> +    }
>>>>> +  else
>>>>> +    {
>>>>> +      gcc_assert (TREE_CODE (index) == FIELD_DECL);
>>>>> +
>>>>> +      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>>>>> +	 Usually we meet initializers in that order, but it is
>>>>> +	 possible for base types to be placed not in program
>>>>> +	 order.  */
>>>>> +      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>>>>> +      unsigned HOST_WIDE_INT idx = 0;
>>>>> +      constructor_elt *cep = NULL;
>>>>> +
>>>>> +      /* First, check if we're changing the active member of a union.  */
>>>>> +      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
>>>>> +	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
>>>>> +	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
>>>>> +      /* Next, check the hint.  */
>>>>> +      else if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS
>>>>> (ctor)
>>>>> +	       && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
>>>>> +	{
>>>>> +	  cep = CONSTRUCTOR_ELT (ctor, pos_hint);
>>>>> +	  goto found;
>>>>> +	}
>>>
>>> Don't we want to use pos_hint for all aggregate types?
>>
>> Oops, yes.  I moved the hint check up to happen first and
>> unconditionally.
>>
>>>
>>>>> +      /* If the hint was wrong, and if the bit offset of INDEX is larger
>>>>> than
>>>>> +	 that of the last constructor_elt, then we can just immediately append
>>>>> a
>>>>> +	 new constructor_elt to the end of CTOR.  */
>>>>> +      else if (CONSTRUCTOR_NELTS (ctor)
>>>>> +	       && tree_int_cst_compare (bit_position (index),
>>>>> +					bit_position (CONSTRUCTOR_ELTS (ctor)
>>>>> +						      ->last().index)) > 0)
>>>>> +	{
>>>>> +	  idx = CONSTRUCTOR_NELTS (ctor);
>>>>> +	  goto insert;
>>>>> +	}
>>>>> +
>>>>> +      /* Otherwise, we need to iterator over CTOR to find or insert INDEX
>>>
>>> s/iterator/iterate/
>>
>> Fixed.
>>
>>>
>>>>> +	 appropriately.  */
>>>>> +
>>>>> +      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
>>>>> +	   idx++, fields = DECL_CHAIN (fields))
>>>>> +	{
>>>>> +	  if (index == cep->index)
>>>>> +	    goto found;
>>>>> +
>>>>> +	  /* The field we're initializing must be on the field
>>>>> +	     list.  Look to see if it is present before the
>>>>> +	     field the current ELT initializes.  */
>>>>> +	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
>>>>> +	    if (index == fields)
>>>>> +	      goto insert;
>>>>> +	}
>>>>> +      /* We fell off the end of the CONSTRUCTOR, so insert a new
>>>>> +	 entry at the end.  */
>>>>> +
>>>>> +    insert:
>>>>> +      {
>>>>> +	constructor_elt ce = { index, NULL_TREE };
>>>>> +
>>>>> +	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
>>>>> +	cep = CONSTRUCTOR_ELT (ctor, idx);
>>>>> +      }
>>>>> +    found:;
>>>>> +
>>>>> +      return cep;
>>>>> +    }
>>>>> +}
>>>>> +
>>>>>    /* Under the control of CTX, issue a detailed diagnostic for
>>>>>       an out-of-bounds subscript INDEX into the expression ARRAY.  */
>>>>>    @@ -3760,14 +3851,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx
>>>>> *ctx, tree t,
>>>>>        {
>>>>>          tree orig_value = value;
>>>>>          init_subob_ctx (ctx, new_ctx, index, value);
>>>>> +      int pos_hint = -1;
>>>>>          if (new_ctx.ctor != ctx->ctor)
>>>>> -	/* If we built a new CONSTRUCTOR, attach it now so that other
>>>>> -	   initializers can refer to it.  */
>>>>> -	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>>>>> +	{
>>>>> +	  /* If we built a new CONSTRUCTOR, attach it now so that other
>>>>> +	     initializers can refer to it.  */
>>>>> +	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>>>>> +	  pos_hint = vec_safe_length (*p) - 1;
>>>>> +	}
>>>>>          else if (TREE_CODE (type) == UNION_TYPE)
>>>>> -	/* Otherwise if we're constructing a union, set the active union
>>>>> member
>>>>> -	   anyway so that we can later detect if the initializer attempts to
>>>>> -	   activate another member.  */
>>>>> +	/* Otherwise if we're constructing a non-aggregate union member, set
>>>>> +	   the active union member now so that we can later detect and
>>>>> diagnose
>>>>> +	   if its initializer attempts to activate another member.  */
>>>>>    	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
>>>>>          tree elt = cxx_eval_constant_expression (&new_ctx, value,
>>>>>    					       lval,
>>>>> @@ -3804,18 +3899,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx,
>>>>> tree t,
>>>>>    	{
>>>>>    	  if (TREE_CODE (type) == UNION_TYPE
>>>>>    	      && (*p)->last().index != index)
>>>>> -	    /* The initializer may have erroneously changed the active union
>>>>> -	       member that we're initializing.  */
>>>>> +	    /* The initializer erroneously changed the active union member
>>>>> that
>>>>> +	       we're initializing.  */
>>>>>    	    gcc_assert (*non_constant_p);
>>>>> -	  else if (new_ctx.ctor != ctx->ctor
>>>>> -		   || TREE_CODE (type) == UNION_TYPE)
>>>>> +	  else
>>>>>    	    {
>>>>> -	      /* We appended this element above; update the value.  */
>>>>> -	      gcc_assert ((*p)->last().index == index);
>>>>> -	      (*p)->last().value = elt;
>>>>> +	      /* The initializer might have mutated the underlying
>>>>> CONSTRUCTOR,
>>>>> +		 so recompute the location of the target constructer_elt.  */
>>>>> +	      constructor_elt *cep
>>>>> +		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
>>>>> +	      cep->value = elt;
>>>>>    	    }
>>>>> -	  else
>>>>> -	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
>>>>> +
>>>>>    	  /* Adding or replacing an element might change the ctor's flags.  */
>>>>>    	  TREE_CONSTANT (ctx->ctor) = constant_p;
>>>>>    	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
>>>>> @@ -4590,8 +4685,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
>>>>> tree t,
>>>>>      type = TREE_TYPE (object);
>>>>>      bool no_zero_init = true;
>>>>>    -  releasing_vec ctors;
>>>>> -  bool changed_active_union_member_p = false;
>>>>> +  releasing_vec ctors, indexes;
>>>>> +  auto_vec<int> index_pos_hints;
>>>>> +  bool activated_union_member_p = false;
>>>>>      while (!refs->is_empty ())
>>>>>        {
>>>>>          if (*valp == NULL_TREE)
>>>>> @@ -4632,94 +4728,49 @@ cxx_eval_store_expression (const constexpr_ctx
>>>>> *ctx, tree t,
>>>>>    	 subobjects will also be zero-initialized.  */
>>>>>          no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
>>>>>    -      vec_safe_push (ctors, *valp);
>>>>> -
>>>>>          enum tree_code code = TREE_CODE (type);
>>>>>          type = refs->pop();
>>>>>          tree index = refs->pop();
>>>>>    -      constructor_elt *cep = NULL;
>>>>> -      if (code == ARRAY_TYPE)
>>>>> +      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>>>>> +	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>>>>>    	{
>>>>> -	  HOST_WIDE_INT i
>>>>> -	    = find_array_ctor_elt (*valp, index, /*insert*/true);
>>>>> -	  gcc_assert (i >= 0);
>>>>> -	  cep = CONSTRUCTOR_ELT (*valp, i);
>>>>> -	  gcc_assert (cep->index == NULL_TREE
>>>>> -		      || TREE_CODE (cep->index) != RANGE_EXPR);
>>>>> -	}
>>>>> -      else
>>>>> -	{
>>>>> -	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
>>>>> -
>>>>> -	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>>>>> -	     Usually we meet initializers in that order, but it is
>>>>> -	     possible for base types to be placed not in program
>>>>> -	     order.  */
>>>>> -	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>>>>> -	  unsigned HOST_WIDE_INT idx;
>>>>> -
>>>>> -	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>>>>> -	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>>>>> +	  if (cxx_dialect < cxx2a)
>>>>>    	    {
>>>>> -	      if (cxx_dialect < cxx2a)
>>>>> -		{
>>>>> -		  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,
>>>>> -			      index);
>>>>> -		  *non_constant_p = true;
>>>>> -		}
>>>>> -	      else if (TREE_CODE (t) == MODIFY_EXPR
>>>>> -		       && CONSTRUCTOR_NO_CLEARING (*valp))
>>>>> -		{
>>>>> -		  /* Diagnose changing the active union member while the union
>>>>> -		     is in the process of being initialized.  */
>>>>> -		  if (!ctx->quiet)
>>>>> -		    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,
>>>>> -			      index);
>>>>> -		  *non_constant_p = true;
>>>>> -		}
>>>>> -	      /* Changing active member.  */
>>>>> -	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
>>>>> -	      no_zero_init = true;
>>>>> +	      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,
>>>>> +			  index);
>>>>> +	      *non_constant_p = true;
>>>>>    	    }
>>>>> -
>>>>> -	  for (idx = 0;
>>>>> -	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
>>>>> -	       idx++, fields = DECL_CHAIN (fields))
>>>>> +	  else if (TREE_CODE (t) == MODIFY_EXPR
>>>>> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
>>>>>    	    {
>>>>> -	      if (index == cep->index)
>>>>> -		goto found;
>>>>> -
>>>>> -	      /* The field we're initializing must be on the field
>>>>> -		 list.  Look to see if it is present before the
>>>>> -		 field the current ELT initializes.  */
>>>>> -	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
>>>>> -		if (index == fields)
>>>>> -		  goto insert;
>>>>> +	      /* Diagnose changing the active union member while the union
>>>>> +		 is in the process of being initialized.  */
>>>>> +	      if (!ctx->quiet)
>>>>> +		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,
>>>>> +			  index);
>>>>> +	      *non_constant_p = true;
>>>>>    	    }
>>>>> +	  no_zero_init = true;
>>>>> +	}
>>>>>    -	  /* We fell off the end of the CONSTRUCTOR, so insert a new
>>>>> -	     entry at the end.  */
>>>>> -	insert:
>>>>> -	  {
>>>>> -	    constructor_elt ce = { index, NULL_TREE };
>>>>> +      vec_safe_push (ctors, *valp);
>>>>> +      vec_safe_push (indexes, index);
>>>>>    -	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
>>>>> -	    cep = CONSTRUCTOR_ELT (*valp, idx);
>>>>> +      constructor_elt *cep
>>>>> +	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
>>>>> +      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELT (*valp, 0));
>>>
>>> I wonder if you want to use valp->begin() rather than CONSTRUCTOR_ELT?
>>
>> Fixed to do CONSTRUCTOR_ELTS (*valp)->begin().
> 
> Hmm, but I don't think this is necessary, because after the call to
> get_or_insert_ctor_field, *valp will surely have at least one
> constructor_elt.

No, it's not necessary.  The patch is OK either way.

>>
>> Here's the updated patch with the above changes incorporated:
>>
>> -- >8 --
>>
>> gcc/cp/ChangeLog:
>>
>> 	PR c++/94219
>> 	PR c++/94205
>> 	* constexpr.c (get_or_insert_ctor_field): Split out (while adding
>> 	support for VECTOR_TYPEs, and optimizations for the common case)
>> 	from ...
>> 	(cxx_eval_store_expression): ... here.  Rename local variable
>> 	'changed_active_union_member_p' to 'activated_union_member_p'.  Record
>> 	the sequence of indexes into 'indexes' that yields the subobject we're
>> 	assigning to.  Record the integer offsets of the constructor indexes
>> 	we're assigning through into 'index_pos_hints'.  After evaluating the
>> 	initializer of the store expression, recompute 'valp' using 'indexes'
>> 	and using 'index_pos_hints' as hints.
>> 	(cxx_eval_bare_aggregate): Tweak comments.  Use get_or_insert_ctor_field
>> 	to recompute the pointer to the constructor_elt we're assigning through
>> 	after evaluating each initializer.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR c++/94219
>> 	PR c++/94205
>> 	* g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
>> 	* g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
>> 	* g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
>> 	* g++.dg/cpp1z/lambda-this5.C: New test.
>> ---
>>   gcc/cp/constexpr.c                            | 250 +++++++++++-------
>>   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C |  19 ++
>>   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C |  21 ++
>>   gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C |  22 ++
>>   gcc/testsuite/g++.dg/cpp1z/lambda-this5.C     |  11 +
>>   5 files changed, 226 insertions(+), 97 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>>
>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>> index 91f0c3ba269..3c5137bc3b0 100644
>> --- a/gcc/cp/constexpr.c
>> +++ b/gcc/cp/constexpr.c
>> @@ -3151,6 +3151,95 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert)
>>     return -1;
>>   }
>>   
>> +/* Return a pointer to the constructor_elt of CTOR which matches INDEX.  If no
>> +   matching constructor_elt exists, then add one to CTOR.
>> +
>> +   As an optimization, if POS_HINT is non-negative then it is used as a guess
>> +   for the (integer) index of the matching constructor_elt within CTOR.  */
>> +
>> +static constructor_elt *
>> +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
>> +{
>> +  /* Check the hint first.  */
>> +  if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor)
>> +      && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
>> +    return CONSTRUCTOR_ELT (ctor, pos_hint);
>> +
>> +  tree type = TREE_TYPE (ctor);
>> +  if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
>> +    {
>> +      CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
>> +      return &CONSTRUCTOR_ELTS (ctor)->last();
>> +    }
>> +  else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
>> +    {
>> +      HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
>> +      gcc_assert (i >= 0);
>> +      constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
>> +      gcc_assert (cep->index == NULL_TREE
>> +		  || TREE_CODE (cep->index) != RANGE_EXPR);
>> +      return cep;
>> +    }
>> +  else
>> +    {
>> +      gcc_assert (TREE_CODE (index) == FIELD_DECL);
>> +
>> +      /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>> +	 Usually we meet initializers in that order, but it is
>> +	 possible for base types to be placed not in program
>> +	 order.  */
>> +      tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>> +      unsigned HOST_WIDE_INT idx = 0;
>> +      constructor_elt *cep = NULL;
>> +
>> +      /* Check if we're changing the active member of a union.  */
>> +      if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
>> +	  && CONSTRUCTOR_ELT (ctor, 0)->index != index)
>> +	vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
>> +      /* If the bit offset of INDEX is larger than that of the last
>> +	 constructor_elt, then we can just immediately append a new
>> +	 constructor_elt to the end of CTOR.  */
>> +      else if (CONSTRUCTOR_NELTS (ctor)
>> +	       && tree_int_cst_compare (bit_position (index),
>> +					bit_position (CONSTRUCTOR_ELTS (ctor)
>> +						      ->last().index)) > 0)
>> +	{
>> +	  idx = CONSTRUCTOR_NELTS (ctor);
>> +	  goto insert;
>> +	}
>> +
>> +      /* Otherwise, we need to iterate over CTOR to find or insert INDEX
>> +	 appropriately.  */
>> +
>> +      for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
>> +	   idx++, fields = DECL_CHAIN (fields))
>> +	{
>> +	  if (index == cep->index)
>> +	    goto found;
>> +
>> +	  /* The field we're initializing must be on the field
>> +	     list.  Look to see if it is present before the
>> +	     field the current ELT initializes.  */
>> +	  for (; fields != cep->index; fields = DECL_CHAIN (fields))
>> +	    if (index == fields)
>> +	      goto insert;
>> +	}
>> +      /* We fell off the end of the CONSTRUCTOR, so insert a new
>> +	 entry at the end.  */
>> +
>> +    insert:
>> +      {
>> +	constructor_elt ce = { index, NULL_TREE };
>> +
>> +	vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
>> +	cep = CONSTRUCTOR_ELT (ctor, idx);
>> +      }
>> +    found:;
>> +
>> +      return cep;
>> +    }
>> +}
>> +
>>   /* Under the control of CTX, issue a detailed diagnostic for
>>      an out-of-bounds subscript INDEX into the expression ARRAY.  */
>>   
>> @@ -3760,14 +3849,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>>       {
>>         tree orig_value = value;
>>         init_subob_ctx (ctx, new_ctx, index, value);
>> +      int pos_hint = -1;
>>         if (new_ctx.ctor != ctx->ctor)
>> -	/* If we built a new CONSTRUCTOR, attach it now so that other
>> -	   initializers can refer to it.  */
>> -	CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>> +	{
>> +	  /* If we built a new CONSTRUCTOR, attach it now so that other
>> +	     initializers can refer to it.  */
>> +	  CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>> +	  pos_hint = vec_safe_length (*p) - 1;
>> +	}
>>         else if (TREE_CODE (type) == UNION_TYPE)
>> -	/* Otherwise if we're constructing a union, set the active union member
>> -	   anyway so that we can later detect if the initializer attempts to
>> -	   activate another member.  */
>> +	/* Otherwise if we're constructing a non-aggregate union member, set
>> +	   the active union member now so that we can later detect and diagnose
>> +	   if its initializer attempts to activate another member.  */
>>   	CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
>>         tree elt = cxx_eval_constant_expression (&new_ctx, value,
>>   					       lval,
>> @@ -3804,18 +3897,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>>   	{
>>   	  if (TREE_CODE (type) == UNION_TYPE
>>   	      && (*p)->last().index != index)
>> -	    /* The initializer may have erroneously changed the active union
>> -	       member that we're initializing.  */
>> +	    /* The initializer erroneously changed the active union member that
>> +	       we're initializing.  */
>>   	    gcc_assert (*non_constant_p);
>> -	  else if (new_ctx.ctor != ctx->ctor
>> -		   || TREE_CODE (type) == UNION_TYPE)
>> +	  else
>>   	    {
>> -	      /* We appended this element above; update the value.  */
>> -	      gcc_assert ((*p)->last().index == index);
>> -	      (*p)->last().value = elt;
>> +	      /* The initializer might have mutated the underlying CONSTRUCTOR,
>> +		 so recompute the location of the target constructer_elt.  */
>> +	      constructor_elt *cep
>> +		= get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
>> +	      cep->value = elt;
>>   	    }
>> -	  else
>> -	    CONSTRUCTOR_APPEND_ELT (*p, index, elt);
>> +
>>   	  /* Adding or replacing an element might change the ctor's flags.  */
>>   	  TREE_CONSTANT (ctx->ctor) = constant_p;
>>   	  TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
>> @@ -4590,8 +4683,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>     type = TREE_TYPE (object);
>>     bool no_zero_init = true;
>>   
>> -  releasing_vec ctors;
>> -  bool changed_active_union_member_p = false;
>> +  releasing_vec ctors, indexes;
>> +  auto_vec<int> index_pos_hints;
>> +  bool activated_union_member_p = false;
>>     while (!refs->is_empty ())
>>       {
>>         if (*valp == NULL_TREE)
>> @@ -4632,94 +4726,49 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>   	 subobjects will also be zero-initialized.  */
>>         no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
>>   
>> -      vec_safe_push (ctors, *valp);
>> -
>>         enum tree_code code = TREE_CODE (type);
>>         type = refs->pop();
>>         tree index = refs->pop();
>>   
>> -      constructor_elt *cep = NULL;
>> -      if (code == ARRAY_TYPE)
>> +      if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>> +	  && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>>   	{
>> -	  HOST_WIDE_INT i
>> -	    = find_array_ctor_elt (*valp, index, /*insert*/true);
>> -	  gcc_assert (i >= 0);
>> -	  cep = CONSTRUCTOR_ELT (*valp, i);
>> -	  gcc_assert (cep->index == NULL_TREE
>> -		      || TREE_CODE (cep->index) != RANGE_EXPR);
>> -	}
>> -      else
>> -	{
>> -	  gcc_assert (TREE_CODE (index) == FIELD_DECL);
>> -
>> -	  /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>> -	     Usually we meet initializers in that order, but it is
>> -	     possible for base types to be placed not in program
>> -	     order.  */
>> -	  tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>> -	  unsigned HOST_WIDE_INT idx;
>> -
>> -	  if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>> -	      && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>> +	  if (cxx_dialect < cxx2a)
>>   	    {
>> -	      if (cxx_dialect < cxx2a)
>> -		{
>> -		  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,
>> -			      index);
>> -		  *non_constant_p = true;
>> -		}
>> -	      else if (TREE_CODE (t) == MODIFY_EXPR
>> -		       && CONSTRUCTOR_NO_CLEARING (*valp))
>> -		{
>> -		  /* Diagnose changing the active union member while the union
>> -		     is in the process of being initialized.  */
>> -		  if (!ctx->quiet)
>> -		    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,
>> -			      index);
>> -		  *non_constant_p = true;
>> -		}
>> -	      /* Changing active member.  */
>> -	      vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
>> -	      no_zero_init = true;
>> +	      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,
>> +			  index);
>> +	      *non_constant_p = true;
>>   	    }
>> -
>> -	  for (idx = 0;
>> -	       vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
>> -	       idx++, fields = DECL_CHAIN (fields))
>> +	  else if (TREE_CODE (t) == MODIFY_EXPR
>> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
>>   	    {
>> -	      if (index == cep->index)
>> -		goto found;
>> -
>> -	      /* The field we're initializing must be on the field
>> -		 list.  Look to see if it is present before the
>> -		 field the current ELT initializes.  */
>> -	      for (; fields != cep->index; fields = DECL_CHAIN (fields))
>> -		if (index == fields)
>> -		  goto insert;
>> +	      /* Diagnose changing the active union member while the union
>> +		 is in the process of being initialized.  */
>> +	      if (!ctx->quiet)
>> +		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,
>> +			  index);
>> +	      *non_constant_p = true;
>>   	    }
>> +	  no_zero_init = true;
>> +	}
>>   
>> -	  /* We fell off the end of the CONSTRUCTOR, so insert a new
>> -	     entry at the end.  */
>> -	insert:
>> -	  {
>> -	    constructor_elt ce = { index, NULL_TREE };
>> +      vec_safe_push (ctors, *valp);
>> +      vec_safe_push (indexes, index);
>>   
>> -	    vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
>> -	    cep = CONSTRUCTOR_ELT (*valp, idx);
>> +      constructor_elt *cep
>> +	= get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
>> +      index_pos_hints.safe_push (cep - CONSTRUCTOR_ELTS (*valp)->begin());
>> +
>> +      if (code == UNION_TYPE)
>> +	activated_union_member_p = true;
>>   
>> -	    if (code == UNION_TYPE)
>> -	      /* Record that we've changed an active union member.  */
>> -	      changed_active_union_member_p = true;
>> -	  }
>> -	found:;
>> -	}
>>         valp = &cep->value;
>>       }
>>   
>> @@ -4800,9 +4849,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>   	  init = tinit;
>>         init = cxx_eval_constant_expression (&new_ctx, init, false,
>>   					   non_constant_p, overflow_p);
>> -      if (ctors->is_empty())
>> -	/* The hash table might have moved since the get earlier.  */
>> -	valp = ctx->global->values.get (object);
>> +      /* The hash table might have moved since the get earlier, and the
>> +	 initializer might have mutated the underlying CONSTRUCTORs, so we must
>> +	 recompute VALP. */
>> +      valp = ctx->global->values.get (object);
>> +      for (unsigned i = 0; i < vec_safe_length (indexes); i++)
>> +	{
>> +	  constructor_elt *cep
>> +	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
>> +	  valp = &cep->value;
>> +	}
>>       }
>>   
>>     /* Don't share a CONSTRUCTOR that might be changed later.  */
>> @@ -4847,7 +4903,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>>     unsigned i;
>>     bool c = TREE_CONSTANT (init);
>>     bool s = TREE_SIDE_EFFECTS (init);
>> -  if (!c || s || changed_active_union_member_p)
>> +  if (!c || s || activated_union_member_p)
>>       FOR_EACH_VEC_ELT (*ctors, i, elt)
>>         {
>>   	if (!c)
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>> new file mode 100644
>> index 00000000000..0f91e93ca3f
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>> @@ -0,0 +1,19 @@
>> +// PR c++/94205
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct S
>> +{
>> +  struct A
>> +  {
>> +    S *p;
>> +    constexpr A(S* p): p(p) {}
>> +    constexpr operator int() { p->a = 5; return 6; }
>> +  };
>> +  int a = A(this);
>> +};
>> +
>> +
>> +constexpr S s = {};
>> +
>> +#define SA(X) static_assert((X),#X)
>> +SA(s.a == 6);
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>> new file mode 100644
>> index 00000000000..0ceef1bb29f
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>> @@ -0,0 +1,21 @@
>> +// PR c++/94219
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct A { long x; };
>> +
>> +struct U;
>> +constexpr A foo(U *up);
>> +
>> +struct U {
>> +  A a = foo(this); int y;
>> +};
>> +
>> +constexpr A foo(U *up) {
>> +  up->y = 11;
>> +  return {42};
>> +}
>> +
>> +extern constexpr U u = {};
>> +
>> +static_assert(u.y == 11, "");
>> +static_assert(u.a.x == 42, "");
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>> new file mode 100644
>> index 00000000000..59e7a10d6e8
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>> @@ -0,0 +1,22 @@
>> +// PR c++/94219
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct A { long x; };
>> +
>> +struct U;
>> +constexpr A foo(U *up);
>> +
>> +struct U {
>> +  U() = default;
>> +  int y; A a = foo(this);
>> +};
>> +
>> +constexpr A foo(U *up) {
>> +  up->y = 11;
>> +  return {42};
>> +}
>> +
>> +extern constexpr U u = {};
>> +
>> +static_assert(u.y == 11, "");
>> +static_assert(u.a.x == 42, "");
>> diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>> new file mode 100644
>> index 00000000000..c6d44d7fd0b
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>> @@ -0,0 +1,11 @@
>> +// PR c++/94205
>> +// { dg-do compile { target c++17 } }
>> +
>> +struct S
>> +{
>> +  int a = [this] { this->a = 5; return 6; } ();
>> +};
>> +
>> +constexpr S s = {};
>> +
>> +static_assert(s.a == 6);
>> -- 
>> 2.26.0.106.g9fadedd637
>>
>>
>