On 9/8/20 10:34 PM, Marek Polacek wrote:
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;
You decided against unwrapping a reference_related_p initializer here?
Jason
