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

[Jason Merrill]

On 9/9/20 3:33 PM, Marek Polacek wrote:
> 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{}.  */

Don't we also get here for a compound literal without elided braces? 
I'm not sure this comment is needed.  OK either way.

> +       || 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
>