* [PATCH] c++: Further tweaks for new-expression and paren-init [PR77841]
@ 2020-09-06 15:34 Marek Polacek
2020-09-08 3:19 ` Jason Merrill
0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2020-09-06 15:34 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
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.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
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++.dg/cpp2a/paren-init36.C: New test.
---
gcc/cp/init.c | 37 ++++++++++---------
gcc/testsuite/g++.dg/cpp2a/paren-init36.C | 14 +++++++
gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C | 2 +-
gcc/testsuite/g++.old-deja/g++.robertl/eb58.C | 2 +-
4 files changed, 35 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init36.C
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 3268ae4ad3f..537651778b9 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,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)))
{
/* 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;
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);
@@ -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);
+ 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++.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" }
}
base-commit: 0dc80505562c89df617ea11c733ee2cfab53c3f7
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] c++: Further tweaks for new-expression and paren-init [PR77841]
2020-09-06 15:34 [PATCH] c++: Further tweaks for new-expression and paren-init [PR77841] Marek Polacek
@ 2020-09-08 3:19 ` Jason Merrill
2020-09-08 20:06 ` [PATCH v2] " Marek Polacek
0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2020-09-08 3:19 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 9/6/20 11:34 AM, Marek Polacek wrote:
> 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.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> 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++.dg/cpp2a/paren-init36.C: New test.
> ---
> gcc/cp/init.c | 37 ++++++++++---------
> gcc/testsuite/g++.dg/cpp2a/paren-init36.C | 14 +++++++
> gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C | 2 +-
> gcc/testsuite/g++.old-deja/g++.robertl/eb58.C | 2 +-
> 4 files changed, 35 insertions(+), 20 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init36.C
>
> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index 3268ae4ad3f..537651778b9 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,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.
> {
> /* 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.
> 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?
> @@ -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.
Why are we reshaping here, anyway? Won't that lead to undesired brace
elision?
> + 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++.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" }
> }
>
> base-commit: 0dc80505562c89df617ea11c733ee2cfab53c3f7
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] c++: Further tweaks for new-expression and paren-init [PR77841]
2020-09-08 3:19 ` Jason Merrill
@ 2020-09-08 20:06 ` Marek Polacek
2020-09-08 20:19 ` Jason Merrill
0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2020-09-08 20:06 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] c++: Further tweaks for new-expression and paren-init [PR77841]
2020-09-08 20:06 ` [PATCH v2] " Marek Polacek
@ 2020-09-08 20:19 ` Jason Merrill
2020-09-09 2:34 ` [PATCH v3] " Marek Polacek
0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2020-09-08 20:19 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] c++: Further tweaks for new-expression and paren-init [PR77841]
2020-09-08 20:19 ` Jason Merrill
@ 2020-09-09 2:34 ` Marek Polacek
2020-09-09 21:02 ` Jason Merrill
0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2020-09-09 2:34 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] c++: Further tweaks for new-expression and paren-init [PR77841]
2020-09-09 2:34 ` [PATCH v3] " Marek Polacek
@ 2020-09-09 21:02 ` Jason Merrill
2020-09-09 21:35 ` Marek Polacek
0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2020-09-09 21:02 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] c++: Further tweaks for new-expression and paren-init [PR77841]
2020-09-09 21:02 ` Jason Merrill
@ 2020-09-09 21:35 ` Marek Polacek
2020-09-09 21:43 ` Jason Merrill
0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2020-09-09 21:35 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Wed, Sep 09, 2020 at 05:02:24PM -0400, Jason Merrill wrote:
> 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?
Yeah, it doesn't seem to be needed: e.g. in
struct X { int i; };
struct Y : X { };
int main ()
{
Y y;
y.i = 42;
X x2(y);
__builtin_printf ("%d\n", x2.i);
}
we don't reshape_init at all, and build_aggr_init creates an INIT_EXPR
x2 = y.D.2087
which is the same code we generate with e.g. GCC 8, or if we use x2{y}.
Also, P0960 says that any existing meaning of A(b) should not change.
Marek
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] c++: Further tweaks for new-expression and paren-init [PR77841]
2020-09-09 21:35 ` Marek Polacek
@ 2020-09-09 21:43 ` Jason Merrill
0 siblings, 0 replies; 8+ messages in thread
From: Jason Merrill @ 2020-09-09 21:43 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On 9/9/20 5:35 PM, Marek Polacek wrote:
> On Wed, Sep 09, 2020 at 05:02:24PM -0400, Jason Merrill wrote:
>> 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?
>
> Yeah, it doesn't seem to be needed: e.g. in
>
> struct X { int i; };
> struct Y : X { };
> int main ()
> {
> Y y;
> y.i = 42;
> X x2(y);
> __builtin_printf ("%d\n", x2.i);
> }
>
> we don't reshape_init at all, and build_aggr_init creates an INIT_EXPR
> x2 = y.D.2087
> which is the same code we generate with e.g. GCC 8, or if we use x2{y}.
>
> Also, P0960 says that any existing meaning of A(b) should not change.
The patch is OK.
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-09 21:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-06 15:34 [PATCH] c++: Further tweaks for new-expression and paren-init [PR77841] Marek Polacek
2020-09-08 3:19 ` Jason Merrill
2020-09-08 20:06 ` [PATCH v2] " Marek Polacek
2020-09-08 20:19 ` Jason Merrill
2020-09-09 2:34 ` [PATCH v3] " Marek Polacek
2020-09-09 21:02 ` Jason Merrill
2020-09-09 21:35 ` Marek Polacek
2020-09-09 21:43 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).