Message ID | or1sgi3nrl.fsf@lxoliva.fsfla.org |
---|---|
State | New |
Headers | show |
Series | [PR,c++/71965] silence multi-dim array init sorry without tf_error | expand |
On Mar 17, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > We shouldn't substitute templates into short-circuited-out concepts > constraints, but we do, and to add insult to injury, we issue a > sorry() error when a concept that shouldn't even have been substituted > attempts to perform a multi-dimensional array initialization with a > new{} expression. > Although fixing the requirements short-circuiting is probably too > risky at this point, we can get closer to the intended effect by > silencing that sorry just as we silence other errors. Err... Regstrapped on i686- and x86_64-linux-gnu. Ok to install? > for gcc/cp/ChangeLog > PR c++/71965 > * init.c (build_vec_init): Silence sorry without tf_error. > for gcc/testsuite/ChangeLog > PR c++/71965 > * g++.dg/concepts/pr71965.C: New.
On Sat, Mar 17, 2018 at 8:11 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > We shouldn't substitute templates into short-circuited-out concepts > constraints, but we do, and to add insult to injury, we issue a > sorry() error when a concept that shouldn't even have been substituted > attempts to perform a multi-dimensional array initialization with a > new{} expression. > > Although fixing the requirements short-circuiting is probably too > risky at this point, we can get closer to the intended effect by > silencing that sorry just as we silence other errors. > > for gcc/cp/ChangeLog > > PR c++/71965 > * init.c (build_vec_init): Silence sorry without tf_error. > > for gcc/testsuite/ChangeLog > > PR c++/71965 > * g++.dg/concepts/pr71965.C: New. > --- > gcc/cp/init.c | 19 ++++++++++++------- > gcc/testsuite/g++.dg/concepts/pr71965.C | 27 +++++++++++++++++++++++++++ > 2 files changed, 39 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/pr71965.C > > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > index cb62f4886e6d..dcaad730dc86 100644 > --- a/gcc/cp/init.c > +++ b/gcc/cp/init.c > @@ -4384,12 +4384,17 @@ build_vec_init (tree base, tree maxindex, tree init, > else if (TREE_CODE (type) == ARRAY_TYPE) > { > if (init && !BRACE_ENCLOSED_INITIALIZER_P (init)) > - sorry > - ("cannot initialize multi-dimensional array with initializer"); > - elt_init = build_vec_init (build1 (INDIRECT_REF, type, base), > - 0, init, > - explicit_value_init_p, > - 0, complain); > + { > + if ((complain & tf_error)) > + sorry ("cannot initialize multi-dimensional" > + " array with initializer"); > + elt_init = error_mark_node; This shouldn't even be a sorry anymore; in build_aggr_init, we have permerror (init_loc, "array must be initialized " "with a brace-enclosed initializer"); Let's make it a hard error here. BTW, please include the word "PATCH" in the subject line of patch submissions. I will also see them faster if you CC me. Jason
On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > On Sat, Mar 17, 2018 at 8:11 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >> - sorry >> - ("cannot initialize multi-dimensional array with initializer"); > This shouldn't even be a sorry anymore > Let's make it a hard error here. Like this? [PR c++/71965] silence multi-dim array init sorry without tf_error We shouldn't substitute templates into short-circuited-out concepts constraints, but we do, and to add insult to injury, we issue a sorry() error when a concept that shouldn't even have been substituted attempts to perform a multi-dimensional array initialization with a new{} expression. Although fixing the requirements short-circuiting is probably too risky at this point, we can get closer to the intended effect by silencing that sorry just as we silence other errors. Regstrapping... Ok to install if it passes? for gcc/cp/ChangeLog PR c++/71965 * init.c (build_vec_init): Silence error, former sorry, without tf_error. for gcc/testsuite/ChangeLog PR c++/71965 * g++.dg/concepts/pr71965.C: New. --- gcc/cp/init.c | 19 ++++++++++++------- gcc/testsuite/g++.dg/concepts/pr71965.C | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/pr71965.C diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 9091eaa90267..5dd4b407d73f 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -4384,12 +4384,17 @@ build_vec_init (tree base, tree maxindex, tree init, else if (TREE_CODE (type) == ARRAY_TYPE) { if (init && !BRACE_ENCLOSED_INITIALIZER_P (init)) - sorry - ("cannot initialize multi-dimensional array with initializer"); - elt_init = build_vec_init (build1 (INDIRECT_REF, type, base), - 0, init, - explicit_value_init_p, - 0, complain); + { + if ((complain & tf_error)) + error ("cannot initialize multi-dimensional" + " array with initializer"); + elt_init = error_mark_node; + } + else + elt_init = build_vec_init (build1 (INDIRECT_REF, type, base), + 0, init, + explicit_value_init_p, + 0, complain); } else if (explicit_value_init_p) { @@ -4449,7 +4454,7 @@ build_vec_init (tree base, tree maxindex, tree init, } current_stmt_tree ()->stmts_are_full_exprs_p = 1; - if (elt_init) + if (elt_init && !errors) finish_expr_stmt (elt_init); current_stmt_tree ()->stmts_are_full_exprs_p = 0; diff --git a/gcc/testsuite/g++.dg/concepts/pr71965.C b/gcc/testsuite/g++.dg/concepts/pr71965.C new file mode 100644 index 000000000000..6bfaef19211f --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/pr71965.C @@ -0,0 +1,27 @@ +// { dg-do compile { target c++14 } } +// { dg-options "-fconcepts" } + +template <class T> +concept bool Destructible() { + return false; +} + +template <class T, class...Args> +concept bool ConstructibleObject = + // Concept evaluation should short-circuit even the template + // substitution, so we shouldn't even substitute into the requires + // constraint and the unimplemented multi-dimensional new T{...} + // initialization. ATM we do, but as long as we don't output the + // sorry() message we used to for such constructs when asked not + // to issue errors, this shouldn't be a problem for this and + // similar cases. + Destructible<T>() && requires (Args&&...args) { + new T{ (Args&&)args... }; + }; + +int main() { + using T = int[2][2]; + // GCC has not implemented initialization of multi-dimensional + // arrays with new{} expressions. + static_assert(!ConstructibleObject<T, T>); +}
On Tue, Mar 20, 2018 at 5:55 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > >> On Sat, Mar 17, 2018 at 8:11 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> - sorry >>> - ("cannot initialize multi-dimensional array with initializer"); > >> This shouldn't even be a sorry anymore > >> Let's make it a hard error here. > > Like this? > > > [PR c++/71965] silence multi-dim array init sorry without tf_error > > We shouldn't substitute templates into short-circuited-out concepts > constraints, but we do, and to add insult to injury, we issue a > sorry() error when a concept that shouldn't even have been substituted > attempts to perform a multi-dimensional array initialization with a > new{} expression. > > Although fixing the requirements short-circuiting is probably too > risky at this point, we can get closer to the intended effect by > silencing that sorry just as we silence other errors. > > Regstrapping... Ok to install if it passes? > > for gcc/cp/ChangeLog > > PR c++/71965 > * init.c (build_vec_init): Silence error, former sorry, > without tf_error. > > for gcc/testsuite/ChangeLog > > PR c++/71965 > * g++.dg/concepts/pr71965.C: New. > --- > gcc/cp/init.c | 19 ++++++++++++------- > gcc/testsuite/g++.dg/concepts/pr71965.C | 27 +++++++++++++++++++++++++++ > 2 files changed, 39 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/pr71965.C > > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > index 9091eaa90267..5dd4b407d73f 100644 > --- a/gcc/cp/init.c > +++ b/gcc/cp/init.c > @@ -4384,12 +4384,17 @@ build_vec_init (tree base, tree maxindex, tree init, > else if (TREE_CODE (type) == ARRAY_TYPE) > { > if (init && !BRACE_ENCLOSED_INITIALIZER_P (init)) > - sorry > - ("cannot initialize multi-dimensional array with initializer"); > - elt_init = build_vec_init (build1 (INDIRECT_REF, type, base), > - 0, init, > - explicit_value_init_p, > - 0, complain); > + { > + if ((complain & tf_error)) > + error ("cannot initialize multi-dimensional" > + " array with initializer"); Let's also use the other diagnostic message: "array must be initialized with a brace-enclosed initializer". OK with that change. Jason
On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: >> + if ((complain & tf_error)) >> + error ("cannot initialize multi-dimensional" >> + " array with initializer"); > Let's also use the other diagnostic message: "array must be > initialized with a brace-enclosed initializer". > OK with that change. Thanks. Besides changing the message, I added the location of the initializer to the message. Here's what I'm checking in: [PR c++/71965] silence multi-dim array init sorry without tf_error We shouldn't substitute templates into short-circuited-out concepts constraints, but we do, and to add insult to injury, we issue a sorry() error when a concept that shouldn't even have been substituted attempts to perform a multi-dimensional array initialization with a new{} expression. Although fixing the requirements short-circuiting is probably too risky at this point, we can get closer to the intended effect by silencing that sorry just as we silence other errors. for gcc/cp/ChangeLog PR c++/71965 * init.c (build_vec_init): Silence error, former sorry, without tf_error. for gcc/testsuite/ChangeLog PR c++/71965 * g++.dg/concepts/pr71965.C: New. --- gcc/cp/init.c | 19 ++++++++++++------- gcc/testsuite/g++.dg/concepts/pr71965.C | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/pr71965.C diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 2263d12563cd..ff52c42c1ad8 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -4381,12 +4381,17 @@ build_vec_init (tree base, tree maxindex, tree init, else if (TREE_CODE (type) == ARRAY_TYPE) { if (init && !BRACE_ENCLOSED_INITIALIZER_P (init)) - sorry - ("cannot initialize multi-dimensional array with initializer"); - elt_init = build_vec_init (build1 (INDIRECT_REF, type, base), - 0, init, - explicit_value_init_p, - 0, complain); + { + if ((complain & tf_error)) + error_at (loc, "array must be initialized " + "with a brace-enclosed initializer"); + elt_init = error_mark_node; + } + else + elt_init = build_vec_init (build1 (INDIRECT_REF, type, base), + 0, init, + explicit_value_init_p, + 0, complain); } else if (explicit_value_init_p) { @@ -4446,7 +4451,7 @@ build_vec_init (tree base, tree maxindex, tree init, } current_stmt_tree ()->stmts_are_full_exprs_p = 1; - if (elt_init) + if (elt_init && !errors) finish_expr_stmt (elt_init); current_stmt_tree ()->stmts_are_full_exprs_p = 0; diff --git a/gcc/testsuite/g++.dg/concepts/pr71965.C b/gcc/testsuite/g++.dg/concepts/pr71965.C new file mode 100644 index 000000000000..6bfaef19211f --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/pr71965.C @@ -0,0 +1,27 @@ +// { dg-do compile { target c++14 } } +// { dg-options "-fconcepts" } + +template <class T> +concept bool Destructible() { + return false; +} + +template <class T, class...Args> +concept bool ConstructibleObject = + // Concept evaluation should short-circuit even the template + // substitution, so we shouldn't even substitute into the requires + // constraint and the unimplemented multi-dimensional new T{...} + // initialization. ATM we do, but as long as we don't output the + // sorry() message we used to for such constructs when asked not + // to issue errors, this shouldn't be a problem for this and + // similar cases. + Destructible<T>() && requires (Args&&...args) { + new T{ (Args&&)args... }; + }; + +int main() { + using T = int[2][2]; + // GCC has not implemented initialization of multi-dimensional + // arrays with new{} expressions. + static_assert(!ConstructibleObject<T, T>); +}
diff --git a/gcc/cp/init.c b/gcc/cp/init.c index cb62f4886e6d..dcaad730dc86 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -4384,12 +4384,17 @@ build_vec_init (tree base, tree maxindex, tree init, else if (TREE_CODE (type) == ARRAY_TYPE) { if (init && !BRACE_ENCLOSED_INITIALIZER_P (init)) - sorry - ("cannot initialize multi-dimensional array with initializer"); - elt_init = build_vec_init (build1 (INDIRECT_REF, type, base), - 0, init, - explicit_value_init_p, - 0, complain); + { + if ((complain & tf_error)) + sorry ("cannot initialize multi-dimensional" + " array with initializer"); + elt_init = error_mark_node; + } + else + elt_init = build_vec_init (build1 (INDIRECT_REF, type, base), + 0, init, + explicit_value_init_p, + 0, complain); } else if (explicit_value_init_p) { @@ -4455,7 +4460,7 @@ build_vec_init (tree base, tree maxindex, tree init, } current_stmt_tree ()->stmts_are_full_exprs_p = 1; - if (elt_init) + if (elt_init && !errors) finish_expr_stmt (elt_init); current_stmt_tree ()->stmts_are_full_exprs_p = 0; diff --git a/gcc/testsuite/g++.dg/concepts/pr71965.C b/gcc/testsuite/g++.dg/concepts/pr71965.C new file mode 100644 index 000000000000..6bfaef19211f --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/pr71965.C @@ -0,0 +1,27 @@ +// { dg-do compile { target c++14 } } +// { dg-options "-fconcepts" } + +template <class T> +concept bool Destructible() { + return false; +} + +template <class T, class...Args> +concept bool ConstructibleObject = + // Concept evaluation should short-circuit even the template + // substitution, so we shouldn't even substitute into the requires + // constraint and the unimplemented multi-dimensional new T{...} + // initialization. ATM we do, but as long as we don't output the + // sorry() message we used to for such constructs when asked not + // to issue errors, this shouldn't be a problem for this and + // similar cases. + Destructible<T>() && requires (Args&&...args) { + new T{ (Args&&)args... }; + }; + +int main() { + using T = int[2][2]; + // GCC has not implemented initialization of multi-dimensional + // arrays with new{} expressions. + static_assert(!ConstructibleObject<T, T>); +}