C++ PATCH for c++/84927, ICE with NSDMI and reference

This nice test crashes in verify_constructor_flags because while the
constructor as a whole is TREE_CONSTANT, one of its elements is not.

We wound up in this situation because when we first get to
cxx_eval_bare_aggregate, ctx->ctor is "{}", which is TREE_CONSTANT, while T is
"{.r=(int &) &j, .i=*(&<PLACEHOLDER_EXPR struct A>)->r}".  We evaluate the
first element of T and then append it to the empty ctx->ctor, so now ctx->ctor
is "{.r=(int &) &j}", a constant ctor with a non-constant element.  Then we get
onto evaluating the second element of T.  The second elt has a
PLACEHOLDER_EXPR, so we get to
4695     case PLACEHOLDER_EXPR:
4696       /* Use of the value or address of the current object.  */
4697       if (tree ctor = lookup_placeholder (ctx, lval, TREE_TYPE (t)))
4698         return cxx_eval_constant_expression (ctx, ctor, lval,
4699                                              non_constant_p, overflow_p);
where lookup_placeholder returns "{.r=(int &) &j}", which we try to
evaluate, and crash in verify_constructor_flags in the CONSTRUCTOR case.

I think let's update the flags of the constructor as we evaluate elements in
cxx_eval_bare_aggregate; this should be cheaper than calling
recompute_constructor_flags someplace else.

I took the liberty to remove a stale FIXME; testing didn't reveal any new
issues.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-03-19  Marek Polacek  <polacek@redhat.com>

	PR c++/84927
	* constexpr.c (cxx_eval_bare_aggregate): Update constructor's flags
	as we evaluate the elements.
	(cxx_eval_constant_expression): Verify constructor's flags
	unconditionally.

	* g++.dg/cpp1y/nsdmi-aggr9.C: New test.


	Marek

OK.

On Mon, Mar 19, 2018 at 2:22 PM, Marek Polacek <polacek@redhat.com> wrote:
> This nice test crashes in verify_constructor_flags because while the
> constructor as a whole is TREE_CONSTANT, one of its elements is not.
>
> We wound up in this situation because when we first get to
> cxx_eval_bare_aggregate, ctx->ctor is "{}", which is TREE_CONSTANT, while T is
> "{.r=(int &) &j, .i=*(&<PLACEHOLDER_EXPR struct A>)->r}".  We evaluate the
> first element of T and then append it to the empty ctx->ctor, so now ctx->ctor
> is "{.r=(int &) &j}", a constant ctor with a non-constant element.  Then we get
> onto evaluating the second element of T.  The second elt has a
> PLACEHOLDER_EXPR, so we get to
> 4695     case PLACEHOLDER_EXPR:
> 4696       /* Use of the value or address of the current object.  */
> 4697       if (tree ctor = lookup_placeholder (ctx, lval, TREE_TYPE (t)))
> 4698         return cxx_eval_constant_expression (ctx, ctor, lval,
> 4699                                              non_constant_p, overflow_p);
> where lookup_placeholder returns "{.r=(int &) &j}", which we try to
> evaluate, and crash in verify_constructor_flags in the CONSTRUCTOR case.
>
> I think let's update the flags of the constructor as we evaluate elements in
> cxx_eval_bare_aggregate; this should be cheaper than calling
> recompute_constructor_flags someplace else.
>
> I took the liberty to remove a stale FIXME; testing didn't reveal any new
> issues.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-03-19  Marek Polacek  <polacek@redhat.com>
>
>         PR c++/84927
>         * constexpr.c (cxx_eval_bare_aggregate): Update constructor's flags
>         as we evaluate the elements.
>         (cxx_eval_constant_expression): Verify constructor's flags
>         unconditionally.
>
>         * g++.dg/cpp1y/nsdmi-aggr9.C: New test.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 05a1cb64d61..894bcd0bb3e 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -2880,7 +2880,12 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>           (*p)->last().value = elt;
>         }
>        else
> -       CONSTRUCTOR_APPEND_ELT (*p, index, elt);
> +       {
> +         CONSTRUCTOR_APPEND_ELT (*p, index, elt);
> +         /* Adding an element might change the ctor's flags.  */
> +         TREE_CONSTANT (ctx->ctor) = constant_p;
> +         TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
> +       }
>      }
>    if (*non_constant_p || !changed)
>      return t;
> @@ -4530,11 +4535,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>         {
>           /* Don't re-process a constant CONSTRUCTOR, but do fold it to
>              VECTOR_CST if applicable.  */
> -         /* FIXME after GCC 6 branches, make the verify unconditional.  */
> -         if (CHECKING_P)
> -           verify_constructor_flags (t);
> -         else
> -           recompute_constructor_flags (t);
> +         verify_constructor_flags (t);
>           if (TREE_CONSTANT (t))
>             return fold (t);
>         }
> diff --git gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr9.C gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr9.C
> index e69de29bb2d..4e13fc5c9d8 100644
> --- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr9.C
> +++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr9.C
> @@ -0,0 +1,14 @@
> +// PR c++/84927 - ICE with NSDMI and reference
> +// { dg-do compile { target c++14 } }
> +
> +struct A
> +{
> +  int& r;
> +  int i = r;
> +};
> +
> +void foo()
> +{
> +  int j;
> +  A a = A{j};
> +}
>
>         Marek