c++: Don't reject GOTO_EXPRs to cdtor_label in potential_constant_expression_1 [PR104513]

Message ID 20220214095435.GF2646553@tucnak
State New
Headers
Series c++: Don't reject GOTO_EXPRs to cdtor_label in potential_constant_expression_1 [PR104513] |

Commit Message

Jakub Jelinek Feb. 14, 2022, 9:54 a.m. UTC
  Hi!

return in ctors on targetm.cxx.cdtor_returns_this () target like arm
is emitted as GOTO_EXPR cdtor_label where at cdtor_label it emits
RETURN_EXPR with the this.
Similarly, in all dtors regardless of targetm.cxx.cdtor_returns_this ()
a return is emitted similarly.

potential_constant_expression_1 was rejecting these gotos and so we
incorrectly rejected these testcases, but actual cxx_eval* is apparently
handling these just fine.  I was a little bit worried that for the
destruction of bases we wouldn't evaluate something we should, but as the
testcase shows, that is evaluated through try ... finally and there is
nothing after the cdtor_label.  For arm there is RETURN_EXPR this; but we
don't really care about the return value from ctors and dtors during the
constexpr evaluation.

Bootstrapped/regtested on {x86_64,i686,aarch64,powerpc64le,s390x}-linux
and armv7hl-linux-gnueabi, ok for trunk?

I must say I don't see much the point of cdtor_labels at all, I'd think
that with try ... finally around it for non-arm we could just RETURN_EXPR
instead of the GOTO_EXPR and the try/finally gimplification would DTRT,
and we could just add the right return value for the arm case.

2022-02-14  Jakub Jelinek  <jakub@redhat.com>

	PR c++/104513
	* constexpr.cc (potential_constant_expression_1) <case GOTO_EXPR>:
	Don't punt if returns (target).

	* g++.dg/cpp1y/constexpr-104513.C: New test.
	* g++.dg/cpp2a/constexpr-dtor12.C: New test.


	Jakub
  

Comments

Jason Merrill Feb. 14, 2022, 3:48 p.m. UTC | #1
On 2/14/22 04:54, Jakub Jelinek wrote:
> 
> return in ctors on targetm.cxx.cdtor_returns_this () target like arm
> is emitted as GOTO_EXPR cdtor_label where at cdtor_label it emits
> RETURN_EXPR with the this.
> Similarly, in all dtors regardless of targetm.cxx.cdtor_returns_this ()
> a return is emitted similarly.
> 
> potential_constant_expression_1 was rejecting these gotos and so we
> incorrectly rejected these testcases, but actual cxx_eval* is apparently
> handling these just fine.  I was a little bit worried that for the
> destruction of bases we wouldn't evaluate something we should, but as the
> testcase shows, that is evaluated through try ... finally and there is
> nothing after the cdtor_label.  For arm there is RETURN_EXPR this; but we
> don't really care about the return value from ctors and dtors during the
> constexpr evaluation.
> 
> Bootstrapped/regtested on {x86_64,i686,aarch64,powerpc64le,s390x}-linux
> and armv7hl-linux-gnueabi, ok for trunk?

OK.

> I must say I don't see much the point of cdtor_labels at all, I'd think
> that with try ... finally around it for non-arm we could just RETURN_EXPR
> instead of the GOTO_EXPR and the try/finally gimplification would DTRT,
> and we could just add the right return value for the arm case.

Good point.  I suspect that it's a relic from before try/finally, seems 
like it should be simple to remove at this point.

> 2022-02-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/104513
> 	* constexpr.cc (potential_constant_expression_1) <case GOTO_EXPR>:
> 	Don't punt if returns (target).
> 
> 	* g++.dg/cpp1y/constexpr-104513.C: New test.
> 	* g++.dg/cpp2a/constexpr-dtor12.C: New test.
> 
> --- gcc/cp/constexpr.cc.jj	2022-02-11 13:52:32.697425776 +0100
> +++ gcc/cp/constexpr.cc	2022-02-12 13:51:21.000274390 +0100
> @@ -9364,7 +9364,7 @@ potential_constant_expression_1 (tree t,
>         {
>   	tree *target = &TREE_OPERAND (t, 0);
> - 	/* Gotos representing break and continue are OK.  */
> -	if (breaks (target) || continues (target))
> + 	/* Gotos representing break, continue and cdtor return are OK.  */
> +	if (breaks (target) || continues (target) || returns (target))
>   	  {
>   	    *jump_target = *target;
>   	    return true;
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-104513.C.jj	2022-02-12 19:05:14.374101202 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-104513.C	2022-02-12 19:06:09.745332658 +0100
> @@ -0,0 +1,10 @@
> +// PR c++/104513
> +// { dg-do compile { target c++14 } }
> +
> +struct A {
> +  int a1;
> +  short a2, a3;
> +  long a4;
> +  constexpr A() : a1(42), a2(42), a3(42), a4(42) { return; }
> +};
> +constexpr A a;
> --- gcc/testsuite/g++.dg/cpp2a/constexpr-dtor12.C.jj	2022-02-12 19:04:41.610555957 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor12.C	2022-02-12 19:04:18.473877087 +0100
> @@ -0,0 +1,13 @@
> +// PR c++/104513
> +// { dg-do compile { target c++20 } }
> +
> +struct S {
> +  constexpr S () : s (nullptr) {}
> +  constexpr ~S () { delete s; }
> +  int *s;
> +};
> +struct T : S {
> +  constexpr T () : S () {}
> +  constexpr ~T () { s = new int (42); return; }
> +};
> +constexpr T t;
> 
> 	Jakub
>
  

Patch

--- gcc/cp/constexpr.cc.jj	2022-02-11 13:52:32.697425776 +0100
+++ gcc/cp/constexpr.cc	2022-02-12 13:51:21.000274390 +0100
@@ -9364,7 +9364,7 @@  potential_constant_expression_1 (tree t,
       {
 	tree *target = &TREE_OPERAND (t, 0);
- 	/* Gotos representing break and continue are OK.  */
-	if (breaks (target) || continues (target))
+ 	/* Gotos representing break, continue and cdtor return are OK.  */
+	if (breaks (target) || continues (target) || returns (target))
 	  {
 	    *jump_target = *target;
 	    return true;
--- gcc/testsuite/g++.dg/cpp1y/constexpr-104513.C.jj	2022-02-12 19:05:14.374101202 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-104513.C	2022-02-12 19:06:09.745332658 +0100
@@ -0,0 +1,10 @@ 
+// PR c++/104513
+// { dg-do compile { target c++14 } }
+
+struct A {
+  int a1;
+  short a2, a3;
+  long a4;
+  constexpr A() : a1(42), a2(42), a3(42), a4(42) { return; }
+};
+constexpr A a;
--- gcc/testsuite/g++.dg/cpp2a/constexpr-dtor12.C.jj	2022-02-12 19:04:41.610555957 +0100
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor12.C	2022-02-12 19:04:18.473877087 +0100
@@ -0,0 +1,13 @@ 
+// PR c++/104513
+// { dg-do compile { target c++20 } }
+
+struct S {
+  constexpr S () : s (nullptr) {}
+  constexpr ~S () { delete s; }
+  int *s;
+};
+struct T : S {
+  constexpr T () : S () {}
+  constexpr ~T () { s = new int (42); return; }
+};
+constexpr T t;