Re: [PATCH v2] c++: direct-init of an array of class type [PR59465]
On Mon, Mar 25, 2024 at 01:39:39PM +0100, Stephan Bergmann wrote:
> On 3/25/24 13:07, Jakub Jelinek wrote:
> > On Mon, Mar 25, 2024 at 12:36:46PM +0100, Stephan Bergmann wrote:
> > > This started to break
> > >
> > > > $ cat test.cc
> > > > struct S1 { S1(); };
> > > > struct S2 {
> > > > S2() {}
> > > > S1 a[1] {};
> > > > };
> > >
> > > > $ g++ -fsyntax-only test.cc
> > > > test.cc: In constructor ‘S2::S2()’:
> > > > test.cc:3:10: error: invalid initializer for array member ‘S1 S2::a [1]’
> > > > 3 | S2() {}
> > > >| ^
> >
> > https://gcc.gnu.org/PR114439 ?
>
> yes, sorry, missed that already-existing bugracker issue
I have a patch now, sorry about the breakage. I'm surprised we had no test
covering this :(.
Marek
Re: [PATCH v2] c++: direct-init of an array of class type [PR59465]
On 3/25/24 13:07, Jakub Jelinek wrote:
On Mon, Mar 25, 2024 at 12:36:46PM +0100, Stephan Bergmann wrote:
This started to break
$ cat test.cc
struct S1 { S1(); };
struct S2 {
S2() {}
S1 a[1] {};
};
$ g++ -fsyntax-only test.cc
test.cc: In constructor ‘S2::S2()’:
test.cc:3:10: error: invalid initializer for array member ‘S1 S2::a [1]’
3 | S2() {}
| ^
https://gcc.gnu.org/PR114439 ?
yes, sorry, missed that already-existing bugracker issue
Re: [PATCH v2] c++: direct-init of an array of class type [PR59465]
On Mon, Mar 25, 2024 at 12:36:46PM +0100, Stephan Bergmann wrote:
> On 3/21/24 10:28 PM, Jason Merrill wrote:
> > On 3/21/24 16:48, Marek Polacek wrote:
> > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > OK.
>
> This started to break
>
> > $ cat test.cc
> > struct S1 { S1(); };
> > struct S2 {
> > S2() {}
> > S1 a[1] {};
> > };
>
> > $ g++ -fsyntax-only test.cc
> > test.cc: In constructor ‘S2::S2()’:
> > test.cc:3:10: error: invalid initializer for array member ‘S1 S2::a [1]’
> > 3 | S2() {}
> > | ^
https://gcc.gnu.org/PR114439 ?
Jakub
Re: [PATCH v2] c++: direct-init of an array of class type [PR59465]
On 3/21/24 10:28 PM, Jason Merrill wrote:
On 3/21/24 16:48, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK.
This started to break
$ cat test.cc
struct S1 { S1(); };
struct S2 {
S2() {}
S1 a[1] {};
};
$ g++ -fsyntax-only test.cc
test.cc: In constructor ‘S2::S2()’:
test.cc:3:10: error: invalid initializer for array member ‘S1 S2::a [1]’
3 | S2() {}
| ^
Re: [PATCH v2] c++: direct-init of an array of class type [PR59465]
On 3/21/24 16:48, Marek Polacek wrote:
On Wed, Mar 20, 2024 at 09:21:02PM -0400, Jason Merrill wrote:
On 3/1/24 19:58, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? I don't
claim that this has to go to 14 though.
-- >8 --
...from another array in a mem-initializer should not be accepted.
We already reject
struct string {} a[1];
string x[1](a);
but
struct pair {
string s[1];
pair() : s(a) {}
};
is wrongly accepted.
It started to be accepted with r0-110915-ga034826198b771:
which was supposed to be a cleanup, not a deliberate change to start
accepting the code. The build_vec_init_expr code was added in r165976:
.
It appears that we do the magic copy array when we have a defaulted
constructor and we generate code for its mem-initializer which
initializes an array. I also see that we go that path for compound
literals. So when initializing an array member, we can limit building
up a VEC_INIT_EXPR to those special cases.
PR c++/59465
gcc/cp/ChangeLog:
* init.cc (can_init_array_with_p): New.
(perform_member_init): Check it.
gcc/testsuite/ChangeLog:
* g++.dg/init/array62.C: New test.
* g++.dg/init/array63.C: New test.
* g++.dg/init/array64.C: New test.
---
gcc/cp/init.cc | 27 ++-
gcc/testsuite/g++.dg/init/array62.C | 19 +++
gcc/testsuite/g++.dg/init/array63.C | 13 +
gcc/testsuite/g++.dg/init/array64.C | 22 ++
4 files changed, 80 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/init/array62.C
create mode 100644 gcc/testsuite/g++.dg/init/array63.C
create mode 100644 gcc/testsuite/g++.dg/init/array64.C
diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index d2586fad86b..fb8c0e521fb 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -934,6 +934,31 @@ find_uninit_fields (tree *t, hash_set
*uninitialized, tree member)
}
}
+/* Return true if it's OK to initialize an array from INIT. Mere mortals
+ can't copy arrays, but the compiler can do so with a VEC_INIT_EXPR in
+ certain cases. */
+
+static bool
+can_init_array_with_p (tree init)
+{
+ if (!init)
+return true;
+
+ /* We're called from synthesize_method, and we're processing the
+ mem-initializers of a constructor. */
+ if (DECL_DEFAULTED_FN (current_function_decl))
+return true;
+ /* As an extension, we allow copying from a compound literal. */
+ else if (TREE_CODE (init) == TARGET_EXPR)
+{
+ init = TARGET_EXPR_INITIAL (init);
+ if (TREE_CODE (init) == CONSTRUCTOR)
+ return CONSTRUCTOR_C99_COMPOUND_LITERAL (init);
+}
+
+ return false;
+}
+
/* Initialize MEMBER, a FIELD_DECL, with INIT, a TREE_LIST of
arguments. If TREE_LIST is void_type_node, an empty initializer
list was given; if NULL_TREE no initializer was given. UNINITIALIZED
@@ -1085,7 +1110,7 @@ perform_member_init (tree member, tree init, hash_set
&uninitialized)
else if (type_build_ctor_call (type)
|| (init && CLASS_TYPE_P (strip_array_types (type
{
- if (TREE_CODE (type) == ARRAY_TYPE)
+ if (TREE_CODE (type) == ARRAY_TYPE && can_init_array_with_p (init))
{
if (init == NULL_TREE
|| same_type_ignoring_top_level_qualifiers_p (type,
It seems like these last two existing lines also fall under "init is
suitable to initialize type", so let's fold them into the new function.
Sounds good. Here it is:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK.
-- >8 --
...from another array in a mem-initializer should not be accepted.
We already reject
struct string {} a[1];
string x[1](a);
but
struct pair {
string s[1];
pair() : s(a) {}
};
is wrongly accepted.
It started to be accepted with r0-110915-ga034826198b771:
which was supposed to be a cleanup, not a deliberate change to start
accepting the code. The build_vec_init_expr code was added in r165976:
.
It appears that we do the magic copy array when we have a defaulted
constructor and we generate code for its mem-initializer which
initializes an array. I also see that we go that path for compound
literals. So when initializing an array member, we can limit building
up a VEC_INIT_EXPR to those special cases.
PR c++/59465
gcc/cp/ChangeLog:
* init.cc (can_init_array_with_p): New.
(perform_member_init): Check it.
gcc/testsuite/ChangeLog:
* g++.dg/init/array62.C: New test.
* g++.dg/init/array63.C: New test.
* g++.dg/init/array64.C: New test.
---
gcc/cp/init.cc | 31 +++
[PATCH v2] c++: direct-init of an array of class type [PR59465]
On Wed, Mar 20, 2024 at 09:21:02PM -0400, Jason Merrill wrote:
> On 3/1/24 19:58, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? I don't
> > claim that this has to go to 14 though.
> >
> > -- >8 --
> > ...from another array in a mem-initializer should not be accepted.
> >
> > We already reject
> >
> >struct string {} a[1];
> >string x[1](a);
> >
> > but
> >
> >struct pair {
> > string s[1];
> > pair() : s(a) {}
> >};
> >
> > is wrongly accepted.
> >
> > It started to be accepted with r0-110915-ga034826198b771:
> >
> > which was supposed to be a cleanup, not a deliberate change to start
> > accepting the code. The build_vec_init_expr code was added in r165976:
> > .
> >
> > It appears that we do the magic copy array when we have a defaulted
> > constructor and we generate code for its mem-initializer which
> > initializes an array. I also see that we go that path for compound
> > literals. So when initializing an array member, we can limit building
> > up a VEC_INIT_EXPR to those special cases.
> >
> > PR c++/59465
> >
> > gcc/cp/ChangeLog:
> >
> > * init.cc (can_init_array_with_p): New.
> > (perform_member_init): Check it.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/init/array62.C: New test.
> > * g++.dg/init/array63.C: New test.
> > * g++.dg/init/array64.C: New test.
> > ---
> > gcc/cp/init.cc | 27 ++-
> > gcc/testsuite/g++.dg/init/array62.C | 19 +++
> > gcc/testsuite/g++.dg/init/array63.C | 13 +
> > gcc/testsuite/g++.dg/init/array64.C | 22 ++
> > 4 files changed, 80 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/init/array62.C
> > create mode 100644 gcc/testsuite/g++.dg/init/array63.C
> > create mode 100644 gcc/testsuite/g++.dg/init/array64.C
> >
> > diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> > index d2586fad86b..fb8c0e521fb 100644
> > --- a/gcc/cp/init.cc
> > +++ b/gcc/cp/init.cc
> > @@ -934,6 +934,31 @@ find_uninit_fields (tree *t, hash_set
> > *uninitialized, tree member)
> > }
> > }
> > +/* Return true if it's OK to initialize an array from INIT. Mere mortals
> > + can't copy arrays, but the compiler can do so with a VEC_INIT_EXPR in
> > + certain cases. */
> > +
> > +static bool
> > +can_init_array_with_p (tree init)
> > +{
> > + if (!init)
> > +return true;
> > +
> > + /* We're called from synthesize_method, and we're processing the
> > + mem-initializers of a constructor. */
> > + if (DECL_DEFAULTED_FN (current_function_decl))
> > +return true;
> > + /* As an extension, we allow copying from a compound literal. */
> > + else if (TREE_CODE (init) == TARGET_EXPR)
> > +{
> > + init = TARGET_EXPR_INITIAL (init);
> > + if (TREE_CODE (init) == CONSTRUCTOR)
> > + return CONSTRUCTOR_C99_COMPOUND_LITERAL (init);
> > +}
> > +
> > + return false;
> > +}
> > +
> > /* Initialize MEMBER, a FIELD_DECL, with INIT, a TREE_LIST of
> > arguments. If TREE_LIST is void_type_node, an empty initializer
> > list was given; if NULL_TREE no initializer was given. UNINITIALIZED
> > @@ -1085,7 +1110,7 @@ perform_member_init (tree member, tree init,
> > hash_set &uninitialized)
> > else if (type_build_ctor_call (type)
> >|| (init && CLASS_TYPE_P (strip_array_types (type
> > {
> > - if (TREE_CODE (type) == ARRAY_TYPE)
> > + if (TREE_CODE (type) == ARRAY_TYPE && can_init_array_with_p (init))
> > {
> > if (init == NULL_TREE
> > || same_type_ignoring_top_level_qualifiers_p (type,
>
> It seems like these last two existing lines also fall under "init is
> suitable to initialize type", so let's fold them into the new function.
Sounds good. Here it is:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
...from another array in a mem-initializer should not be accepted.
We already reject
struct string {} a[1];
string x[1](a);
but
struct pair {
string s[1];
pair() : s(a) {}
};
is wrongly accepted.
It started to be accepted with r0-110915-ga034826198b771:
which was supposed to be a cleanup, not a deliberate change to start
accepting the code. The build_vec_init_expr code was added in r165976:
.
It appears that we do the magic copy array when we have a defaulted
constructor and we generate code for its mem-initializer which
initializes an array. I also see that we go that path for compound
literals. So when initializing an array member, we can limit building
up a VEC_INIT_EXPR to those special cases.
PR c++/59465
gcc/cp/ChangeLog:
* init
