| Message ID | or6064b0so.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PR,c++/84729] convert new init to array elt type | expand |
On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > A parenthesized initializer is only accepted when new()ing an array in > permissive mode. We were not careful, however, to convert the > TREE_LIST initializer to the array element type in this extension. > This patch fixes it: after turning the TREE_LIST initializer to a > compound_expr, we convert it to the base type. I think I'd rather turn the permerror into a hard error than improve support for a deprecated extension. Jason
On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >> A parenthesized initializer is only accepted when new()ing an array in >> permissive mode. We were not careful, however, to convert the >> TREE_LIST initializer to the array element type in this extension. >> This patch fixes it: after turning the TREE_LIST initializer to a >> compound_expr, we convert it to the base type. > I think I'd rather turn the permerror into a hard error than improve > support for a deprecated extension. Like this? [PR c++/84729] convert new init to array elt type A parenthesized initializer was only accepted when new()ing an array in permissive mode. We were not careful, however, to convert the TREE_LIST initializer to the array element type in this extension. Instead of fixing it, converting the initializer to the base type after turning the TREE_LIST initializer to a compound_expr, we disable this deprecated extension. Regstrapping. Ok to install if it passes? for gcc/cp/ChangeLog PR c++/84729 * init.c (build_vec_init): Convert tree list to base type. for gcc/testsuite/ChangeLog PR c++/84729 * g++.dg/pr84729.C: New. --- gcc/cp/init.c | 4 ++-- gcc/testsuite/g++.dg/pr84729.C | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr84729.C diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 15cee17c780c..9091eaa90267 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -3370,8 +3370,8 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, else if (*init) { if (complain & tf_error) - permerror (input_location, - "parenthesized initializer in array new"); + error_at (input_location, + "parenthesized initializer in array new"); else return error_mark_node; vecinit = build_tree_list_vec (*init); diff --git a/gcc/testsuite/g++.dg/pr84729.C b/gcc/testsuite/g++.dg/pr84729.C new file mode 100644 index 000000000000..e5d689e0460c --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84729.C @@ -0,0 +1,7 @@ +// { dg-do compile } +// { dg-options "-fpermissive" } + +typedef int b[2]; +void a() { + new b(a); // { dg-error "parenthesized initializer in array new" } +}
On Tue, Mar 20, 2018 at 5:56 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > >> On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> A parenthesized initializer is only accepted when new()ing an array in >>> permissive mode. We were not careful, however, to convert the >>> TREE_LIST initializer to the array element type in this extension. >>> This patch fixes it: after turning the TREE_LIST initializer to a >>> compound_expr, we convert it to the base type. > >> I think I'd rather turn the permerror into a hard error than improve >> support for a deprecated extension. > > Like this? > > [PR c++/84729] convert new init to array elt type > > A parenthesized initializer was only accepted when new()ing an array in > permissive mode. We were not careful, however, to convert the > TREE_LIST initializer to the array element type in this extension. > > Instead of fixing it, converting the initializer to the base type > after turning the TREE_LIST initializer to a compound_expr, we disable > this deprecated extension. > > Regstrapping. Ok to install if it passes? > > for gcc/cp/ChangeLog > > PR c++/84729 > * init.c (build_vec_init): Convert tree list to base type. > > for gcc/testsuite/ChangeLog > > PR c++/84729 > * g++.dg/pr84729.C: New. > --- > gcc/cp/init.c | 4 ++-- > gcc/testsuite/g++.dg/pr84729.C | 7 +++++++ > 2 files changed, 9 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/pr84729.C > > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > index 15cee17c780c..9091eaa90267 100644 > --- a/gcc/cp/init.c > +++ b/gcc/cp/init.c > @@ -3370,8 +3370,8 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, > else if (*init) > { > if (complain & tf_error) > - permerror (input_location, > - "parenthesized initializer in array new"); > + error_at (input_location, > + "parenthesized initializer in array new"); > else > return error_mark_node; I suspect you'll need to make the return unconditional to avoid the ICE; OK either way. Jason
On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: >> - permerror (input_location, >> - "parenthesized initializer in array new"); >> + error_at (input_location, >> + "parenthesized initializer in array new"); > I suspect you'll need to make the return unconditional to avoid the > ICE; OK either way. I didn't, but I had to adjust 3 preexisting testcases that relied on this extension. Last night I still had them with dg-do run, and that didn't work because compilation fails, so now I've adjusted them to compile only, and will retest. While at that, I simplified error_at (input_location, "... to error ("... I've also updated the patch description and the ChangeLog entry, which I had failed to do before posting the previous version of the patch. I looked for this extension in gcc/doc/extend.texi, to remove it, but I couldn't find it; is it really not there? Ok to install if regstrap succeeds? [PR c++/84729] reject parenthesized array init A parenthesized initializer was only accepted when new()ing an array in permissive mode. We were not careful, however, to convert the TREE_LIST initializer to the array element type in this extension. Instead of fixing it, converting the initializer to the base type after turning the TREE_LIST initializer to a compound_expr, we disable this deprecated extension. for gcc/cp/ChangeLog PR c++/84729 * init.c (build_vec_init): Error at parenthesized array init. for gcc/testsuite/ChangeLog PR c++/84729 * g++.dg/pr84729.C: New. * g++.old-deja/g++.ext/arrnew2.C: Require error. * g++.old-deja/g++.robertl/eb58.C: Likewise. * g++.old-deja/g++.robertl/eb63.C: Likewise. --- gcc/cp/init.c | 7 ++----- gcc/testsuite/g++.dg/pr84729.C | 7 +++++++ gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C | 4 ++-- gcc/testsuite/g++.old-deja/g++.robertl/eb58.C | 4 ++-- gcc/testsuite/g++.old-deja/g++.robertl/eb63.C | 4 ++-- 5 files changed, 15 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr84729.C diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 15cee17c780c..2263d12563cd 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -3370,11 +3370,8 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, else if (*init) { if (complain & tf_error) - permerror (input_location, - "parenthesized initializer in array new"); - else - return error_mark_node; - vecinit = build_tree_list_vec (*init); + error ("parenthesized initializer in array new"); + return error_mark_node; } init_expr = build_vec_init (data_addr, diff --git a/gcc/testsuite/g++.dg/pr84729.C b/gcc/testsuite/g++.dg/pr84729.C new file mode 100644 index 000000000000..e5d689e0460c --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84729.C @@ -0,0 +1,7 @@ +// { dg-do compile } +// { dg-options "-fpermissive" } + +typedef int b[2]; +void a() { + new b(a); // { dg-error "parenthesized initializer in array new" } +} diff --git a/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C b/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C index c6a967ccc385..aff6b9c7c63b 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 run } +// { dg-do compile } // { dg-options "-w -fpermissive" } -int *foo = new int[1](42); // { dg-bogus "" } +int *foo = new int[1](42); // { dg-error "parenthesized" } 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 04ec92a30afc..d702296bdc78 100644 --- a/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C +++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C @@ -1,4 +1,4 @@ -// { dg-do run } +// { dg-do compile } // { dg-options "-w -fpermissive" } // Test for g++ array init extension @@ -11,5 +11,5 @@ private: main() { - A *list = new A[10](4); + A *list = new A[10](4); // { dg-error "parenthesized" } } diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C b/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C index a49fb02641cd..653351b8dfad 100644 --- a/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C +++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C @@ -1,4 +1,4 @@ -// { dg-do run } +// { dg-do compile } // { dg-options "-w -fpermissive" } //This uses GNU extensions, so disable -ansi #include <stdio.h> @@ -13,5 +13,5 @@ public: main() { A* a; - a = new A[2](1,false); + a = new A[2](1,false); // { dg-error "parenthesized" } }
OK. On Wed, Mar 21, 2018 at 5:52 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > >>> - permerror (input_location, >>> - "parenthesized initializer in array new"); >>> + error_at (input_location, >>> + "parenthesized initializer in array new"); > >> I suspect you'll need to make the return unconditional to avoid the >> ICE; OK either way. > > I didn't, but I had to adjust 3 preexisting testcases that relied on > this extension. Last night I still had them with dg-do run, and that > didn't work because compilation fails, so now I've adjusted them to > compile only, and will retest. While at that, I simplified > error_at (input_location, "... > to > error ("... > > I've also updated the patch description and the ChangeLog entry, which I > had failed to do before posting the previous version of the patch. > > I looked for this extension in gcc/doc/extend.texi, to remove it, but I > couldn't find it; is it really not there? > > Ok to install if regstrap succeeds? > > > [PR c++/84729] reject parenthesized array init > > A parenthesized initializer was only accepted when new()ing an array in > permissive mode. We were not careful, however, to convert the > TREE_LIST initializer to the array element type in this extension. > > Instead of fixing it, converting the initializer to the base type > after turning the TREE_LIST initializer to a compound_expr, we disable > this deprecated extension. > > > for gcc/cp/ChangeLog > > PR c++/84729 > * init.c (build_vec_init): Error at parenthesized array init. > > for gcc/testsuite/ChangeLog > > PR c++/84729 > * g++.dg/pr84729.C: New. > * g++.old-deja/g++.ext/arrnew2.C: Require error. > * g++.old-deja/g++.robertl/eb58.C: Likewise. > * g++.old-deja/g++.robertl/eb63.C: Likewise. > --- > gcc/cp/init.c | 7 ++----- > gcc/testsuite/g++.dg/pr84729.C | 7 +++++++ > gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C | 4 ++-- > gcc/testsuite/g++.old-deja/g++.robertl/eb58.C | 4 ++-- > gcc/testsuite/g++.old-deja/g++.robertl/eb63.C | 4 ++-- > 5 files changed, 15 insertions(+), 11 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/pr84729.C > > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > index 15cee17c780c..2263d12563cd 100644 > --- a/gcc/cp/init.c > +++ b/gcc/cp/init.c > @@ -3370,11 +3370,8 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, > else if (*init) > { > if (complain & tf_error) > - permerror (input_location, > - "parenthesized initializer in array new"); > - else > - return error_mark_node; > - vecinit = build_tree_list_vec (*init); > + error ("parenthesized initializer in array new"); > + return error_mark_node; > } > init_expr > = build_vec_init (data_addr, > diff --git a/gcc/testsuite/g++.dg/pr84729.C b/gcc/testsuite/g++.dg/pr84729.C > new file mode 100644 > index 000000000000..e5d689e0460c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pr84729.C > @@ -0,0 +1,7 @@ > +// { dg-do compile } > +// { dg-options "-fpermissive" } > + > +typedef int b[2]; > +void a() { > + new b(a); // { dg-error "parenthesized initializer in array new" } > +} > diff --git a/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C b/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C > index c6a967ccc385..aff6b9c7c63b 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 run } > +// { dg-do compile } > // { dg-options "-w -fpermissive" } > > -int *foo = new int[1](42); // { dg-bogus "" } > +int *foo = new int[1](42); // { dg-error "parenthesized" } > 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 04ec92a30afc..d702296bdc78 100644 > --- a/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C > +++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C > @@ -1,4 +1,4 @@ > -// { dg-do run } > +// { dg-do compile } > // { dg-options "-w -fpermissive" } > // Test for g++ array init extension > > @@ -11,5 +11,5 @@ private: > > main() > { > - A *list = new A[10](4); > + A *list = new A[10](4); // { dg-error "parenthesized" } > } > diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C b/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C > index a49fb02641cd..653351b8dfad 100644 > --- a/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C > +++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C > @@ -1,4 +1,4 @@ > -// { dg-do run } > +// { dg-do compile } > // { dg-options "-w -fpermissive" } > //This uses GNU extensions, so disable -ansi > #include <stdio.h> > @@ -13,5 +13,5 @@ public: > main() { > A* a; > > - a = new A[2](1,false); > + a = new A[2](1,false); // { dg-error "parenthesized" } > } > > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 15cee17c780c..cb62f4886e6d 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -4405,8 +4405,14 @@ build_vec_init (tree base, tree maxindex, tree init, else { if (TREE_CODE (init) == TREE_LIST) - init = build_x_compound_expr_from_list (init, ELK_INIT, - complain); + { + init = build_x_compound_expr_from_list (init, ELK_INIT, + complain); + init + = convert_for_initialization (NULL_TREE, type, init, + LOOKUP_IMPLICIT, ICR_INIT, + NULL_TREE, 0, complain); + } elt_init = (init == error_mark_node ? error_mark_node : build2 (INIT_EXPR, type, to, init)); diff --git a/gcc/testsuite/g++.dg/pr84729.C b/gcc/testsuite/g++.dg/pr84729.C new file mode 100644 index 000000000000..6ca7fb0032f5 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84729.C @@ -0,0 +1,7 @@ +// { dg-do compile } +// { dg-options "-fpermissive" } + +typedef int b[2]; +void a() { + new b(a); // { dg-warning "parenthesized initializer in array new|invalid conversion" } +}