* [PATCH] c++: direct-init of an array of class type [PR59465] @ 2024-03-02 0:58 Marek Polacek 2024-03-19 19:51 ` Marek Polacek 2024-03-21 1:21 ` Jason Merrill 0 siblings, 2 replies; 9+ messages in thread From: Marek Polacek @ 2024-03-02 0:58 UTC (permalink / raw) To: GCC Patches, Jason Merrill 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: <https://gcc.gnu.org/pipermail/gcc-patches/2011-August/320236.html> 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: <https://gcc.gnu.org/pipermail/gcc-patches/2010-October/297582.html>. 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<tree> *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<tree> &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, diff --git a/gcc/testsuite/g++.dg/init/array62.C b/gcc/testsuite/g++.dg/init/array62.C new file mode 100644 index 00000000000..6d3935d7a66 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/array62.C @@ -0,0 +1,19 @@ +// PR c++/59465 +// { dg-do compile } + +struct string {} a[1]; +struct pair { + string s[1]; + pair() : s(a) {} // { dg-error "array must be initialized" } +}; + +struct S { + char s[10]; + S() : s("aaa") {} +}; + +void +g () +{ + string x[1](a); // { dg-error "array must be initialized" } +} diff --git a/gcc/testsuite/g++.dg/init/array63.C b/gcc/testsuite/g++.dg/init/array63.C new file mode 100644 index 00000000000..96bc9a64b26 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/array63.C @@ -0,0 +1,13 @@ +// PR c++/59465 +// { dg-do compile } + +struct I { + const bool b; +}; +struct O { + I a[2]; + static I const data[2]; + O() : a(data){} // { dg-error "array must be initialized" } +}; + +I const O::data[2] = {true, false}; diff --git a/gcc/testsuite/g++.dg/init/array64.C b/gcc/testsuite/g++.dg/init/array64.C new file mode 100644 index 00000000000..bbdd70c6df8 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/array64.C @@ -0,0 +1,22 @@ +// PR c++/59465 +// { dg-do compile } + +static const int my_size = 10; + +class UserType +{ +public: + UserType(): f_(){} +private: +int f_; +}; + +typedef UserType Array[my_size]; + +class Foo +{ +public: + Foo(Array& m) : m_(m) {}; // { dg-error "array must be initialized" } +private: + Array m_; +}; base-commit: 574fd1f17f100c7c355ad26bc525ab5a3386bb2d -- 2.44.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++: direct-init of an array of class type [PR59465] 2024-03-02 0:58 [PATCH] c++: direct-init of an array of class type [PR59465] Marek Polacek @ 2024-03-19 19:51 ` Marek Polacek 2024-03-21 1:21 ` Jason Merrill 1 sibling, 0 replies; 9+ messages in thread From: Marek Polacek @ 2024-03-19 19:51 UTC (permalink / raw) To: GCC Patches, Jason Merrill Ping. Though I reckon it may be better to defer this to 15. On Fri, Mar 01, 2024 at 07:58:51PM -0500, 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: > <https://gcc.gnu.org/pipermail/gcc-patches/2011-August/320236.html> > 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: > <https://gcc.gnu.org/pipermail/gcc-patches/2010-October/297582.html>. > > 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<tree> *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<tree> &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, > diff --git a/gcc/testsuite/g++.dg/init/array62.C b/gcc/testsuite/g++.dg/init/array62.C > new file mode 100644 > index 00000000000..6d3935d7a66 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/init/array62.C > @@ -0,0 +1,19 @@ > +// PR c++/59465 > +// { dg-do compile } > + > +struct string {} a[1]; > +struct pair { > + string s[1]; > + pair() : s(a) {} // { dg-error "array must be initialized" } > +}; > + > +struct S { > + char s[10]; > + S() : s("aaa") {} > +}; > + > +void > +g () > +{ > + string x[1](a); // { dg-error "array must be initialized" } > +} > diff --git a/gcc/testsuite/g++.dg/init/array63.C b/gcc/testsuite/g++.dg/init/array63.C > new file mode 100644 > index 00000000000..96bc9a64b26 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/init/array63.C > @@ -0,0 +1,13 @@ > +// PR c++/59465 > +// { dg-do compile } > + > +struct I { > + const bool b; > +}; > +struct O { > + I a[2]; > + static I const data[2]; > + O() : a(data){} // { dg-error "array must be initialized" } > +}; > + > +I const O::data[2] = {true, false}; > diff --git a/gcc/testsuite/g++.dg/init/array64.C b/gcc/testsuite/g++.dg/init/array64.C > new file mode 100644 > index 00000000000..bbdd70c6df8 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/init/array64.C > @@ -0,0 +1,22 @@ > +// PR c++/59465 > +// { dg-do compile } > + > +static const int my_size = 10; > + > +class UserType > +{ > +public: > + UserType(): f_(){} > +private: > +int f_; > +}; > + > +typedef UserType Array[my_size]; > + > +class Foo > +{ > +public: > + Foo(Array& m) : m_(m) {}; // { dg-error "array must be initialized" } > +private: > + Array m_; > +}; > > base-commit: 574fd1f17f100c7c355ad26bc525ab5a3386bb2d > -- > 2.44.0 > Marek ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++: direct-init of an array of class type [PR59465] 2024-03-02 0:58 [PATCH] c++: direct-init of an array of class type [PR59465] Marek Polacek 2024-03-19 19:51 ` Marek Polacek @ 2024-03-21 1:21 ` Jason Merrill 2024-03-21 20:48 ` [PATCH v2] " Marek Polacek 1 sibling, 1 reply; 9+ messages in thread From: Jason Merrill @ 2024-03-21 1:21 UTC (permalink / raw) To: Marek Polacek, GCC Patches 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: > <https://gcc.gnu.org/pipermail/gcc-patches/2011-August/320236.html> > 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: > <https://gcc.gnu.org/pipermail/gcc-patches/2010-October/297582.html>. > > 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<tree> *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<tree> &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. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] c++: direct-init of an array of class type [PR59465] 2024-03-21 1:21 ` Jason Merrill @ 2024-03-21 20:48 ` Marek Polacek 2024-03-21 21:28 ` Jason Merrill 0 siblings, 1 reply; 9+ messages in thread From: Marek Polacek @ 2024-03-21 20:48 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches 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: > > <https://gcc.gnu.org/pipermail/gcc-patches/2011-August/320236.html> > > 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: > > <https://gcc.gnu.org/pipermail/gcc-patches/2010-October/297582.html>. > > > > 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<tree> *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<tree> &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: <https://gcc.gnu.org/pipermail/gcc-patches/2011-August/320236.html> 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: <https://gcc.gnu.org/pipermail/gcc-patches/2010-October/297582.html>. 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 ++++++++++++++++++++++++++--- 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, 82 insertions(+), 3 deletions(-) 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..dbd37d47cbf 100644 --- a/gcc/cp/init.cc +++ b/gcc/cp/init.cc @@ -934,6 +934,33 @@ find_uninit_fields (tree *t, hash_set<tree> *uninitialized, tree member) } } +/* Return true if it's OK to initialize an array TYPE 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 type, tree init) +{ + if (!init) + /* Value-init, OK. */ + return true; + if (!same_type_ignoring_top_level_qualifiers_p (type, TREE_TYPE (init))) + return false; + /* 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. */ + 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 @@ -1087,9 +1114,7 @@ perform_member_init (tree member, tree init, hash_set<tree> &uninitialized) { if (TREE_CODE (type) == ARRAY_TYPE) { - if (init == NULL_TREE - || same_type_ignoring_top_level_qualifiers_p (type, - TREE_TYPE (init))) + if (can_init_array_with_p (type, init)) { if (TYPE_DOMAIN (type) && TYPE_MAX_VALUE (TYPE_DOMAIN (type))) { diff --git a/gcc/testsuite/g++.dg/init/array62.C b/gcc/testsuite/g++.dg/init/array62.C new file mode 100644 index 00000000000..2a786a36e4e --- /dev/null +++ b/gcc/testsuite/g++.dg/init/array62.C @@ -0,0 +1,19 @@ +// PR c++/59465 +// { dg-do compile } + +struct string {} a[1]; +struct pair { + string s[1]; + pair() : s(a) {} // { dg-error "invalid initializer for array member" } +}; + +struct S { + char s[10]; + S() : s("aaa") {} +}; + +void +g () +{ + string x[1](a); // { dg-error "array must be initialized" } +} diff --git a/gcc/testsuite/g++.dg/init/array63.C b/gcc/testsuite/g++.dg/init/array63.C new file mode 100644 index 00000000000..57e98056168 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/array63.C @@ -0,0 +1,13 @@ +// PR c++/59465 +// { dg-do compile } + +struct I { + const bool b; +}; +struct O { + I a[2]; + static I const data[2]; + O() : a(data){} // { dg-error "invalid initializer for array member" } +}; + +I const O::data[2] = {true, false}; diff --git a/gcc/testsuite/g++.dg/init/array64.C b/gcc/testsuite/g++.dg/init/array64.C new file mode 100644 index 00000000000..e0afdfab39a --- /dev/null +++ b/gcc/testsuite/g++.dg/init/array64.C @@ -0,0 +1,22 @@ +// PR c++/59465 +// { dg-do compile } + +static const int my_size = 10; + +class UserType +{ +public: + UserType(): f_(){} +private: +int f_; +}; + +typedef UserType Array[my_size]; + +class Foo +{ +public: + Foo(Array& m) : m_(m) {}; // { dg-error "invalid initializer for array member" } +private: + Array m_; +}; base-commit: 509352069d6f166d396f4b4a86e71ea521f2ca78 -- 2.44.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: direct-init of an array of class type [PR59465] 2024-03-21 20:48 ` [PATCH v2] " Marek Polacek @ 2024-03-21 21:28 ` Jason Merrill 2024-03-25 11:36 ` Stephan Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Jason Merrill @ 2024-03-21 21:28 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches 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: >>> <https://gcc.gnu.org/pipermail/gcc-patches/2011-August/320236.html> >>> 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: >>> <https://gcc.gnu.org/pipermail/gcc-patches/2010-October/297582.html>. >>> >>> 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<tree> *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<tree> &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: > <https://gcc.gnu.org/pipermail/gcc-patches/2011-August/320236.html> > 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: > <https://gcc.gnu.org/pipermail/gcc-patches/2010-October/297582.html>. > > 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 ++++++++++++++++++++++++++--- > 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, 82 insertions(+), 3 deletions(-) > 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..dbd37d47cbf 100644 > --- a/gcc/cp/init.cc > +++ b/gcc/cp/init.cc > @@ -934,6 +934,33 @@ find_uninit_fields (tree *t, hash_set<tree> *uninitialized, tree member) > } > } > > +/* Return true if it's OK to initialize an array TYPE 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 type, tree init) > +{ > + if (!init) > + /* Value-init, OK. */ > + return true; > + if (!same_type_ignoring_top_level_qualifiers_p (type, TREE_TYPE (init))) > + return false; > + /* 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. */ > + 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 > @@ -1087,9 +1114,7 @@ perform_member_init (tree member, tree init, hash_set<tree> &uninitialized) > { > if (TREE_CODE (type) == ARRAY_TYPE) > { > - if (init == NULL_TREE > - || same_type_ignoring_top_level_qualifiers_p (type, > - TREE_TYPE (init))) > + if (can_init_array_with_p (type, init)) > { > if (TYPE_DOMAIN (type) && TYPE_MAX_VALUE (TYPE_DOMAIN (type))) > { > diff --git a/gcc/testsuite/g++.dg/init/array62.C b/gcc/testsuite/g++.dg/init/array62.C > new file mode 100644 > index 00000000000..2a786a36e4e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/init/array62.C > @@ -0,0 +1,19 @@ > +// PR c++/59465 > +// { dg-do compile } > + > +struct string {} a[1]; > +struct pair { > + string s[1]; > + pair() : s(a) {} // { dg-error "invalid initializer for array member" } > +}; > + > +struct S { > + char s[10]; > + S() : s("aaa") {} > +}; > + > +void > +g () > +{ > + string x[1](a); // { dg-error "array must be initialized" } > +} > diff --git a/gcc/testsuite/g++.dg/init/array63.C b/gcc/testsuite/g++.dg/init/array63.C > new file mode 100644 > index 00000000000..57e98056168 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/init/array63.C > @@ -0,0 +1,13 @@ > +// PR c++/59465 > +// { dg-do compile } > + > +struct I { > + const bool b; > +}; > +struct O { > + I a[2]; > + static I const data[2]; > + O() : a(data){} // { dg-error "invalid initializer for array member" } > +}; > + > +I const O::data[2] = {true, false}; > diff --git a/gcc/testsuite/g++.dg/init/array64.C b/gcc/testsuite/g++.dg/init/array64.C > new file mode 100644 > index 00000000000..e0afdfab39a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/init/array64.C > @@ -0,0 +1,22 @@ > +// PR c++/59465 > +// { dg-do compile } > + > +static const int my_size = 10; > + > +class UserType > +{ > +public: > + UserType(): f_(){} > +private: > +int f_; > +}; > + > +typedef UserType Array[my_size]; > + > +class Foo > +{ > +public: > + Foo(Array& m) : m_(m) {}; // { dg-error "invalid initializer for array member" } > +private: > + Array m_; > +}; > > base-commit: 509352069d6f166d396f4b4a86e71ea521f2ca78 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: direct-init of an array of class type [PR59465] 2024-03-21 21:28 ` Jason Merrill @ 2024-03-25 11:36 ` Stephan Bergmann 2024-03-25 12:07 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Stephan Bergmann @ 2024-03-25 11:36 UTC (permalink / raw) To: GCC Patches; +Cc: Marek Polacek, Jason Merrill 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() {} > | ^ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: direct-init of an array of class type [PR59465] 2024-03-25 11:36 ` Stephan Bergmann @ 2024-03-25 12:07 ` Jakub Jelinek 2024-03-25 12:39 ` Stephan Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2024-03-25 12:07 UTC (permalink / raw) To: Stephan Bergmann; +Cc: GCC Patches, Marek Polacek, Jason Merrill 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: direct-init of an array of class type [PR59465] 2024-03-25 12:07 ` Jakub Jelinek @ 2024-03-25 12:39 ` Stephan Bergmann 2024-03-25 19:58 ` Marek Polacek 0 siblings, 1 reply; 9+ messages in thread From: Stephan Bergmann @ 2024-03-25 12:39 UTC (permalink / raw) To: GCC Patches; +Cc: Marek Polacek, Jason Merrill, Jakub Jelinek 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: direct-init of an array of class type [PR59465] 2024-03-25 12:39 ` Stephan Bergmann @ 2024-03-25 19:58 ` Marek Polacek 0 siblings, 0 replies; 9+ messages in thread From: Marek Polacek @ 2024-03-25 19:58 UTC (permalink / raw) To: Stephan Bergmann; +Cc: GCC Patches, Jason Merrill, Jakub Jelinek 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-03-25 19:58 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-02 0:58 [PATCH] c++: direct-init of an array of class type [PR59465] Marek Polacek 2024-03-19 19:51 ` Marek Polacek 2024-03-21 1:21 ` Jason Merrill 2024-03-21 20:48 ` [PATCH v2] " Marek Polacek 2024-03-21 21:28 ` Jason Merrill 2024-03-25 11:36 ` Stephan Bergmann 2024-03-25 12:07 ` Jakub Jelinek 2024-03-25 12:39 ` Stephan Bergmann 2024-03-25 19:58 ` Marek Polacek
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).