diff mbox series

c++: direct-init of an array of class type [PR59465]

Message ID 20240302005851.924184-1-polacek@redhat.com
State New
Headers show
Series c++: direct-init of an array of class type [PR59465] | expand

Commit Message

Marek Polacek March 2, 2024, 12:58 a.m. UTC 
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


base-commit: 574fd1f17f100c7c355ad26bc525ab5a3386bb2d

Comments

Marek Polacek March 19, 2024, 7:51 p.m. UTC | #1 
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
Jason Merrill March 21, 2024, 1:21 a.m. UTC | #2 
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 mbox series

Patch

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_;
+};