On Wed, Jan 30, 2019 at 04:11:11PM -0500, Marek Polacek wrote:
> On Tue, Jan 29, 2019 at 09:40:18PM -0500, Jason Merrill wrote:
> > On Tue, Jan 29, 2019 at 6:53 PM Marek Polacek <pola...@redhat.com> wrote:
> > >
> > > My recent patch for 88815 and 78244 caused 89083, a P1 9 regression, which
> > > happens to be the same problem as 80864 and its many dupes, something I'd
> > > been meaning to fix for a long time.
> > >
> > > Basically, the problem is repeated reshaping of a constructor, once when
> > > parsing, and then again when substituting. With the recent fix, we call
> > > reshape_init + digest_init in finish_compound_literal even in a template
> > > if the expression is not instantiation-dependent, and then again when
> > > tsubst_*.
> > >
> > > For instance, in initlist107.C, when parsing a functional cast, we call
> > > finish_compound_literal which calls reshape_init, which turns
> > >
> > > { NON_LVALUE_EXPR<1>, NON_LVALUE_EXPR<2> }
> > >
> > > into
> > >
> > > { { NON_LVALUE_EXPR<1>, NON_LVALUE_EXPR<2> } }
> > >
> > > and then digest_init turns that into
> > >
> > > { .x = { 1, 2 } }
> > >
> > > which is a compound literal (TREE_HAS_CONSTRUCTOR set), but the
> > > subexpression
> > > "{ 1, 2 }" isn't. "{ 1, 2 }" will now have the type int[3], so it's not
> > > BRACE_ENCLOSED_INITIALIZER_P.
> > >
> > > And then tsubst_* processes "{ .x = { 1, 2 } }". The case CONSTRUCTOR
> > > in tsubst_copy_and_build will call finish_compound_literal on a copy of
> > > "{ 1, 2 }" wrapped in a new { }, because the whole expr has
> > > TREE_HAS_CONSTRUCTOR.
> > > That crashes in reshape_init_r in the
> > > 6155 if (TREE_CODE (stripped_init) == CONSTRUCTOR)
> > > block; we have a constructor, it's not COMPOUND_LITERAL_P, and because
> > > digest_init had given it the type int[3], we hit
> > > 6172 gcc_assert (BRACE_ENCLOSED_INITIALIZER_P
> > > (stripped_init));
> > >
> > > As expand_default_init explains in a comment, a CONSTRUCTOR of the
> > > target's type
> > > is a previously digested initializer, so we should probably do a similar
> > > trick
> > > here. This fixes all the variants of the problem I've come up with.
> > >
> > > 80864 is a similar case, we reshape when parsing and then second time in
> > > fold_non_dependent_expr called from store_init_value, because of the
> > > 'constexpr'.
> > >
> > > Also update a stale comment.
> > >
> > > Bootstrapped/regtest running on x86_64-linux, ok for trunk and 8 after a
> > > while?
> > >
> > > 2019-01-29 Marek Polacek <pola...@redhat.com>
> > >
> > > PR c++/89083, c++/80864 - ICE with list initialization in
> > > template.
> > > * decl.c (reshape_init_r): Don't reshape a digested initializer.
> > >
> > > * g++.dg/cpp0x/initlist107.C: New test.
> > > * g++.dg/cpp0x/initlist108.C: New test.
> > > * g++.dg/cpp0x/initlist109.C: New test.
> > >
> > > diff --git gcc/cp/decl.c gcc/cp/decl.c
> > > index 79eeac177b6..da08ecc21aa 100644
> > > --- gcc/cp/decl.c
> > > +++ gcc/cp/decl.c
> > > @@ -6161,11 +6161,17 @@ reshape_init_r (tree type, reshape_iter *d, bool
> > > first_initializer_p,
> > > ;
> > > else if (COMPOUND_LITERAL_P (stripped_init))
> > > /* For a nested compound literal, there is no need to reshape
> > > since
> > > - brace elision is not allowed. Even if we decided to allow it,
> > > - we should add a call to reshape_init in
> > > finish_compound_literal,
> > > - before calling digest_init, so changing this code would still
> > > - not be necessary. */
> > > + we called reshape_init in finish_compound_literal, before
> > > calling
> > > + digest_init. */
> > > gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init));
> > > + /* Similarly, a CONSTRUCTOR of the target's type is a previously
> > > + digested initializer. */
> > > + else if (same_type_ignoring_top_level_qualifiers_p (type,
> > > + TREE_TYPE
> > > (init)))
> >
> > Hmm, aren't both of these tests true for a dependent compound literal,
> > which won't have been reshaped already?
>
> I'm hoping that can't happen, but it's a good question. When we have a
> dependent compound literal, finish_compound_literal just sets
> TREE_HAS_CONSTRUCTOR and returns it, so then in tsubst_*, after substituting
> each element of the constructor, we call finish_compound_literal. The
> constructor is no longer dependent and since we operate on a copy on which
> we didn't set TREE_HAS_CONSTRUCTOR, the first condition shouldn't be true.
>
> And the second condition should also never be true for a compound literal
> that hasn't been reshaped, because digest_init is only ever called after
> reshape_init (and the comment for digest_init_r says it assumes that
> reshape_init has already run). The type of a CONSTRUCTOR can also by changed
> in tsubst_copy_and_build:
> 19269 TREE_TYPE (r) = type;
> but I haven't been able to trigger any problem yet. Worst comes to worst this
> patch changes the ICE to another ICE, but I'm not finding a testcase.
And if that ever happens, I guess we'll have to add a new flag that says if
a CONSTRUCTOR has been reshaped already.
Marek
