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.
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.
> > @@ -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} }
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;
+ /* 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 (elt_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);
@@ -3977,9 +3971,20 @@ 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 (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/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: 87603e565615db055f7f60db0c9888f71d233826
--
2.26.2
