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.
@@ -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.
+ /* 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
