diff mbox series

c++: Fix array new with value-initialization [PR97523]

Message ID 20201119161130.3747041-1-polacek@redhat.com
State New
Headers show
Series c++: Fix array new with value-initialization [PR97523] | expand

Commit Message

Marek Polacek Nov. 19, 2020, 4:11 p.m. UTC
Since my r11-3092 the following is rejected with -std=c++20:

  struct T { explicit T(); };
  void fn(int n) {
    new T[1]();
  }

with "would use explicit constructor 'T::T()'".  It is because since
that change we go into the P1009 block in build_new (array_p is false,
but nelts is non-null and we're in C++20).  Since we only have (), we
build a {} and continue to build_new_1, which then calls build_vec_init
and then we error because the {} isn't CONSTRUCTOR_IS_DIRECT_INIT.

For (), which is value-initializing, we want to do what we were doing
before: pass empty init and let build_value_init take care of it.

For various reasons I wanted to dig a little bit deeper into this,
and as a result, I'm adding a test for [expr.new]/24 (and checked that
out current behavior matches clang++).

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

gcc/cp/ChangeLog:

	PR c++/97523
	* init.c (build_new): When value-initializing an array new,
	leave the INIT as an empty vector.

gcc/testsuite/ChangeLog:

	PR c++/97523
	* g++.dg/expr/anew5.C: New test.
	* g++.dg/expr/anew6.C: New test.
---
 gcc/cp/init.c                     |  6 +++++-
 gcc/testsuite/g++.dg/expr/anew5.C | 26 ++++++++++++++++++++++++
 gcc/testsuite/g++.dg/expr/anew6.C | 33 +++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/expr/anew5.C
 create mode 100644 gcc/testsuite/g++.dg/expr/anew6.C


base-commit: 2729378d0905a04e476a8bdcaaf0288f417810ec

Comments

Jason Merrill Nov. 19, 2020, 6:43 p.m. UTC | #1
On 11/19/20 11:11 AM, Marek Polacek wrote:
> Since my r11-3092 the following is rejected with -std=c++20:
> 
>    struct T { explicit T(); };
>    void fn(int n) {
>      new T[1]();
>    }
> 
> with "would use explicit constructor 'T::T()'".  It is because since
> that change we go into the P1009 block in build_new (array_p is false,
> but nelts is non-null and we're in C++20).  Since we only have (), we
> build a {} and continue to build_new_1, which then calls build_vec_init
> and then we error because the {} isn't CONSTRUCTOR_IS_DIRECT_INIT.
> 
> For (), which is value-initializing, we want to do what we were doing
> before: pass empty init and let build_value_init take care of it.
> 
> For various reasons I wanted to dig a little bit deeper into this,
> and as a result, I'm adding a test for [expr.new]/24 (and checked that
> out current behavior matches clang++).
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

> gcc/cp/ChangeLog:
> 
> 	PR c++/97523
> 	* init.c (build_new): When value-initializing an array new,
> 	leave the INIT as an empty vector.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/97523
> 	* g++.dg/expr/anew5.C: New test.
> 	* g++.dg/expr/anew6.C: New test.
> ---
>   gcc/cp/init.c                     |  6 +++++-
>   gcc/testsuite/g++.dg/expr/anew5.C | 26 ++++++++++++++++++++++++
>   gcc/testsuite/g++.dg/expr/anew6.C | 33 +++++++++++++++++++++++++++++++
>   3 files changed, 64 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/expr/anew5.C
>   create mode 100644 gcc/testsuite/g++.dg/expr/anew6.C
> 
> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index ffb84ea5b09..0b98f338feb 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -3766,7 +3766,11 @@ build_new (location_t loc, vec<tree, va_gc> **placement, tree type,
>   
>     /* P1009: Array size deduction in new-expressions.  */
>     const bool array_p = TREE_CODE (type) == ARRAY_TYPE;
> -  if (*init && (array_p || (nelts && cxx_dialect >= cxx20)))
> +  if (*init
> +      /* If ARRAY_P, we have to deduce the array bound.  For C++20 paren-init,
> +	 we have to process the parenthesized-list.  But don't do it for (),
> +	 which is value-initialization, and INIT should stay empty.  */
> +      && (array_p || (cxx_dialect >= cxx20 && nelts && !(*init)->is_empty ())))
>       {
>         /* This means we have 'new T[]()'.  */
>         if ((*init)->is_empty ())
> diff --git a/gcc/testsuite/g++.dg/expr/anew5.C b/gcc/testsuite/g++.dg/expr/anew5.C
> new file mode 100644
> index 00000000000..d597caf5483
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/expr/anew5.C
> @@ -0,0 +1,26 @@
> +// PR c++/97523
> +// { dg-do compile }
> +// We were turning the () into {} which made it seem like
> +// aggregate-initialization (we are dealing with arrays here), which
> +// performs copy-initialization, which only accepts converting constructors.
> +
> +struct T {
> +  explicit T();
> +  T(int);
> +};
> +
> +void
> +fn (int n)
> +{
> +  new T[1]();
> +  new T[2]();
> +  new T[3]();
> +  new T[n]();
> +#if __cpp_aggregate_paren_init
> +  new T[]();
> +  new T[2](1, 2);
> +  // T[2] is initialized via copy-initialization, so we can't call
> +  // explicit T().
> +  new T[3](1, 2); // { dg-error "explicit constructor" "" { target c++20 } }
> +#endif
> +}
> diff --git a/gcc/testsuite/g++.dg/expr/anew6.C b/gcc/testsuite/g++.dg/expr/anew6.C
> new file mode 100644
> index 00000000000..0542daac275
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/expr/anew6.C
> @@ -0,0 +1,33 @@
> +// PR c++/97523
> +// { dg-do compile { target c++11 } }
> +
> +// [expr.new]/24: If the new-expression creates an object or an array of
> +// objects of class type, access and ambiguity control are done for the
> +// [...] constructor selected for the initialization (if any).
> +// NB: We only check for a default constructor if the array has a non-constant
> +// bound, or there are insufficient initializers.  Since an array is an
> +// aggregate, we perform aggregate-initialization, which performs
> +// copy-initialization, so we only accept converting constructors.
> +
> +struct T {
> +  explicit T();
> +  T(int);
> +};
> +
> +struct S {
> +  S(int);
> +};
> +
> +void
> +fn (int n)
> +{
> +  new T[1]{}; // { dg-error "explicit constructor" }
> +  new T[2]{1, 2};
> +  new T[3]{1, 2}; // { dg-error "explicit constructor" }
> +  new T[n]{}; // { dg-error "explicit constructor" }
> +
> +  new S[1]{}; // { dg-error "could not convert" }
> +  new S[2]{1, 2};
> +  new S[3]{1, 2}; // { dg-error "could not convert" }
> +  new S[n]{}; // { dg-error "could not convert" }
> +}
> 
> base-commit: 2729378d0905a04e476a8bdcaaf0288f417810ec
>
diff mbox series

Patch

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index ffb84ea5b09..0b98f338feb 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3766,7 +3766,11 @@  build_new (location_t loc, vec<tree, va_gc> **placement, tree type,
 
   /* P1009: Array size deduction in new-expressions.  */
   const bool array_p = TREE_CODE (type) == ARRAY_TYPE;
-  if (*init && (array_p || (nelts && cxx_dialect >= cxx20)))
+  if (*init
+      /* If ARRAY_P, we have to deduce the array bound.  For C++20 paren-init,
+	 we have to process the parenthesized-list.  But don't do it for (),
+	 which is value-initialization, and INIT should stay empty.  */
+      && (array_p || (cxx_dialect >= cxx20 && nelts && !(*init)->is_empty ())))
     {
       /* This means we have 'new T[]()'.  */
       if ((*init)->is_empty ())
diff --git a/gcc/testsuite/g++.dg/expr/anew5.C b/gcc/testsuite/g++.dg/expr/anew5.C
new file mode 100644
index 00000000000..d597caf5483
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/anew5.C
@@ -0,0 +1,26 @@ 
+// PR c++/97523
+// { dg-do compile }
+// We were turning the () into {} which made it seem like
+// aggregate-initialization (we are dealing with arrays here), which
+// performs copy-initialization, which only accepts converting constructors.
+
+struct T {
+  explicit T();
+  T(int);
+};
+
+void
+fn (int n)
+{
+  new T[1]();
+  new T[2]();
+  new T[3]();
+  new T[n]();
+#if __cpp_aggregate_paren_init
+  new T[]();
+  new T[2](1, 2);
+  // T[2] is initialized via copy-initialization, so we can't call
+  // explicit T().
+  new T[3](1, 2); // { dg-error "explicit constructor" "" { target c++20 } }
+#endif
+}
diff --git a/gcc/testsuite/g++.dg/expr/anew6.C b/gcc/testsuite/g++.dg/expr/anew6.C
new file mode 100644
index 00000000000..0542daac275
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/anew6.C
@@ -0,0 +1,33 @@ 
+// PR c++/97523
+// { dg-do compile { target c++11 } }
+
+// [expr.new]/24: If the new-expression creates an object or an array of
+// objects of class type, access and ambiguity control are done for the
+// [...] constructor selected for the initialization (if any).
+// NB: We only check for a default constructor if the array has a non-constant
+// bound, or there are insufficient initializers.  Since an array is an
+// aggregate, we perform aggregate-initialization, which performs
+// copy-initialization, so we only accept converting constructors.
+
+struct T {
+  explicit T();
+  T(int);
+};
+
+struct S {
+  S(int);
+};
+
+void
+fn (int n)
+{
+  new T[1]{}; // { dg-error "explicit constructor" }
+  new T[2]{1, 2};
+  new T[3]{1, 2}; // { dg-error "explicit constructor" }
+  new T[n]{}; // { dg-error "explicit constructor" }
+
+  new S[1]{}; // { dg-error "could not convert" }
+  new S[2]{1, 2};
+  new S[3]{1, 2}; // { dg-error "could not convert" }
+  new S[n]{}; // { dg-error "could not convert" }
+}