diff mbox series

c++: array new with value-initialization, again [PR115645]

Message ID 20240717213349.460422-1-polacek@redhat.com
State New
Headers show
Series c++: array new with value-initialization, again [PR115645] | expand

Commit Message

Marek Polacek July 17, 2024, 9:33 p.m. UTC 
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Unfortunately, my r15-1946 fix broke the attached testcase.  In it,
we no longer go into the
  /* P1009: Array size deduction in new-expressions.  */
block, and instead generate an operator new [] call along with a loop
in build_new_1, which we can't constexpr-evaluate.  So this patch
reverts r15-1946 and uses CONSTRUCTOR_IS_PAREN_INIT to distinguish
between () and {} to fix the original testcase (anew7.C).

	PR c++/115645

gcc/cp/ChangeLog:

	* call.cc (convert_like_internal) <case ck_user>: Don't report errors
	about calling an explicit constructor when the constructor was marked
	CONSTRUCTOR_IS_PAREN_INIT.
	* init.cc (build_new): Revert r15-1946.  Set CONSTRUCTOR_IS_PAREN_INIT.
	(build_vec_init): Maybe set CONSTRUCTOR_IS_PAREN_INIT.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/constexpr-new23.C: New test.
---
 gcc/cp/call.cc                               |  2 ++
 gcc/cp/init.cc                               | 17 ++++-----
 gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C | 38 ++++++++++++++++++++
 3 files changed, 49 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C


base-commit: 43a7ece873eba47a11c0b21b0068eee53740551a

Comments

Jason Merrill July 23, 2024, 9:18 p.m. UTC | #1 
On 7/17/24 5:33 PM, Marek Polacek wrote:
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

Hmm, I thought I had replied to this already.

> -- >8 --
> Unfortunately, my r15-1946 fix broke the attached testcase.  In it,
> we no longer go into the
>    /* P1009: Array size deduction in new-expressions.  */
> block, and instead generate an operator new [] call along with a loop
> in build_new_1, which we can't constexpr-evaluate.  So this patch
> reverts r15-1946 and uses CONSTRUCTOR_IS_PAREN_INIT to distinguish
> between () and {} to fix the original testcase (anew7.C).
> 
> 	PR c++/115645
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (convert_like_internal) <case ck_user>: Don't report errors
> 	about calling an explicit constructor when the constructor was marked
> 	CONSTRUCTOR_IS_PAREN_INIT.
> 	* init.cc (build_new): Revert r15-1946.  Set CONSTRUCTOR_IS_PAREN_INIT.
> 	(build_vec_init): Maybe set CONSTRUCTOR_IS_PAREN_INIT.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/constexpr-new23.C: New test.
> ---
>   gcc/cp/call.cc                               |  2 ++
>   gcc/cp/init.cc                               | 17 ++++-----
>   gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C | 38 ++++++++++++++++++++
>   3 files changed, 49 insertions(+), 8 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index a5d3426b70c..2d94d5e0d07 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -8592,6 +8592,8 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
>   	    && BRACE_ENCLOSED_INITIALIZER_P (expr)
>   	    /* Unless this is for direct-list-initialization.  */
>   	    && (!CONSTRUCTOR_IS_DIRECT_INIT (expr) || convs->need_temporary_p)
> +	    /* And it wasn't a ()-init.  */
> +	    && !CONSTRUCTOR_IS_PAREN_INIT (expr)
>   	    /* And in C++98 a default constructor can't be explicit.  */
>   	    && cxx_dialect >= cxx11)
>   	  {
> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> index e9561c146d7..4138a6077dd 100644
> --- a/gcc/cp/init.cc
> +++ b/gcc/cp/init.cc
> @@ -4005,20 +4005,17 @@ 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
> -      /* If the array didn't specify its bound, we have to deduce it.  */
> -      && ((array_p && !TYPE_DOMAIN (type))
> -	  /* For C++20 array with parenthesized-init, we have to process
> -	     the parenthesized-list.  But don't do it for (), which is
> -	     value-initialization, and INIT should stay empty.  */
> -	  || (cxx_dialect >= cxx20
> -	      && (array_p || nelts)
> -	      && !(*init)->is_empty ())))
> +      /* 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 ())
>   	{
>   	  tree ctor = build_constructor (init_list_type_node, NULL);
>   	  CONSTRUCTOR_IS_DIRECT_INIT (ctor) = true;
> +	  CONSTRUCTOR_IS_PAREN_INIT (ctor) = true;
>   	  vec_safe_push (*init, ctor);
>   	}
>         tree &elt = (**init)[0];
> @@ -4735,6 +4732,9 @@ build_vec_init (tree base, tree maxindex, tree init,
>     bool do_static_init = (DECL_P (obase) && TREE_STATIC (obase));
>   
>     bool empty_list = false;
> +  const bool paren_init_p = (init
> +			     && TREE_CODE (init) == CONSTRUCTOR
> +			     && CONSTRUCTOR_IS_PAREN_INIT (init));

I think rather than recognizing paren-init in general, we want to 
recognize () specifically, and set explicit_value_init_p...

>     if (init && BRACE_ENCLOSED_INITIALIZER_P (init)
>         && CONSTRUCTOR_NELTS (init) == 0)
>       /* Skip over the handling of non-empty init lists.  */
> @@ -4927,6 +4927,7 @@ build_vec_init (tree base, tree maxindex, tree init,
>   		  || TREE_CODE (type) == ARRAY_TYPE))
>   	    {
>   	      init = build_constructor (init_list_type_node, NULL);
> +	      CONSTRUCTOR_IS_PAREN_INIT (init) = paren_init_p;
>   	    }
>   	  else
>   	    {

...by taking the else branch here.  Then we shouldn't need the 
convert_like change.

Jason
Marek Polacek July 29, 2024, 8:18 p.m. UTC | #2 
On Tue, Jul 23, 2024 at 05:18:52PM -0400, Jason Merrill wrote:
> On 7/17/24 5:33 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> Hmm, I thought I had replied to this already.
> 
> > -- >8 --
> > Unfortunately, my r15-1946 fix broke the attached testcase.  In it,
> > we no longer go into the
> >    /* P1009: Array size deduction in new-expressions.  */
> > block, and instead generate an operator new [] call along with a loop
> > in build_new_1, which we can't constexpr-evaluate.  So this patch
> > reverts r15-1946 and uses CONSTRUCTOR_IS_PAREN_INIT to distinguish
> > between () and {} to fix the original testcase (anew7.C).
> > 
> > 	PR c++/115645
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* call.cc (convert_like_internal) <case ck_user>: Don't report errors
> > 	about calling an explicit constructor when the constructor was marked
> > 	CONSTRUCTOR_IS_PAREN_INIT.
> > 	* init.cc (build_new): Revert r15-1946.  Set CONSTRUCTOR_IS_PAREN_INIT.
> > 	(build_vec_init): Maybe set CONSTRUCTOR_IS_PAREN_INIT.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/constexpr-new23.C: New test.
> > ---
> >   gcc/cp/call.cc                               |  2 ++
> >   gcc/cp/init.cc                               | 17 ++++-----
> >   gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C | 38 ++++++++++++++++++++
> >   3 files changed, 49 insertions(+), 8 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C
> > 
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index a5d3426b70c..2d94d5e0d07 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -8592,6 +8592,8 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
> >   	    && BRACE_ENCLOSED_INITIALIZER_P (expr)
> >   	    /* Unless this is for direct-list-initialization.  */
> >   	    && (!CONSTRUCTOR_IS_DIRECT_INIT (expr) || convs->need_temporary_p)
> > +	    /* And it wasn't a ()-init.  */
> > +	    && !CONSTRUCTOR_IS_PAREN_INIT (expr)
> >   	    /* And in C++98 a default constructor can't be explicit.  */
> >   	    && cxx_dialect >= cxx11)
> >   	  {
> > diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> > index e9561c146d7..4138a6077dd 100644
> > --- a/gcc/cp/init.cc
> > +++ b/gcc/cp/init.cc
> > @@ -4005,20 +4005,17 @@ 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
> > -      /* If the array didn't specify its bound, we have to deduce it.  */
> > -      && ((array_p && !TYPE_DOMAIN (type))
> > -	  /* For C++20 array with parenthesized-init, we have to process
> > -	     the parenthesized-list.  But don't do it for (), which is
> > -	     value-initialization, and INIT should stay empty.  */
> > -	  || (cxx_dialect >= cxx20
> > -	      && (array_p || nelts)
> > -	      && !(*init)->is_empty ())))
> > +      /* 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 ())
> >   	{
> >   	  tree ctor = build_constructor (init_list_type_node, NULL);
> >   	  CONSTRUCTOR_IS_DIRECT_INIT (ctor) = true;
> > +	  CONSTRUCTOR_IS_PAREN_INIT (ctor) = true;
> >   	  vec_safe_push (*init, ctor);
> >   	}
> >         tree &elt = (**init)[0];
> > @@ -4735,6 +4732,9 @@ build_vec_init (tree base, tree maxindex, tree init,
> >     bool do_static_init = (DECL_P (obase) && TREE_STATIC (obase));
> >     bool empty_list = false;
> > +  const bool paren_init_p = (init
> > +			     && TREE_CODE (init) == CONSTRUCTOR
> > +			     && CONSTRUCTOR_IS_PAREN_INIT (init));
> 
> I think rather than recognizing paren-init in general, we want to recognize
> () specifically, and set explicit_value_init_p...
> 
> >     if (init && BRACE_ENCLOSED_INITIALIZER_P (init)
> >         && CONSTRUCTOR_NELTS (init) == 0)
> >       /* Skip over the handling of non-empty init lists.  */
> > @@ -4927,6 +4927,7 @@ build_vec_init (tree base, tree maxindex, tree init,
> >   		  || TREE_CODE (type) == ARRAY_TYPE))
> >   	    {
> >   	      init = build_constructor (init_list_type_node, NULL);
> > +	      CONSTRUCTOR_IS_PAREN_INIT (init) = paren_init_p;
> >   	    }
> >   	  else
> >   	    {
> 
> ...by taking the else branch here.  Then we shouldn't need the convert_like
> change.

Unfortunately that then breaks Jon's test (constexpr-new23.C which this
patch is adding).  The problem is that if we do *not* create a new {}, and
do explicit_value_init_p, we end up with

  int[1] * D.2643;
  <<< Unknown tree: expr_stmt
    (void) (D.2643 = (int[1] *) D.2642) >>>;
  int[1] * D.2644;
  <<< Unknown tree: expr_stmt
    (void) (D.2644 = D.2643) >>>;
  TARGET_EXPR <D.2645, 0>;
  <<< Unknown tree: for_stmt
    
    D.2645 > -1
    
    <<cleanup_point <<< Unknown tree: expr_stmt
      *(int[1] &)     int * D.2646;
      <<< Unknown tree: expr_stmt
	(void) (D.2646 = (int *) D.2644) >>>;
	  int * D.2647;
      <<< Unknown tree: expr_stmt
	(void) (D.2647 = D.2646) >>>;
      TARGET_EXPR <D.2648, 0>;
      <<< Unknown tree: for_stmt
	
	D.2648 > -1
	
	<<cleanup_point <<< Unknown tree: expr_stmt
	  *D.2647 = 0,  --D.2648 >>>>>;
	<<< Unknown tree: expr_stmt
	  (void)  ++D.2647 >>>;
	 >>>;
      D.2646,  --D.2645 >>>>>;
    <<< Unknown tree: expr_stmt
      (void)  ++D.2644 >>>;
     >>>;
  D.2643

rather than:

  int[1] * D.2643;
  <<< Unknown tree: expr_stmt
    (void) (D.2643 = (int[1] *) D.2642) >>>;
  int[1] * D.2644;
  <<< Unknown tree: expr_stmt
    (void) (D.2644 = D.2643) >>>;
  TARGET_EXPR <D.2645, 0>;
  <<< Unknown tree: for_stmt
    
    D.2645 > -1
    
    <<cleanup_point <<< Unknown tree: expr_stmt
      *D.2644 = {},  --D.2645 >>>>>;
    <<< Unknown tree: expr_stmt
      (void)  ++D.2644 >>>;
     >>>;
  D.2643

In the former, the "*D.2647 = 0" assignment is what breaks constexpr,
which then complains: 

constexpr-new23.C:16:16: error: accessing 'test_array()::U::arr' member instead of initialized 'test_array()::U::x' member in constant expression
   16 |         return ::new((void*)p) T[1]();
      |                ^~~~~~~~~~~~~~~~~~~~~~
constexpr-new23.C:31:9: note: initializing 'test_array()::U::arr' requires a member access expression as the left operand of the assignment
   31 |     int arr[4];


If there is no bug in constexpr, then it looks like we need to
initialize using a {} rather than a loop that assigns 0 to each
element.

Marek
Jason Merrill July 29, 2024, 10:34 p.m. UTC | #3 
On 7/29/24 4:18 PM, Marek Polacek wrote:
> On Tue, Jul 23, 2024 at 05:18:52PM -0400, Jason Merrill wrote:
>> On 7/17/24 5:33 PM, Marek Polacek wrote:
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>
>> Hmm, I thought I had replied to this already.
>>
>>> -- >8 --
>>> Unfortunately, my r15-1946 fix broke the attached testcase.  In it,
>>> we no longer go into the
>>>     /* P1009: Array size deduction in new-expressions.  */
>>> block, and instead generate an operator new [] call along with a loop
>>> in build_new_1, which we can't constexpr-evaluate.  So this patch
>>> reverts r15-1946 and uses CONSTRUCTOR_IS_PAREN_INIT to distinguish
>>> between () and {} to fix the original testcase (anew7.C).
>>>
>>> 	PR c++/115645
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* call.cc (convert_like_internal) <case ck_user>: Don't report errors
>>> 	about calling an explicit constructor when the constructor was marked
>>> 	CONSTRUCTOR_IS_PAREN_INIT.
>>> 	* init.cc (build_new): Revert r15-1946.  Set CONSTRUCTOR_IS_PAREN_INIT.
>>> 	(build_vec_init): Maybe set CONSTRUCTOR_IS_PAREN_INIT.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp2a/constexpr-new23.C: New test.
>>> ---
>>>    gcc/cp/call.cc                               |  2 ++
>>>    gcc/cp/init.cc                               | 17 ++++-----
>>>    gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C | 38 ++++++++++++++++++++
>>>    3 files changed, 49 insertions(+), 8 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C
>>>
>>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>>> index a5d3426b70c..2d94d5e0d07 100644
>>> --- a/gcc/cp/call.cc
>>> +++ b/gcc/cp/call.cc
>>> @@ -8592,6 +8592,8 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
>>>    	    && BRACE_ENCLOSED_INITIALIZER_P (expr)
>>>    	    /* Unless this is for direct-list-initialization.  */
>>>    	    && (!CONSTRUCTOR_IS_DIRECT_INIT (expr) || convs->need_temporary_p)
>>> +	    /* And it wasn't a ()-init.  */
>>> +	    && !CONSTRUCTOR_IS_PAREN_INIT (expr)
>>>    	    /* And in C++98 a default constructor can't be explicit.  */
>>>    	    && cxx_dialect >= cxx11)
>>>    	  {
>>> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
>>> index e9561c146d7..4138a6077dd 100644
>>> --- a/gcc/cp/init.cc
>>> +++ b/gcc/cp/init.cc
>>> @@ -4005,20 +4005,17 @@ 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
>>> -      /* If the array didn't specify its bound, we have to deduce it.  */
>>> -      && ((array_p && !TYPE_DOMAIN (type))
>>> -	  /* For C++20 array with parenthesized-init, we have to process
>>> -	     the parenthesized-list.  But don't do it for (), which is
>>> -	     value-initialization, and INIT should stay empty.  */
>>> -	  || (cxx_dialect >= cxx20
>>> -	      && (array_p || nelts)
>>> -	      && !(*init)->is_empty ())))
>>> +      /* 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 ())
>>>    	{
>>>    	  tree ctor = build_constructor (init_list_type_node, NULL);
>>>    	  CONSTRUCTOR_IS_DIRECT_INIT (ctor) = true;
>>> +	  CONSTRUCTOR_IS_PAREN_INIT (ctor) = true;
>>>    	  vec_safe_push (*init, ctor);
>>>    	}
>>>          tree &elt = (**init)[0];
>>> @@ -4735,6 +4732,9 @@ build_vec_init (tree base, tree maxindex, tree init,
>>>      bool do_static_init = (DECL_P (obase) && TREE_STATIC (obase));
>>>      bool empty_list = false;
>>> +  const bool paren_init_p = (init
>>> +			     && TREE_CODE (init) == CONSTRUCTOR
>>> +			     && CONSTRUCTOR_IS_PAREN_INIT (init));
>>
>> I think rather than recognizing paren-init in general, we want to recognize
>> () specifically, and set explicit_value_init_p...
>>
>>>      if (init && BRACE_ENCLOSED_INITIALIZER_P (init)
>>>          && CONSTRUCTOR_NELTS (init) == 0)
>>>        /* Skip over the handling of non-empty init lists.  */
>>> @@ -4927,6 +4927,7 @@ build_vec_init (tree base, tree maxindex, tree init,
>>>    		  || TREE_CODE (type) == ARRAY_TYPE))
>>>    	    {
>>>    	      init = build_constructor (init_list_type_node, NULL);
>>> +	      CONSTRUCTOR_IS_PAREN_INIT (init) = paren_init_p;
>>>    	    }
>>>    	  else
>>>    	    {
>>
>> ...by taking the else branch here.  Then we shouldn't need the convert_like
>> change.
> 
> Unfortunately that then breaks Jon's test (constexpr-new23.C which this
> patch is adding).  The problem is that if we do *not* create a new {}, and
> do explicit_value_init_p, we end up with
> 
>    int[1] * D.2643;
>    <<< Unknown tree: expr_stmt
>      (void) (D.2643 = (int[1] *) D.2642) >>>;
>    int[1] * D.2644;
>    <<< Unknown tree: expr_stmt
>      (void) (D.2644 = D.2643) >>>;
>    TARGET_EXPR <D.2645, 0>;
>    <<< Unknown tree: for_stmt
>      
>      D.2645 > -1
>      
>      <<cleanup_point <<< Unknown tree: expr_stmt
>        *(int[1] &)     int * D.2646;
>        <<< Unknown tree: expr_stmt
> 	(void) (D.2646 = (int *) D.2644) >>>;
> 	  int * D.2647;
>        <<< Unknown tree: expr_stmt
> 	(void) (D.2647 = D.2646) >>>;
>        TARGET_EXPR <D.2648, 0>;
>        <<< Unknown tree: for_stmt
> 	
> 	D.2648 > -1
> 	
> 	<<cleanup_point <<< Unknown tree: expr_stmt
> 	  *D.2647 = 0,  --D.2648 >>>>>;
> 	<<< Unknown tree: expr_stmt
> 	  (void)  ++D.2647 >>>;
> 	 >>>;
>        D.2646,  --D.2645 >>>>>;
>      <<< Unknown tree: expr_stmt
>        (void)  ++D.2644 >>>;
>       >>>;
>    D.2643
> 
> rather than:
> 
>    int[1] * D.2643;
>    <<< Unknown tree: expr_stmt
>      (void) (D.2643 = (int[1] *) D.2642) >>>;
>    int[1] * D.2644;
>    <<< Unknown tree: expr_stmt
>      (void) (D.2644 = D.2643) >>>;
>    TARGET_EXPR <D.2645, 0>;
>    <<< Unknown tree: for_stmt
>      
>      D.2645 > -1
>      
>      <<cleanup_point <<< Unknown tree: expr_stmt
>        *D.2644 = {},  --D.2645 >>>>>;
>      <<< Unknown tree: expr_stmt
>        (void)  ++D.2644 >>>;
>       >>>;
>    D.2643
> 
> In the former, the "*D.2647 = 0" assignment is what breaks constexpr,
> which then complains:
> 
> constexpr-new23.C:16:16: error: accessing 'test_array()::U::arr' member instead of initialized 'test_array()::U::x' member in constant expression
>     16 |         return ::new((void*)p) T[1]();
>        |                ^~~~~~~~~~~~~~~~~~~~~~
> constexpr-new23.C:31:9: note: initializing 'test_array()::U::arr' requires a member access expression as the left operand of the assignment
>     31 |     int arr[4];
> 
> 
> If there is no bug in constexpr, then it looks like we need to
> initialize using a {} rather than a loop that assigns 0 to each
> element.

Ah, thanks.

It looks like the first bug is that build_vec_init wrongly leaves 
try_const false for this case (without your patch) because int doesn't 
have a constexpr default constructor, failing to consider that 
value-initialization of scalars is constexpr.

Then, once we're into the looping initialization, we aren't expressing 
it in a way that will satisfy the strict checking in constexpr 
evaluation; it needs to initialize the array, not just its elements.

I expect we could fix that with something like

>       /* Start array lifetime before accessing elements.  */
>       if (TREE_CODE (atype) == ARRAY_TYPE)
>         {
>           elt_init = build_constructor (atype, nullptr);
>           CONSTRUCTOR_NO_CLEARING (elt_init) = true;
>           for_stmt = build2 (INIT_EXPR, atype, obase, elt_init);
>           finish_expr_stmt (for_stmt);
>         }

but if we're only concerned about constexpr, fixing the first bug ought 
to be enough; in constant evaluation if we don't get a constant 
initializer we're failing anyway.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index a5d3426b70c..2d94d5e0d07 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -8592,6 +8592,8 @@  convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
 	    && BRACE_ENCLOSED_INITIALIZER_P (expr)
 	    /* Unless this is for direct-list-initialization.  */
 	    && (!CONSTRUCTOR_IS_DIRECT_INIT (expr) || convs->need_temporary_p)
+	    /* And it wasn't a ()-init.  */
+	    && !CONSTRUCTOR_IS_PAREN_INIT (expr)
 	    /* And in C++98 a default constructor can't be explicit.  */
 	    && cxx_dialect >= cxx11)
 	  {
diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index e9561c146d7..4138a6077dd 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -4005,20 +4005,17 @@  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
-      /* If the array didn't specify its bound, we have to deduce it.  */
-      && ((array_p && !TYPE_DOMAIN (type))
-	  /* For C++20 array with parenthesized-init, we have to process
-	     the parenthesized-list.  But don't do it for (), which is
-	     value-initialization, and INIT should stay empty.  */
-	  || (cxx_dialect >= cxx20
-	      && (array_p || nelts)
-	      && !(*init)->is_empty ())))
+      /* 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 ())
 	{
 	  tree ctor = build_constructor (init_list_type_node, NULL);
 	  CONSTRUCTOR_IS_DIRECT_INIT (ctor) = true;
+	  CONSTRUCTOR_IS_PAREN_INIT (ctor) = true;
 	  vec_safe_push (*init, ctor);
 	}
       tree &elt = (**init)[0];
@@ -4735,6 +4732,9 @@  build_vec_init (tree base, tree maxindex, tree init,
   bool do_static_init = (DECL_P (obase) && TREE_STATIC (obase));
 
   bool empty_list = false;
+  const bool paren_init_p = (init
+			     && TREE_CODE (init) == CONSTRUCTOR
+			     && CONSTRUCTOR_IS_PAREN_INIT (init));
   if (init && BRACE_ENCLOSED_INITIALIZER_P (init)
       && CONSTRUCTOR_NELTS (init) == 0)
     /* Skip over the handling of non-empty init lists.  */
@@ -4927,6 +4927,7 @@  build_vec_init (tree base, tree maxindex, tree init,
 		  || TREE_CODE (type) == ARRAY_TYPE))
 	    {
 	      init = build_constructor (init_list_type_node, NULL);
+	      CONSTRUCTOR_IS_PAREN_INIT (init) = paren_init_p;
 	    }
 	  else
 	    {
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C
new file mode 100644
index 00000000000..1abbef18fae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new23.C
@@ -0,0 +1,38 @@ 
+// PR c++/115645
+// { dg-do compile { target c++20 } }
+
+using size_t = decltype(sizeof(0));
+
+void* operator new(size_t, void* p) { return p; }
+void* operator new[](size_t, void* p) { return p; }
+
+#define VERIFY(C) if (!(C)) throw
+
+namespace std {
+  template<typename T>
+    constexpr T* construct_at(T* p)
+    {
+      if constexpr (__is_array(T))
+        return ::new((void*)p) T[1]();
+      else
+        return ::new((void*)p) T();
+    }
+}
+
+constexpr void
+test_array()
+{
+  int arr[1] { 99 };
+  std::construct_at(&arr);
+  VERIFY( arr[0] == 0 );
+
+  union U {
+    long long x = -1;
+    int arr[4];
+  } u;
+
+  auto p = std::construct_at(&u.arr);
+  VERIFY( (*p)[0] == 0 );
+}
+
+static_assert( [] { test_array(); return true; }() );