* [PATCH] c++: Fix ICE in reshape_init with init-list [PR95164]
@ 2020-09-04 21:39 Marek Polacek
2020-09-07 22:23 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Marek Polacek @ 2020-09-04 21:39 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
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.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and 10?
gcc/cp/ChangeLog:
PR c++/95164
* decl.c (list_init_from_same_type_p): New function.
(reshape_init_r): Call it. When we've found a missing set of braces,
but list_init_from_same_type_p is true, don't reshape again.
gcc/testsuite/ChangeLog:
PR c++/95164
* g++.dg/cpp0x/initlist123.C: New test.
---
gcc/cp/decl.c | 43 +++++++++++++++++++-----
gcc/testsuite/g++.dg/cpp0x/initlist123.C | 39 +++++++++++++++++++++
2 files changed, 74 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist123.C
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 31d68745844..a07d52cdf5b 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6350,6 +6350,21 @@ has_designator_problem (reshape_iter *d, tsubst_flags_t complain)
return false;
}
+/* Handle [dcl.init.list]p3.2: Return true if we should elide braces when we're
+ initializing an object of class type (or, as an extension, vector type) by
+ list-initialization with a single initializer of the same type as the target.
+
+ TYPE is the type of the target, INIT_TYPE is the type of the element of the
+ initializer list, and D is the iterator within the CONSTRUCTOR. */
+
+static bool
+list_init_from_same_type_p (reshape_iter *d, tree type, tree init_type)
+{
+ return (cxx_dialect >= cxx11 && (CLASS_TYPE_P (type) || VECTOR_TYPE_P (type))
+ && d->end - d->cur == 1
+ && reference_related_p (type, init_type));
+}
+
/* Subroutine of reshape_init, which processes a single initializer (part of
a CONSTRUCTOR). TYPE is the type of the variable being initialized, D is the
iterator within the CONSTRUCTOR which points to the initializer to process.
@@ -6445,13 +6460,13 @@ reshape_init_r (tree type, reshape_iter *d, tree first_initializer_p,
return init;
}
- /* "If T is a class type and the initializer list has a single element of
- type cv U, where U is T or a class derived from T, the object is
- initialized from that element." Even if T is an aggregate. */
- if (cxx_dialect >= cxx11 && (CLASS_TYPE_P (type) || VECTOR_TYPE_P (type))
- && first_initializer_p
- && d->end - d->cur == 1
- && reference_related_p (type, TREE_TYPE (init)))
+ /* [dcl.init.list]p3.2: "If T is an aggregate class and the initializer list
+ has a single element of type cv U, where U is T or a class derived from T,
+ the object is initialized from that element (by copy-initialization for
+ copy-list-initialization, or by direct-initialization for
+ direct-list-initialization)." */
+ if (first_initializer_p
+ && list_init_from_same_type_p (d, type, TREE_TYPE (init)))
{
d->cur++;
return init;
@@ -6531,7 +6546,19 @@ reshape_init_r (tree type, reshape_iter *d, tree first_initializer_p,
/* For a nested compound literal, proceed to specialized routines,
to handle initialization of arrays and similar. */
else if (COMPOUND_LITERAL_P (stripped_init))
- gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init));
+ {
+ gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init));
+ /* 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{}. */
+ if (list_init_from_same_type_p (d, type, init_type))
+ {
+ ++d->cur;
+ return init;
+ }
+ }
/* A CONSTRUCTOR of the target's type is a previously
digested initializer. */
else if (same_type_ignoring_top_level_qualifiers_p (type, init_type))
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: f923c40f9baba19e58f65afa7e5572f08cee93ff
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] c++: Fix ICE in reshape_init with init-list [PR95164]
2020-09-04 21:39 [PATCH] c++: Fix ICE in reshape_init with init-list [PR95164] Marek Polacek
@ 2020-09-07 22:23 ` Jason Merrill
2020-09-09 19:33 ` [PATCH v2] " Marek Polacek
0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2020-09-07 22:23 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
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.
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] c++: Fix ICE in reshape_init with init-list [PR95164]
2020-09-07 22:23 ` Jason Merrill
@ 2020-09-09 19:33 ` Marek Polacek
2020-09-09 21:03 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Marek Polacek @ 2020-09-09 19:33 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] c++: Fix ICE in reshape_init with init-list [PR95164]
2020-09-09 19:33 ` [PATCH v2] " Marek Polacek
@ 2020-09-09 21:03 ` Jason Merrill
0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2020-09-09 21:03 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-09 21:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 21:39 [PATCH] c++: Fix ICE in reshape_init with init-list [PR95164] Marek Polacek
2020-09-07 22:23 ` Jason Merrill
2020-09-09 19:33 ` [PATCH v2] " Marek Polacek
2020-09-09 21:03 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).