Re: [PATCH v2] c++: Fix ICE in reshape_init with init-list [PR95164]

[Marek Polacek <polacek@redhat.com>]

On Mon, Sep 07, 2020 at 06:23:01PM -0400, Jason Merrill via Gcc-patches wrote:
> On 9/4/20 5:39 PM, Marek Polacek wrote:
> > This patch fixes a long-standing bug in reshape_init_r.  Since r209314
> > we implement DR 1467 which handles list-initialization with a single
> > initializer of the same type as the target.  In this test this causes
> > a crash in reshape_init_r when we're processing a constructor that has
> > undergone the DR 1467 transformation.
> > 
> > Take e.g. the
> > 
> >    foo({{1, {H{k}}}});
> > 
> > line in the attached test.  {H{k}} initializes the field b of H in I.
> > H{k} is a functional cast, so has TREE_HAS_CONSTRUCTOR set, so is
> > COMPOUND_LITERAL_P.  We perform the DR 1467 transformation and turn
> > {H{k}} into H{k}.  Then we attempt to reshape H{k} again and since
> > first_initializer_p is null and it's COMPOUND_LITERAL_P, we go here:
> > 
> >             else if (COMPOUND_LITERAL_P (stripped_init))
> >               gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init));
> 
> It looks to me like the bug is here:
> 
> >   /* [dcl.init.aggr]
> > All implicit type conversions (clause _conv_) are considered when
> > initializing the aggregate member with an initializer from an
> > initializer-list.  If the initializer can initialize a member,
> > the member is initialized.  Otherwise, if the member is itself a
> > non-empty subaggregate, brace elision is assumed and the
> > initializer is considered for the initialization of the first
> > member of the subaggregate.  */
> >   if (TREE_CODE (init) != CONSTRUCTOR
> >       /* But don't try this for the first initializer, since that would
> > be                                              looking through the
> > outermost braces; A a2 = { a1 }; is not a
> > valid aggregate initialization.  */
> >       && !first_initializer_p
> >       && (same_type_ignoring_top_level_qualifiers_p (type, TREE_TYPE (init))
> >           || can_convert_arg (type, TREE_TYPE (init), init, LOOKUP_NORMAL,
> >                               complain)))
> >     {
> >       d->cur++;
> >       return init;
> >     }
> 
> We ought to handle H{k} here, treat it as the initializer for the member,
> and not get as far as the code you quote above.

Like this?  When we have a COMPOUND_LITERAL_P, then I think we don't need
to check cxx11, or CLASS_TYPE, or d.end - d.cur, because that's inherent.

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

-- >8 --
This patch fixes a long-standing bug in reshape_init_r.  Since r209314
we implement DR 1467 which handles list-initialization with a single
initializer of the same type as the target.  In this test this causes
a crash in reshape_init_r when we're processing a constructor that has
undergone the DR 1467 transformation.

Take e.g. the

  foo({{1, {H{k}}}});

line in the attached test.  {H{k}} initializes the field b of H in I.
H{k} is a functional cast, so has TREE_HAS_CONSTRUCTOR set, so is
COMPOUND_LITERAL_P.  We perform the DR 1467 transformation and turn
{H{k}} into H{k}.  Then we attempt to reshape H{k} again and since
first_initializer_p is null and it's COMPOUND_LITERAL_P, we go here:

           else if (COMPOUND_LITERAL_P (stripped_init))
             gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init));

then complain about the missing braces, go to reshape_init_class and ICE
on
               gcc_checking_assert (d->cur->index
                                    == get_class_binding (type, id));

because due to the missing { } we're looking for 'b' in H, but that's
not found.

So we have to be prepared to handle an initializer whose outer braces
have been removed due to DR 1467.

gcc/cp/ChangeLog:

	PR c++/95164
	* decl.c (reshape_init_r): When we've found a missing set of braces
	as a result of the DR 1467 transformation, don't reshape again.

gcc/testsuite/ChangeLog:

	PR c++/95164
	* g++.dg/cpp0x/initlist123.C: New test.
---
 gcc/cp/decl.c                            |  8 ++++-
 gcc/testsuite/g++.dg/cpp0x/initlist123.C | 39 ++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist123.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 31d68745844..6565cd7199b 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6466,7 +6466,13 @@ reshape_init_r (tree type, reshape_iter *d, tree first_initializer_p,
      non-empty subaggregate, brace elision is assumed and the
      initializer is considered for the initialization of the first
      member of the subaggregate.  */
-  if (TREE_CODE (init) != CONSTRUCTOR
+  if ((TREE_CODE (init) != CONSTRUCTOR
+       /* If we previously elided the braces around the single element
+	  of an initializer list when initializing an object of the same
+	  class type, don't report missing braces or reshape again.  In
+	  this case the braces had been enclosing a compound literal or
+	  functional cast with aggregate, e.g. {S{}} -> S{}.  */
+       || COMPOUND_LITERAL_P (init))
       /* But don't try this for the first initializer, since that would be
 	 looking through the outermost braces; A a2 = { a1 }; is not a
 	 valid aggregate initialization.  */
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist123.C b/gcc/testsuite/g++.dg/cpp0x/initlist123.C
new file mode 100644
index 00000000000..29f037f07ef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist123.C
@@ -0,0 +1,39 @@
+// PR c++/95164
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wmissing-braces" }
+
+struct H {
+  int a;
+};
+
+struct X : H { };
+
+struct I {
+  int c;
+  H b;
+};
+struct E { I d; };
+void foo(E);
+
+template<int N>
+void fn ()
+{
+  int a = 42;
+  int &k = a;
+
+  foo({1, {H{k}}}); // { dg-warning "missing braces around initializer for .I." }
+  foo({1, {X{k}}}); // { dg-warning "missing braces around initializer for .I." }
+
+  foo({{1, {k}}});
+  foo({{1, {N}}});
+
+  foo({{1, H{k}}});
+  foo({{1, H{N}}});
+  foo({{1, X{k}}});
+  foo({{1, X{N}}});
+
+  foo({{1, {H{k}}}});
+  foo({{1, {H{N}}}});
+  foo({{1, {X{k}}}});
+  foo({{1, {X{N}}}});
+}

base-commit: 919373a6bfff415db7676c9f92a356ddfc501dfe
-- 
2.26.2