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

Reply via email to