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