[PR c++/71965] silence multi-dim array init sorry without tf_error

[PR c++/71965] silence multi-dim array init sorry without tf_error

[Alexandre Oliva]

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;
+	    }
+	  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>);
+}


-- 
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

Re: [PR c++/71965] silence multi-dim array init sorry without tf_error

[Alexandre Oliva]

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.

-- 
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

Re: [PR c++/71965] silence multi-dim array init sorry without tf_error

[Jason Merrill]

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

Re: [PATCH] [PR c++/71965] silence multi-dim array init sorry without tf_error

[Alexandre Oliva]

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>);
+}


-- 
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

Re: [PATCH] [PR c++/71965] silence multi-dim array init sorry without tf_error

[Jason Merrill]

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

Re: [PATCH] [PR c++/71965] silence multi-dim array init sorry without tf_error

[Alexandre Oliva]

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>);
+}


-- 
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