| Message ID | ZfydGKmoi7TroA96@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] c++: direct-init of an array of class type [PR59465] | expand |
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
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() {} > | ^
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
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
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
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_; +};