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
