[PATCH v2] c++: Fix ICE with ill-formed array list-initialization [PR93712]
Jason Merrill
jason@redhat.com
Mon Feb 24 14:54:00 GMT 2020
On 2/21/20 6:48 PM, Marek Polacek wrote:
> On Thu, Feb 20, 2020 at 12:28:42AM +0000, Jason Merrill wrote:
>> On 2/19/20 7:30 PM, Marek Polacek wrote:
>>> On Fri, Feb 14, 2020 at 09:12:58AM +0100, Jason Merrill wrote:
>>>> On 2/13/20 8:56 PM, Marek Polacek wrote:
>>>>> My P0388R4 patch changed build_array_conv to create an identity
>>>>> conversion at the start of the conversion chain.
>>>>
>>>> Hmm, an identity conversion of {} suggests that it has a type, which it
>>>> doesn't in the language. I'm not strongly against it, but what was the
>>>> reason for this change?
>>>
>>> There are two reasons:
>>> 1) without it we couldn't get to the original expression at the start
>>> of the conversion chain (saved in .u.expr), this is needed in compare_ics:
>>> 10660 tree n1 = nelts_initialized_by_list_init (t1);
>>> 10661 tree n2 = nelts_initialized_by_list_init (t2);
>>> and nelts_initialized_by_list_init uses conv_get_original_expr for
>>> arrays that have no dimensions.
>>
>> Ah, ck_aggr and ck_list probably should have used u.expr like ck_identity
>> and ck_ambig....
>>
>>> 2) struct conversion says
>>> /* An implicit conversion sequence, in the sense of [over.best.ics].
>>> The first conversion to be performed is at the end of the chain.
>>> That conversion is always a cr_identity conversion. */
>>> and we were breaking that promise.
>>
>> ...or, if we're going to enforce this, ck_ambig will need to change as well.
>>
>> And build_aggr_conv and build_complex_conv will need adjusting (one way or
>> another).
>
> Changed my mind :-). Turned out using u.expr for ck_aggr is a much smaller
> change. Moreover, that struct conversion talks about cr_identity, a rank,
> not ck_identity.
>
> ck_list uses u.list, so I'm not changing that.
>
> -- >8 --
> My P0388R4 patch changed build_array_conv to create an identity
> conversion at the start of the conversion chain and now we crash
> in convert_like_real:
>
> 7457 case ck_identity:
> 7458 if (BRACE_ENCLOSED_INITIALIZER_P (expr))
> 7459 {
> 7460 int nelts = CONSTRUCTOR_NELTS (expr);
> 7461 if (nelts == 0)
> 7462 expr = build_value_init (totype, complain);
> 7463 else if (nelts == 1)
> 7464 expr = CONSTRUCTOR_ELT (expr, 0)->value;
> 7465 else
> 7466 gcc_unreachable (); // HERE
> 7467 }
>
> in a test like this
>
> int f (int const (&)[2])
> { return f({1, "M"}); }
>
> Instead of creating a ck_identity at the start of the conversion chain,
> so that conv_get_original_expr can be used with a ck_aggr, let's set
> u.expr for a ck_aggr, and adjust next_conversion not to try to see
> what's next in the chain if it gets a ck_aggr.
>
> Bootstrapped/regtested on x86_64-linux, built cmcstl2 and Boost, ok for
> trunk?
OK.
> 2020-02-21 Marek Polacek <polacek@redhat.com>
>
> PR c++/93712 - ICE with ill-formed array list-initialization.
> * call.c (next_conversion): Return NULL for ck_aggr.
> (build_aggr_conv): Set u.expr instead of u.next.
> (build_array_conv): Likewise.
> (build_complex_conv): Likewise.
> (conv_get_original_expr): Handle ck_aggr.
>
> * g++.dg/cpp0x/initlist-array11.C: New test.
> ---
> gcc/cp/call.c | 17 +++++++++--------
> gcc/testsuite/g++.dg/cpp0x/initlist-array11.C | 10 ++++++++++
> 2 files changed, 19 insertions(+), 8 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-array11.C
>
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index f47f96bf1c2..84230b9ecb8 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -117,13 +117,13 @@ struct conversion {
> /* The next conversion in the chain. Since the conversions are
> arranged from outermost to innermost, the NEXT conversion will
> actually be performed before this conversion. This variant is
> - used only when KIND is neither ck_identity, ck_ambig nor
> + used only when KIND is neither ck_identity, ck_aggr, ck_ambig nor
> ck_list. Please use the next_conversion function instead
> of using this field directly. */
> conversion *next;
> /* The expression at the beginning of the conversion chain. This
> - variant is used only if KIND is ck_identity or ck_ambig. You can
> - use conv_get_original_expr to get this expression. */
> + variant is used only if KIND is ck_identity, ck_aggr, or ck_ambig.
> + You can use conv_get_original_expr to get this expression. */
> tree expr;
> /* The array of conversions for an initializer_list, so this
> variant is used only when KIN D is ck_list. */
> @@ -861,7 +861,8 @@ next_conversion (conversion *conv)
> if (conv == NULL
> || conv->kind == ck_identity
> || conv->kind == ck_ambig
> - || conv->kind == ck_list)
> + || conv->kind == ck_list
> + || conv->kind == ck_aggr)
> return NULL;
> return conv->u.next;
> }
> @@ -1030,7 +1031,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
> c->rank = cr_exact;
> c->user_conv_p = true;
> c->check_narrowing = true;
> - c->u.next = NULL;
> + c->u.expr = ctor;
> return c;
> }
>
> @@ -1083,7 +1084,7 @@ build_array_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
> c->rank = rank;
> c->user_conv_p = user;
> c->bad_p = bad;
> - c->u.next = build_identity_conv (TREE_TYPE (ctor), ctor);
> + c->u.expr = ctor;
> return c;
> }
>
> @@ -1129,7 +1130,7 @@ build_complex_conv (tree type, tree ctor, int flags,
> c->rank = rank;
> c->user_conv_p = user;
> c->bad_p = bad;
> - c->u.next = NULL;
> + c->u.expr = ctor;
> return c;
> }
>
> @@ -10498,7 +10499,7 @@ static tree
> conv_get_original_expr (conversion *c)
> {
> for (; c; c = next_conversion (c))
> - if (c->kind == ck_identity || c->kind == ck_ambig)
> + if (c->kind == ck_identity || c->kind == ck_ambig || c->kind == ck_aggr)
> return c->u.expr;
> return NULL_TREE;
> }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-array11.C b/gcc/testsuite/g++.dg/cpp0x/initlist-array11.C
> new file mode 100644
> index 00000000000..d3e58dea129
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/initlist-array11.C
> @@ -0,0 +1,10 @@
> +// PR c++/93712 - ICE with ill-formed array list-initialization.
> +// { dg-do compile { target c++11 } }
> +
> +int f (const int (&)[2]);
> +
> +int g ()
> +{
> + const int (&r)[2] = {1, "foo"}; // { dg-error "conversion" }
> + return f({1, "foo"}); // { dg-error "conversion" }
> +}
>
> base-commit: 8d1780b56d0cb1d50115d4e925e81cd8b9cb2923
>
More information about the Gcc-patches
mailing list