On Tue, Sep 08, 2020 at 04:19:42PM -0400, Jason Merrill wrote: > On 9/8/20 4:06 PM, Marek Polacek wrote: > > On Mon, Sep 07, 2020 at 11:19:47PM -0400, Jason Merrill wrote: > > > On 9/6/20 11:34 AM, Marek Polacek wrote: > > > > @@ -3944,9 +3935,9 @@ build_new (location_t loc, vec<tree, va_gc> > > > > **placement, tree type, > > > > } > > > > /* P1009: Array size deduction in new-expressions. */ > > > > - if (TREE_CODE (type) == ARRAY_TYPE > > > > - && !TYPE_DOMAIN (type) > > > > - && *init) > > > > + const bool deduce_array_p = (TREE_CODE (type) == ARRAY_TYPE > > > > + && !TYPE_DOMAIN (type)); > > > > + if (*init && (deduce_array_p || (nelts && cxx_dialect >= cxx20))) > > > > > > Looks like this won't handle new (char[4]), for which we also get an > > > ARRAY_TYPE. > > > > Good catch. Fixed & paren-init37.C added. > > > > > > { > > > > /* This means we have 'new T[]()'. */ > > > > if ((*init)->is_empty ()) > > > > @@ -3955,16 +3946,20 @@ build_new (location_t loc, vec<tree, va_gc> > > > > **placement, tree type, > > > > CONSTRUCTOR_IS_DIRECT_INIT (ctor) = true; > > > > vec_safe_push (*init, ctor); > > > > } > > > > + tree array_type = deduce_array_p ? TREE_TYPE (type) : type; > > > > > > I'd call this variable elt_type. > > > > Right, and it should be inside the block below. > > > > > > tree &elt = (**init)[0]; > > > > /* The C++20 'new T[](e_0, ..., e_k)' case allowed by P0960. > > > > */ > > > > if (!DIRECT_LIST_INIT_P (elt) && cxx_dialect >= cxx20) > > > > { > > > > - /* Handle new char[]("foo"). */ > > > > + /* Handle new char[]("foo"): turn it into new char[]{"foo"}. > > > > */ > > > > if (vec_safe_length (*init) == 1 > > > > - && char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type))) > > > > + && char_type_p (TYPE_MAIN_VARIANT (array_type)) > > > > && TREE_CODE (tree_strip_any_location_wrapper (elt)) > > > > == STRING_CST) > > > > - /* Leave it alone: the string should not be wrapped in {}. > > > > */; > > > > + { > > > > + elt = build_constructor_single (init_list_type_node, > > > > NULL_TREE, elt); > > > > + CONSTRUCTOR_IS_DIRECT_INIT (elt) = true; > > > > + } > > > > else > > > > { > > > > tree ctor = build_constructor_from_vec > > > > (init_list_type_node, *init); > > > > > > With this change, doesn't the string special case produce the same result > > > as > > > the general case? > > > > The problem is that reshape_init won't do anything for > > CONSTRUCTOR_IS_PAREN_INIT. > > Ah, yes, that flag is the difference. > > > So the reshape_init in build_new_1 wouldn't unwrap the outermost { } around > > a STRING_CST. > > > Perhaps reshape_init should be adjusted to do that unwrapping even when it > > gets > > a CONSTRUCTOR_IS_PAREN_INIT CONSTRUCTOR. But I'm not sure if it should > > also do > > the reference_related_p unwrapping in reshape_init_r in that case. > > That would make sense to me.
Done (but only for the outermost CONSTRUCTOR) in the below. It allowed me to... > > > > @@ -3977,9 +3972,15 @@ build_new (location_t loc, vec<tree, va_gc> > > > > **placement, tree type, > > > > } > > > > } > > > > /* Otherwise we should have 'new T[]{e_0, ..., e_k}'. */ > > > > - if (BRACE_ENCLOSED_INITIALIZER_P (elt)) > > > > - elt = reshape_init (type, elt, complain); > > > > - cp_complete_array_type (&type, elt, /*do_default*/false); > > > > + if (deduce_array_p) > > > > + { > > > > + /* Don't reshape ELT itself: we want to pass a > > > > list-initializer to > > > > + build_new_1, even for STRING_CSTs. */ > > > > + tree e = elt; > > > > + if (BRACE_ENCLOSED_INITIALIZER_P (e)) > > > > + e = reshape_init (type, e, complain); > > > > > > The comment is unclear; this call does reshape the CONSTRUCTOR ELT points > > > to, it just doesn't change ELT if the reshape call returns something else. > > > > Yea, I've amended the comment. > > > > > Why are we reshaping here, anyway? Won't that lead to undesired brace > > > elision? > > > > We have to reshape before deducing the array, otherwise we could deduce the > > wrong number of elements when certain braces were omitted. E.g. in > > > > struct S { int x, y; }; > > new S[]{1, 2, 3, 4}; // braces elided, is { {1, 2}, {3, 4} } > > Ah, right, we also get here for initializers written with actual braces. > > > we want S[2], not S[4]. A way to test it would be > > > > struct S { int x, y; }; > > S *p = new S[]{1, 2, 3, 4}; > > > > void* operator new (unsigned long int size) > > { > > if (size != sizeof (S) * 2) > > __builtin_abort (); > > return __builtin_malloc (size); > > } > > > > int main () { } > > > > I can add that too, if you want. (It'd be safer if cp_complete_array_type > > always reshaped but that's not trivial, as the original patch mentions.) > > ()-init-list wouldn't be reshaped because CONSTRUCTOR_IS_PAREN_INIT is set. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > Thanks, > > > > -- >8 -- > > This patch corrects our handling of array new-expression with ()-init: > > > > new int[4](1, 2, 3, 4); > > > > should work even with the explicit array bound, and > > > > new char[3]("so_sad"); > > > > should cause an error, but we weren't giving any. > > > > Fixed by handling array new-expressions with ()-init in the same spot > > where we deduce the array bound in array new-expression. I'm now > > always passing STRING_CSTs to build_new_1 wrapped in { } which allowed > > me to remove the special handling of STRING_CSTs in build_new_1. And > > since the DIRECT_LIST_INIT_P block in build_new_1 calls digest_init, we > > report errors about too short arrays. > > > > I took a stab at cp_complete_array_type's "FIXME: this code is duplicated > > from reshape_init", but calling reshape_init there, I ran into issues > > with has_designator_problem: when we reshape an already reshaped > > CONSTRUCTOR again, d.cur.index has been filled, so we think that we > > have a user-provided designator (though there was no designator in the > > source code), and report an error. > > > > gcc/cp/ChangeLog: > > > > PR c++/77841 > > * init.c (build_new_1): Don't handle string-initializers here. > > (build_new): Handle new-expression with paren-init when the > > array bound is known. Always pass string constants to build_new_1 > > enclosed in braces. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/77841 > > * g++.old-deja/g++.ext/arrnew2.C: Expect the error only in C++17 > > and less. > > * g++.old-deja/g++.robertl/eb58.C: Adjust dg-error. > > * g++.old-deja/g++.robertl/eb63.C: Expect the error only in C++17 > > and less. > > * g++.dg/cpp2a/paren-init36.C: New test. > > * g++.dg/cpp2a/paren-init37.C: New test. > > * g++.dg/pr84729.C: Adjust dg-error. > > --- > > gcc/cp/init.c | 41 +++++++++++-------- > > gcc/testsuite/g++.dg/cpp2a/paren-init36.C | 14 +++++++ > > gcc/testsuite/g++.dg/cpp2a/paren-init37.C | 14 +++++++ > > gcc/testsuite/g++.dg/pr84729.C | 2 +- > > gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C | 2 +- > > gcc/testsuite/g++.old-deja/g++.robertl/eb58.C | 2 +- > > gcc/testsuite/g++.old-deja/g++.robertl/eb63.C | 2 +- > > 7 files changed, 55 insertions(+), 22 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init36.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init37.C > > > > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > > index 3268ae4ad3f..fe4d49df402 100644 > > --- a/gcc/cp/init.c > > +++ b/gcc/cp/init.c > > @@ -3596,15 +3596,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree > > type, tree nelts, > > vecinit = digest_init (arraytype, vecinit, complain); > > } > > } > > - /* This handles code like new char[]{"foo"}. */ > > - else if (len == 1 > > - && char_type_p (TYPE_MAIN_VARIANT (type)) > > - && TREE_CODE (tree_strip_any_location_wrapper ((**init)[0])) > > - == STRING_CST) > > - { > > - vecinit = (**init)[0]; > > - STRIP_ANY_LOCATION_WRAPPER (vecinit); > > - } > > else if (*init) > > { > > if (complain & tf_error) > > @@ -3944,9 +3935,8 @@ build_new (location_t loc, vec<tree, va_gc> > > **placement, tree type, > > } > > /* P1009: Array size deduction in new-expressions. */ > > - if (TREE_CODE (type) == ARRAY_TYPE > > - && !TYPE_DOMAIN (type) > > - && *init) > > + const bool array_p = TREE_CODE (type) == ARRAY_TYPE; > > + if (*init && (array_p || (nelts && cxx_dialect >= cxx20))) > > { > > /* This means we have 'new T[]()'. */ > > if ((*init)->is_empty ()) > > @@ -3959,12 +3949,16 @@ build_new (location_t loc, vec<tree, va_gc> > > **placement, tree type, > > /* The C++20 'new T[](e_0, ..., e_k)' case allowed by P0960. */ > > if (!DIRECT_LIST_INIT_P (elt) && cxx_dialect >= cxx20) > > { > > - /* Handle new char[]("foo"). */ > > + tree elt_type = array_p ? TREE_TYPE (type) : type; > > I think this should condition on whether nelts is set, to handle e.g. new > char[2][2] properly. ...remove this code. I've added new-array5.C to test multidimensional arrays too. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- This patch corrects our handling of array new-expression with ()-init: new int[4](1, 2, 3, 4); should work even with the explicit array bound, and new char[3]("so_sad"); should cause an error, but we weren't giving any. Fixed by handling array new-expressions with ()-init in the same spot where we deduce the array bound in array new-expression. I'm now always passing STRING_CSTs to build_new_1 wrapped in { } which allowed me to remove the special handling of STRING_CSTs in build_new_1. And since the DIRECT_LIST_INIT_P block in build_new_1 calls digest_init, we report errors about too short arrays. reshape_init now does the {"foo"} -> "foo" transformation even for CONSTRUCTOR_IS_PAREN_INIT, so no need to do it in build_new. I took a stab at cp_complete_array_type's "FIXME: this code is duplicated from reshape_init", but calling reshape_init there, I ran into issues with has_designator_problem: when we reshape an already reshaped CONSTRUCTOR again, d.cur.index has been filled, so we think that we have a user-provided designator (though there was no designator in the source code), and report an error. gcc/cp/ChangeLog: PR c++/77841 * decl.c (reshape_init): If we're initializing a char array from a string-literal that is enclosed in braces, unwrap it. * init.c (build_new_1): Don't handle string-initializers here. (build_new): Handle new-expression with paren-init when the array bound is known. Always pass string constants to build_new_1 enclosed in braces. Don't handle string-initializers in any special way. gcc/testsuite/ChangeLog: PR c++/77841 * g++.old-deja/g++.ext/arrnew2.C: Expect the error only in C++17 and less. * g++.old-deja/g++.robertl/eb58.C: Adjust dg-error. * g++.old-deja/g++.robertl/eb63.C: Expect the error only in C++17 and less. * g++.dg/cpp2a/new-array5.C: New test. * g++.dg/cpp2a/paren-init36.C: New test. * g++.dg/cpp2a/paren-init37.C: New test. * g++.dg/pr84729.C: Adjust dg-error. --- gcc/cp/decl.c | 12 ++++- gcc/cp/init.c | 54 ++++++++----------- gcc/testsuite/g++.dg/cpp2a/new-array5.C | 15 ++++++ gcc/testsuite/g++.dg/cpp2a/paren-init36.C | 14 +++++ gcc/testsuite/g++.dg/cpp2a/paren-init37.C | 14 +++++ gcc/testsuite/g++.dg/pr84729.C | 2 +- gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C | 2 +- gcc/testsuite/g++.old-deja/g++.robertl/eb58.C | 2 +- gcc/testsuite/g++.old-deja/g++.robertl/eb63.C | 2 +- 9 files changed, 81 insertions(+), 36 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/new-array5.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init36.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init37.C diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 31d68745844..1287ce1efd1 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6599,7 +6599,17 @@ reshape_init (tree type, tree init, tsubst_flags_t complain) /* Brace elision is not performed for a CONSTRUCTOR representing parenthesized aggregate initialization. */ if (CONSTRUCTOR_IS_PAREN_INIT (init)) - return init; + { + tree elt = (*v)[0].value; + /* If we're initializing a char array from a string-literal that is + enclosed in braces, unwrap it here. */ + if (TREE_CODE (type) == ARRAY_TYPE + && vec_safe_length (v) == 1 + && char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type))) + && TREE_CODE (tree_strip_any_location_wrapper (elt)) == STRING_CST) + return elt; + return init; + } /* Handle [dcl.init.list] direct-list-initialization from single element of enumeration with a fixed underlying type. */ diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 3268ae4ad3f..e84e985492d 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -3596,15 +3596,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, vecinit = digest_init (arraytype, vecinit, complain); } } - /* This handles code like new char[]{"foo"}. */ - else if (len == 1 - && char_type_p (TYPE_MAIN_VARIANT (type)) - && TREE_CODE (tree_strip_any_location_wrapper ((**init)[0])) - == STRING_CST) - { - vecinit = (**init)[0]; - STRIP_ANY_LOCATION_WRAPPER (vecinit); - } else if (*init) { if (complain & tf_error) @@ -3944,9 +3935,8 @@ build_new (location_t loc, vec<tree, va_gc> **placement, tree type, } /* P1009: Array size deduction in new-expressions. */ - if (TREE_CODE (type) == ARRAY_TYPE - && !TYPE_DOMAIN (type) - && *init) + const bool array_p = TREE_CODE (type) == ARRAY_TYPE; + if (*init && (array_p || (nelts && cxx_dialect >= cxx20))) { /* This means we have 'new T[]()'. */ if ((*init)->is_empty ()) @@ -3959,27 +3949,29 @@ build_new (location_t loc, vec<tree, va_gc> **placement, tree type, /* The C++20 'new T[](e_0, ..., e_k)' case allowed by P0960. */ if (!DIRECT_LIST_INIT_P (elt) && cxx_dialect >= cxx20) { - /* Handle new char[]("foo"). */ - if (vec_safe_length (*init) == 1 - && char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type))) - && TREE_CODE (tree_strip_any_location_wrapper (elt)) - == STRING_CST) - /* Leave it alone: the string should not be wrapped in {}. */; - else - { - tree ctor = build_constructor_from_vec (init_list_type_node, *init); - CONSTRUCTOR_IS_DIRECT_INIT (ctor) = true; - CONSTRUCTOR_IS_PAREN_INIT (ctor) = true; - elt = ctor; - /* We've squashed all the vector elements into the first one; - truncate the rest. */ - (*init)->truncate (1); - } + tree ctor = build_constructor_from_vec (init_list_type_node, *init); + CONSTRUCTOR_IS_DIRECT_INIT (ctor) = true; + CONSTRUCTOR_IS_PAREN_INIT (ctor) = true; + elt = ctor; + /* We've squashed all the vector elements into the first one; + truncate the rest. */ + (*init)->truncate (1); } /* Otherwise we should have 'new T[]{e_0, ..., e_k}'. */ - if (BRACE_ENCLOSED_INITIALIZER_P (elt)) - elt = reshape_init (type, elt, complain); - cp_complete_array_type (&type, elt, /*do_default*/false); + if (array_p && !TYPE_DOMAIN (type)) + { + /* We need to reshape before deducing the bounds to handle code like + + struct S { int x, y; }; + new S[]{1, 2, 3, 4}; + + which should deduce S[2]. But don't change ELT itself: we want to + pass a list-initializer to build_new_1, even for STRING_CSTs. */ + tree e = elt; + if (BRACE_ENCLOSED_INITIALIZER_P (e)) + e = reshape_init (type, e, complain); + cp_complete_array_type (&type, e, /*do_default*/false); + } } /* The type allocated must be complete. If the new-type-id was diff --git a/gcc/testsuite/g++.dg/cpp2a/new-array5.C b/gcc/testsuite/g++.dg/cpp2a/new-array5.C new file mode 100644 index 00000000000..2379079ca85 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/new-array5.C @@ -0,0 +1,15 @@ +// PR c++/77841 +// { dg-do compile { target c++11 } } + +auto p1 = new int[][1](); +auto p2 = new int[1][1](); +#if __cpp_aggregate_paren_init +auto p3 = new int[][4]({1, 2}, {3, 4}); +auto p4 = new int[2][4]({1, 2}, {3, 4}); +auto p5 = new int[2][1]({1, 2}, {3}); // { dg-error "too many initializers" "" { target c++20 } } +#endif + +auto b1 = new int[][1]{}; +auto b2 = new int[1][1]{}; +auto b3 = new int[][4]{{1, 2}, {3, 4}}; +auto b4 = new int[2][4]{{1, 2}, {3, 4}}; diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init36.C b/gcc/testsuite/g++.dg/cpp2a/paren-init36.C new file mode 100644 index 00000000000..996249515bf --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/paren-init36.C @@ -0,0 +1,14 @@ +// PR c++/77841 +// { dg-do compile { target c++20 } } + +int *p0 = new int[1](); +int *p1 = new int[1](1); +int *p2 = new int[4](1, 2, 3, 4); +int *p3 = new int[2](1, 2, 3, 4); // { dg-error "too many initializers" } + +char *c1 = new char[]("foo"); +char *c2 = new char[4]("foo"); +char *c3 = new char[]{"foo"}; +char *c4 = new char[4]{"foo"}; +char *c5 = new char[3]("so_sad"); // { dg-error "too long" } +char *c6 = new char[3]{"so_sad"}; // { dg-error "too long" } diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init37.C b/gcc/testsuite/g++.dg/cpp2a/paren-init37.C new file mode 100644 index 00000000000..551a9822224 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/paren-init37.C @@ -0,0 +1,14 @@ +// PR c++/77841 +// { dg-do compile { target c++20 } } + +int *p0 = new (int[1])(); +int *p1 = new (int[1])(1); +int *p2 = new (int[4])(1, 2, 3, 4); +int *p3 = new (int[2])(1, 2, 3, 4); // { dg-error "too many initializers" } + +char *c1 = new (char[])("foo"); +char *c2 = new (char[4])("foo"); +char *c3 = new (char[]){"foo"}; +char *c4 = new (char[4]){"foo"}; +char *c5 = new (char[3])("so_sad"); // { dg-error "too long" } +char *c6 = new (char[3]){"so_sad"}; // { dg-error "too long" } diff --git a/gcc/testsuite/g++.dg/pr84729.C b/gcc/testsuite/g++.dg/pr84729.C index e5d689e0460..530dbff23e1 100644 --- a/gcc/testsuite/g++.dg/pr84729.C +++ b/gcc/testsuite/g++.dg/pr84729.C @@ -3,5 +3,5 @@ typedef int b[2]; void a() { - new b(a); // { dg-error "parenthesized initializer in array new" } + new b(a); // { dg-error "parenthesized initializer in array new|invalid conversion" } } diff --git a/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C b/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C index aff6b9c7c63..fceb95e9ee5 100644 --- a/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C +++ b/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C @@ -1,7 +1,7 @@ // { dg-do compile } // { dg-options "-w -fpermissive" } -int *foo = new int[1](42); // { dg-error "parenthesized" } +int *foo = new int[1](42); // { dg-error "parenthesized" "" { target c++17_down } } int main () { return foo[0] != 42; diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C b/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C index d702296bdc7..1e51e14c51d 100644 --- a/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C +++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C @@ -11,5 +11,5 @@ private: main() { - A *list = new A[10](4); // { dg-error "parenthesized" } + A *list = new A[10](4); // { dg-error "parenthesized|could not convert" } } diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C b/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C index 653351b8dfa..50cf30f28fc 100644 --- a/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C +++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C @@ -13,5 +13,5 @@ public: main() { A* a; - a = new A[2](1,false); // { dg-error "parenthesized" } + a = new A[2](1,false); // { dg-error "parenthesized" "" { target c++17_down } } } base-commit: 494c5103c9eab8d3cd4364ab1265ee43ee51532f -- 2.26.2