Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Apr 08 2019, Marek Polacek wrote: > Thanks, committed. Andreas -- I hope the failure is fixed now. Yes, all tests of aggr-base[89].C are passing. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Mon, Apr 08, 2019 at 03:24:13PM -0400, Jason Merrill wrote: > On 4/1/19 11:23 AM, Marek Polacek wrote: > > On Mon, Apr 01, 2019 at 10:15:11AM +0200, Andreas Schwab wrote: > > > On Mär 28 2019, Marek Polacek wrote: > > > > > > > Andreas, could you please find out why we're not hitting this code in > > > > digest_init_r: > > > > > > > > 1210 tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value; > > > > 1211 if (reference_related_p (type, TREE_TYPE (elt))) > > > > > > This is never executed if flag_checking is false, of course. > > > > It's certainly wrong for a warning to depend on flag_checking, so this > > patch corrects it, and I hope will fix the ia64 problem as well. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > 2019-04-01 Marek Polacek > > > > * typeck2.c (digest_init_r): Don't condition the object slicing warning > > on flag_checking. > > OK. Thanks, committed. Andreas -- I hope the failure is fixed now. Marek
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On 4/1/19 11:23 AM, Marek Polacek wrote: On Mon, Apr 01, 2019 at 10:15:11AM +0200, Andreas Schwab wrote: On Mär 28 2019, Marek Polacek wrote: Andreas, could you please find out why we're not hitting this code in digest_init_r: 1210 tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value; 1211 if (reference_related_p (type, TREE_TYPE (elt))) This is never executed if flag_checking is false, of course. It's certainly wrong for a warning to depend on flag_checking, so this patch corrects it, and I hope will fix the ia64 problem as well. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-04-01 Marek Polacek * typeck2.c (digest_init_r): Don't condition the object slicing warning on flag_checking. OK. Jason
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
Any comments?
On Mon, Apr 01, 2019 at 11:23:29AM -0400, Marek Polacek wrote:
> On Mon, Apr 01, 2019 at 10:15:11AM +0200, Andreas Schwab wrote:
> > On Mär 28 2019, Marek Polacek wrote:
> >
> > > Andreas, could you please find out why we're not hitting this code in
> > > digest_init_r:
> > >
> > > 1210 tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
> > > 1211 if (reference_related_p (type, TREE_TYPE (elt)))
> >
> > This is never executed if flag_checking is false, of course.
>
> It's certainly wrong for a warning to depend on flag_checking, so this
> patch corrects it, and I hope will fix the ia64 problem as well.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2019-04-01 Marek Polacek
>
> * typeck2.c (digest_init_r): Don't condition the object slicing warning
> on flag_checking.
>
> diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
> index fa98b1cb8b5..55b84f043f4 100644
> --- gcc/cp/typeck2.c
> +++ gcc/cp/typeck2.c
> @@ -1200,8 +1200,7 @@ digest_init_r (tree type, tree init, int nested, int
> flags,
>/* "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." */
> - if (flag_checking
> - && cxx_dialect >= cxx11
> + if (cxx_dialect >= cxx11
>&& BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
>&& CONSTRUCTOR_NELTS (stripped_init) == 1
>&& ((CLASS_TYPE_P (type) && !CLASSTYPE_NON_AGGREGATE (type))
> @@ -1228,7 +1227,7 @@ digest_init_r (tree type, tree init, int nested, int
> flags,
> "results in object slicing", TREE_TYPE (field)))
> inform (loc, "remove %<{ }%> around initializer");
> }
> - else
> + else if (flag_checking)
> /* We should have fixed this in reshape_init. */
> gcc_unreachable ();
> }
Marek
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Mon, Apr 01, 2019 at 10:15:11AM +0200, Andreas Schwab wrote:
> On Mär 28 2019, Marek Polacek wrote:
>
> > Andreas, could you please find out why we're not hitting this code in
> > digest_init_r:
> >
> > 1210 tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
> > 1211 if (reference_related_p (type, TREE_TYPE (elt)))
>
> This is never executed if flag_checking is false, of course.
It's certainly wrong for a warning to depend on flag_checking, so this
patch corrects it, and I hope will fix the ia64 problem as well.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-04-01 Marek Polacek
* typeck2.c (digest_init_r): Don't condition the object slicing warning
on flag_checking.
diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
index fa98b1cb8b5..55b84f043f4 100644
--- gcc/cp/typeck2.c
+++ gcc/cp/typeck2.c
@@ -1200,8 +1200,7 @@ digest_init_r (tree type, tree init, int nested, int
flags,
/* "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." */
- if (flag_checking
- && cxx_dialect >= cxx11
+ if (cxx_dialect >= cxx11
&& BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
&& CONSTRUCTOR_NELTS (stripped_init) == 1
&& ((CLASS_TYPE_P (type) && !CLASSTYPE_NON_AGGREGATE (type))
@@ -1228,7 +1227,7 @@ digest_init_r (tree type, tree init, int nested, int
flags,
"results in object slicing", TREE_TYPE (field)))
inform (loc, "remove %<{ }%> around initializer");
}
- else
+ else if (flag_checking)
/* We should have fixed this in reshape_init. */
gcc_unreachable ();
}
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Mär 28 2019, Marek Polacek wrote: > Andreas, could you please find out why we're not hitting this code in > digest_init_r: > > 1210 tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value; > 1211 if (reference_related_p (type, TREE_TYPE (elt))) This is never executed if flag_checking is false, of course. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Thu, Mar 28, 2019 at 01:30:46PM -0400, Jason Merrill wrote:
> On 3/26/19 11:06 AM, Marek Polacek wrote:
> > On Tue, Mar 26, 2019 at 12:34:03PM +0100, Andreas Schwab wrote:
> > > I don't see any of the warnings in the tests on ia64.
> > >
> > > FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 33)
> > > FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 34)
> > > FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 35)
> > > FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 45)
> > > FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 46)
> > > FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 47)
> > > PASS: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for excess errors)
> > >
> > > FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 22)
> > > FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 23)
> > > FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 31)
> > > FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 32)
> > > PASS: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for excess errors)
> >
> > I don't have access to an ia64 box -- I see none on Compile Farm. But if
> > it doesn't ICE, can we just add ! { target ia64-*-* } to the dg-warnings?
>
> Hmm, this behavior shouldn't be dependent on the target.
True. But I can't reproduce the missing warning on e.g. ppc64.
Andreas, could you please find out why we're not hitting this code in
digest_init_r:
1210 tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
1211 if (reference_related_p (type, TREE_TYPE (elt)))
or if we are, is the reference_related_p condition not true?
Marek
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On 3/26/19 11:06 AM, Marek Polacek wrote:
On Tue, Mar 26, 2019 at 12:34:03PM +0100, Andreas Schwab wrote:
I don't see any of the warnings in the tests on ia64.
FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 33)
FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 34)
FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 35)
FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 45)
FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 46)
FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 47)
PASS: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for excess errors)
FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 22)
FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 23)
FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 31)
FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 32)
PASS: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for excess errors)
I don't have access to an ia64 box -- I see none on Compile Farm. But if
it doesn't ICE, can we just add ! { target ia64-*-* } to the dg-warnings?
Hmm, this behavior shouldn't be dependent on the target.
Jason
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Tue, Mar 26, 2019 at 12:34:03PM +0100, Andreas Schwab wrote:
> I don't see any of the warnings in the tests on ia64.
>
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 33)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 34)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 35)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 45)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 46)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 47)
> PASS: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for excess errors)
>
> FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 22)
> FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 23)
> FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 31)
> FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 32)
> PASS: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for excess errors)
I don't have access to an ia64 box -- I see none on Compile Farm. But if
it doesn't ICE, can we just add ! { target ia64-*-* } to the dg-warnings?
Marek
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
I don't see any of the warnings in the tests on ia64. FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 33) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 34) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 35) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 45) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 46) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 47) PASS: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for excess errors) FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 22) FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 23) FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 31) FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 32) PASS: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for excess errors) Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On 3/25/19 12:02 PM, Marek Polacek wrote:
On Fri, Mar 22, 2019 at 04:20:43PM -0400, Jason Merrill wrote:
On 3/22/19 4:12 PM, Jason Merrill wrote:
On 3/22/19 2:14 PM, Marek Polacek wrote:
On Fri, Mar 22, 2019 at 10:48:32AM -0400, Jason Merrill wrote:
+ B b10 = {{B{42}}};
+ B b11 = {{B{{42;
+ B b12 = {{B{{{42};
These look ill-formed to me: too many braces around the B value.
Looks like the original testcase had the same problem. So I
think this is
ice-on-invalid.
Are you sure? clang/icc/gcc8/msvc compile it. I thought this was a
case of
aggregate initialization, where we have a nested braced-init-list:
http://eel.is/c++draft/dcl.init.aggr#4.2
"If an initializer is itself an initializer list, the element is
list-initialized, which will result in a recursive application of
the rules in
this subclause if the element is an aggregate."
Yes. Since the first element of the outer init-list is also an
init-list, we initialize the first element of D from the inner
init-list. The first element of D is the B base, so we're initializing
a B from {D{42}}.
Ah, and D is derived from B, so the B base is copy-initialized from the
(B base subobject of) the D temporary. So that's why it's different for
a base class.
So originally, when calling reshape_init, we have
{{ TARGET_EXPR<> }} of type D
we recurse on it -- it's a class so we call reshape_init_class.
Then we call
reshape_init_r on a field D.2070 for the base (type B), the ctor is
{ TARGET_EXPR<> }.
Here we elide the braces, so we return just the TARGET_EXPR as a
field_init
for D.2070, but then we're back in reshape_init_class and append the
TARGET_EXPR to new_init constructor:
5969 CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (new_init),
field, field_init);
so we end up with the undesirable form.
I don't think it's undesirable; the temporary initializes the base, not
the complete object. It seems that the assert in digest_init_r is wrong
for C++17.
It seems a bit questionable that adding a layer of braces silently
causes slicing; I'll raise this with the committee.
Thanks for that.
I think let's warn instead of aborting if the first element of the aggregate
is a base class.
Okay. I guess we might raise it to an error later, if it becomes invalid.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-03-25 Marek Polacek
PR c++/89214 - ICE when initializing aggregates with bases.
* typeck2.c (digest_init_r): Warn about object slicing instead of
crashing.
OK.
Jason
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Fri, Mar 22, 2019 at 04:20:43PM -0400, Jason Merrill wrote:
> On 3/22/19 4:12 PM, Jason Merrill wrote:
> > On 3/22/19 2:14 PM, Marek Polacek wrote:
> > > On Fri, Mar 22, 2019 at 10:48:32AM -0400, Jason Merrill wrote:
> >
> > > > > > + B b10 = {{B{42}}};
> > > > > > + B b11 = {{B{{42;
> > > > > > + B b12 = {{B{{{42};
> > > >
> > > > These look ill-formed to me: too many braces around the B value.
> > > >
> > > > Looks like the original testcase had the same problem. So I
> > > > think this is
> > > > ice-on-invalid.
> > >
> > > Are you sure? clang/icc/gcc8/msvc compile it. I thought this was a
> > > case of
> > > aggregate initialization, where we have a nested braced-init-list:
> > > http://eel.is/c++draft/dcl.init.aggr#4.2
> > > "If an initializer is itself an initializer list, the element is
> > > list-initialized, which will result in a recursive application of
> > > the rules in
> > > this subclause if the element is an aggregate."
> >
> > Yes. Since the first element of the outer init-list is also an
> > init-list, we initialize the first element of D from the inner
> > init-list. The first element of D is the B base, so we're initializing
> > a B from {D{42}}.
> >
> > Ah, and D is derived from B, so the B base is copy-initialized from the
> > (B base subobject of) the D temporary. So that's why it's different for
> > a base class.
> >
> > > So originally, when calling reshape_init, we have
> > >
> > > {{ TARGET_EXPR<> }} of type D
> > >
> > > we recurse on it -- it's a class so we call reshape_init_class.
> > > Then we call
> > > reshape_init_r on a field D.2070 for the base (type B), the ctor is
> > > { TARGET_EXPR<> }.
> > > Here we elide the braces, so we return just the TARGET_EXPR as a
> > > field_init
> > > for D.2070, but then we're back in reshape_init_class and append the
> > > TARGET_EXPR to new_init constructor:
> > > 5969 CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (new_init),
> > > field, field_init);
> > > so we end up with the undesirable form.
> >
> > I don't think it's undesirable; the temporary initializes the base, not
> > the complete object. It seems that the assert in digest_init_r is wrong
> > for C++17.
> >
> > It seems a bit questionable that adding a layer of braces silently
> > causes slicing; I'll raise this with the committee.
Thanks for that.
> I think let's warn instead of aborting if the first element of the aggregate
> is a base class.
Okay. I guess we might raise it to an error later, if it becomes invalid.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-03-25 Marek Polacek
PR c++/89214 - ICE when initializing aggregates with bases.
* typeck2.c (digest_init_r): Warn about object slicing instead of
crashing.
* g++.dg/cpp1z/aggr-base8.C: New test.
* g++.dg/cpp1z/aggr-base9.C: New test.
diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
index e50d6ed83c3..7f242ba93da 100644
--- gcc/cp/typeck2.c
+++ gcc/cp/typeck2.c
@@ -1209,8 +1209,29 @@ digest_init_r (tree type, tree init, int nested, int
flags,
{
tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
if (reference_related_p (type, TREE_TYPE (elt)))
- /* We should have fixed this in reshape_init. */
- gcc_unreachable ();
+ {
+ /* In C++17, aggregates can have bases, thus participate in
+aggregate initialization. In the following case:
+
+ struct B { int c; };
+ struct D : B { };
+ D d{{D{{42;
+
+ there's an extra set of braces, so the D temporary initializes
+ the first element of d, which is the B base subobject. The base
+ of type B is copy-initialized from the D temporary, causing
+ object slicing. */
+ tree field = next_initializable_field (TYPE_FIELDS (type));
+ if (field && DECL_FIELD_IS_BASE (field))
+ {
+ if (warning_at (loc, 0, "initializing a base class of type %qT "
+ "results in object slicing", TREE_TYPE (field)))
+ inform (loc, "remove %<{ }%> around initializer");
+ }
+ else
+ /* We should have fixed this in reshape_init. */
+ gcc_unreachable ();
+ }
}
if (BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
diff --git gcc/testsuite/g++.dg/cpp1z/aggr-base8.C
gcc/testsuite/g++.dg/cpp1z/aggr-base8.C
new file mode 100644
index 000..8b495a80cb3
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1z/aggr-base8.C
@@ -0,0 +1,48 @@
+// PR c++/89214
+// { dg-do compile { target c++17 } }
+
+struct A
+{
+ A (int);
+};
+
+struct BB
+{
+ A a;
+};
+
+struct B : BB
+{
+};
+
+void
+foo ()
+{
+ B b1 = {42};
+ B b2 = {{42}};
+ B b3 = {{{42}}};
+
+ B b4 = B{42};
+ B b5 = B{{42}};
+ B b6 = B{{{42}}};
+
+ B b7 = {B{42}};
+ B b8 = {B{{42}}};
+ B b9 = {B{{{42;
+
+ B b10 = {{B{42}}}; // { dg-warning "initializing a base class of type .BB.
results in
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On 3/22/19 4:12 PM, Jason Merrill wrote:
On 3/22/19 2:14 PM, Marek Polacek wrote:
On Fri, Mar 22, 2019 at 10:48:32AM -0400, Jason Merrill wrote:
+ B b10 = {{B{42}}};
+ B b11 = {{B{{42;
+ B b12 = {{B{{{42};
These look ill-formed to me: too many braces around the B value.
Looks like the original testcase had the same problem. So I think
this is
ice-on-invalid.
Are you sure? clang/icc/gcc8/msvc compile it. I thought this was a
case of
aggregate initialization, where we have a nested braced-init-list:
http://eel.is/c++draft/dcl.init.aggr#4.2
"If an initializer is itself an initializer list, the element is
list-initialized, which will result in a recursive application of the
rules in
this subclause if the element is an aggregate."
Yes. Since the first element of the outer init-list is also an
init-list, we initialize the first element of D from the inner
init-list. The first element of D is the B base, so we're initializing
a B from {D{42}}.
Ah, and D is derived from B, so the B base is copy-initialized from the
(B base subobject of) the D temporary. So that's why it's different for
a base class.
So originally, when calling reshape_init, we have
{{ TARGET_EXPR<> }} of type D
we recurse on it -- it's a class so we call reshape_init_class. Then
we call
reshape_init_r on a field D.2070 for the base (type B), the ctor is {
TARGET_EXPR<> }.
Here we elide the braces, so we return just the TARGET_EXPR as a
field_init
for D.2070, but then we're back in reshape_init_class and append the
TARGET_EXPR to new_init constructor:
5969 CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (new_init), field,
field_init);
so we end up with the undesirable form.
I don't think it's undesirable; the temporary initializes the base, not
the complete object. It seems that the assert in digest_init_r is wrong
for C++17.
It seems a bit questionable that adding a layer of braces silently
causes slicing; I'll raise this with the committee.
I think let's warn instead of aborting if the first element of the
aggregate is a base class.
Jason
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On 3/22/19 2:14 PM, Marek Polacek wrote:
On Fri, Mar 22, 2019 at 10:48:32AM -0400, Jason Merrill wrote:
+ B b10 = {{B{42}}};
+ B b11 = {{B{{42;
+ B b12 = {{B{{{42};
These look ill-formed to me: too many braces around the B value.
Looks like the original testcase had the same problem. So I think this is
ice-on-invalid.
Are you sure? clang/icc/gcc8/msvc compile it. I thought this was a case of
aggregate initialization, where we have a nested braced-init-list:
http://eel.is/c++draft/dcl.init.aggr#4.2
"If an initializer is itself an initializer list, the element is
list-initialized, which will result in a recursive application of the rules in
this subclause if the element is an aggregate."
Yes. Since the first element of the outer init-list is also an
init-list, we initialize the first element of D from the inner
init-list. The first element of D is the B base, so we're initializing
a B from {D{42}}.
Ah, and D is derived from B, so the B base is copy-initialized from the
(B base subobject of) the D temporary. So that's why it's different for
a base class.
So originally, when calling reshape_init, we have
{{ TARGET_EXPR<> }} of type D
we recurse on it -- it's a class so we call reshape_init_class. Then we call
reshape_init_r on a field D.2070 for the base (type B), the ctor is {
TARGET_EXPR<> }.
Here we elide the braces, so we return just the TARGET_EXPR as a field_init
for D.2070, but then we're back in reshape_init_class and append the
TARGET_EXPR to new_init constructor:
5969 CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (new_init), field,
field_init);
so we end up with the undesirable form.
I don't think it's undesirable; the temporary initializes the base, not
the complete object. It seems that the assert in digest_init_r is wrong
for C++17.
It seems a bit questionable that adding a layer of braces silently
causes slicing; I'll raise this with the committee.
Jason
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Fri, Mar 22, 2019 at 10:48:32AM -0400, Jason Merrill wrote:
> On 3/21/19 4:51 PM, Marek Polacek wrote:
> > This is a crash in digest_init_r -- we encounter
> >
> >/* "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." */
> >if (flag_checking
> >&& cxx_dialect >= cxx11
> >&& BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
> >&& CONSTRUCTOR_NELTS (stripped_init) == 1
> >&& ((CLASS_TYPE_P (type) && !CLASSTYPE_NON_AGGREGATE (type))
> >|| VECTOR_TYPE_P (type)))
> > {
> >tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
> >if (reference_related_p (type, TREE_TYPE (elt)))
> > /* We should have fixed this in reshape_init. */
> > gcc_unreachable ();
> > }
> >
> > As the comment suggests, we have code to fix this up in reshape_init_r:
> >
> >/* "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)))
> > {
> >d->cur++;
> >return init;
> > }
> >
> > but in this case this didn't work, because reshape_init_class always creates
> > a fresh CONSTRUCTOR.
>
> But we hit that code before reshape_init_class, why didn't it work?
So originally, when calling reshape_init, we have
{{ TARGET_EXPR<> }} of type D
we recurse on it -- it's a class so we call reshape_init_class. Then we call
reshape_init_r on a field D.2070 for the base (type B), the ctor is {
TARGET_EXPR<> }.
Here we elide the braces, so we return just the TARGET_EXPR as a field_init
for D.2070, but then we're back in reshape_init_class and append the
TARGET_EXPR to new_init constructor:
5969 CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (new_init), field,
field_init);
so we end up with the undesirable form. Hence my thinking we should handle
this at the end of reshape_init_class.
Let me know if this exaplanation isn't sufficient.
> > As of C++17, aggregates can have bases
>
> Does it matter whether BB is a base or a member?
Yes, I couldn't trigger an ICE if it's a member rather than a base.
> > > + B b10 = {{B{42}}};
> > > + B b11 = {{B{{42;
> > > + B b12 = {{B{{{42};
>
> These look ill-formed to me: too many braces around the B value.
>
> Looks like the original testcase had the same problem. So I think this is
> ice-on-invalid.
Are you sure? clang/icc/gcc8/msvc compile it. I thought this was a case of
aggregate initialization, where we have a nested braced-init-list:
http://eel.is/c++draft/dcl.init.aggr#4.2
"If an initializer is itself an initializer list, the element is
list-initialized, which will result in a recursive application of the rules in
this subclause if the element is an aggregate."
I also noticed that since r269045 we accept
struct B { int c; };
struct D : B { };
D d = {{{42}}};
whereas before it was rejected with an error. I dropped these cases from my
test as it's unrelated to this PR.
Marek
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On 3/21/19 4:51 PM, Marek Polacek wrote:
This is a crash in digest_init_r -- we encounter
/* "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." */
if (flag_checking
&& cxx_dialect >= cxx11
&& BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
&& CONSTRUCTOR_NELTS (stripped_init) == 1
&& ((CLASS_TYPE_P (type) && !CLASSTYPE_NON_AGGREGATE (type))
|| VECTOR_TYPE_P (type)))
{
tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
if (reference_related_p (type, TREE_TYPE (elt)))
/* We should have fixed this in reshape_init. */
gcc_unreachable ();
}
As the comment suggests, we have code to fix this up in reshape_init_r:
/* "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)))
{
d->cur++;
return init;
}
but in this case this didn't work, because reshape_init_class always creates
a fresh CONSTRUCTOR.
But we hit that code before reshape_init_class, why didn't it work?
As of C++17, aggregates can have bases
Does it matter whether BB is a base or a member?
+ B b10 = {{B{42}}};
+ B b11 = {{B{{42;
+ B b12 = {{B{{{42};
These look ill-formed to me: too many braces around the B value.
Looks like the original testcase had the same problem. So I think this
is ice-on-invalid.
Jason
C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
This is a crash in digest_init_r -- we encounter
/* "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." */
if (flag_checking
&& cxx_dialect >= cxx11
&& BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
&& CONSTRUCTOR_NELTS (stripped_init) == 1
&& ((CLASS_TYPE_P (type) && !CLASSTYPE_NON_AGGREGATE (type))
|| VECTOR_TYPE_P (type)))
{
tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
if (reference_related_p (type, TREE_TYPE (elt)))
/* We should have fixed this in reshape_init. */
gcc_unreachable ();
}
As the comment suggests, we have code to fix this up in reshape_init_r:
/* "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)))
{
d->cur++;
return init;
}
but in this case this didn't work, because reshape_init_class always creates
a fresh CONSTRUCTOR. As of C++17, aggregates can have bases, so we called
reshape_init_class on the initializer for the public base, and returned a
constructor whose single element was of a reference-related type. We can
check for this case and fix it up, as in the below.
I came up with testcases that test both cases of "where U is T *or* a class
derived from T".
Bootstrapped/regtested on x86_64-linux, ok for trunk/8?
2019-03-21 Marek Polacek
PR c++/89214 - ICE when initializing aggregates with bases.
* decl.c (reshape_init_class): Adjust commentary. If the initializer
list has a single element of the same or derived type, return the
element itself, not wrapped in a constructor.
* g++.dg/cpp1z/aggr-base8.C: New test.
* g++.dg/cpp1z/aggr-base9.C: New test.
diff --git gcc/cp/decl.c gcc/cp/decl.c
index c8435e29491..817e33e40d2 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -5873,7 +5873,7 @@ reshape_init_vector (tree type, reshape_iter *d,
tsubst_flags_t complain)
}
/* Subroutine of reshape_init_r, processes the initializers for classes
- or union. Parameters are the same of reshape_init_r. */
+ or union. Parameters are the same of reshape_init_r. */
static tree
reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,
@@ -5884,7 +5884,8 @@ reshape_init_class (tree type, reshape_iter *d, bool
first_initializer_p,
gcc_assert (CLASS_TYPE_P (type));
- /* The initializer for a class is always a CONSTRUCTOR. */
+ /* The initializer for a class is always a CONSTRUCTOR. Unless it
+ has a single element of the same or derived type, see below. */
new_init = build_constructor (init_list_type_node, NULL);
field = next_initializable_field (TYPE_FIELDS (type));
@@ -5978,6 +5979,19 @@ reshape_init_class (tree type, reshape_iter *d, bool
first_initializer_p,
field = next_initializable_field (DECL_CHAIN (field));
}
+ /* "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." But this function
+ always builds up a new CONSTRUCTOR, so undo that here. */
+ if (cxx_dialect >= cxx11
+ && BRACE_ENCLOSED_INITIALIZER_P (new_init)
+ && CONSTRUCTOR_NELTS (new_init) == 1)
+{
+ tree elt = CONSTRUCTOR_ELT (new_init, 0)->value;
+ if (reference_related_p (type, TREE_TYPE (elt)))
+ return elt;
+}
+
return new_init;
}
diff --git gcc/testsuite/g++.dg/cpp1z/aggr-base8.C
gcc/testsuite/g++.dg/cpp1z/aggr-base8.C
new file mode 100644
index 000..10cff027c39
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1z/aggr-base8.C
@@ -0,0 +1,48 @@
+// PR c++/89214
+// { dg-do compile { target c++17 } }
+
+struct A
+{
+ A (int);
+};
+
+struct BB
+{
+ A a;
+};
+
+struct B : BB
+{
+};
+
+void
+foo ()
+{
+ B b1 = {42};
+ B b2 = {{42}};
+ B b3 = {{{42}}};
+
+ B b4 = B{42};
+ B b5 = B{{42}};
+ B b6 = B{{{42}}};
+
+ B b7 = {B{42}};
+ B b8 = {B{{42}}};
+ B b9 = {B{{{42;
+
+ B b10 = {{B{42}}};
+ B b11 = {{B{{42;
+ B b12 = {{B{{{42};
+
+ B bb1{42};
+ B bb2{{42}};
+ B bb3{{{42}}};
+
+ B bb7{B{42}};
+ B bb8{B{{42}}};
+ B bb9{B{{{42;
+
+ B bb10{{B{42}}};
+ B bb11{{B{{42;
+ B bb12{{B{{{42};
+}
diff --git gcc/testsuite/g++.dg/cpp1z/aggr-base9.C
gcc/testsuite/g++.dg/cpp1z/aggr-base9.C
new file mode 100644
index 000..a3fa747efa4
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1z/aggr-base9.C
@@ -0,0 +1,40 @@
+// PR c++/89214
+// { dg-do compile { target c++17 } }
+
+struct B {
+ int c;
+};
+
+struct D : B { };
+
+void
+foo ()
+{
+ D d1 =
