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
