Re: [PATCH] c++: RESULT_DECL replacement in constexpr call result [PR105440]

[Patrick Palka <ppalka@redhat.com>]

On Fri, 28 Apr 2023, Patrick Palka wrote:

> After mechanically replacing RESULT_DECL within a constexpr call result
> (for sake of RVO), we can in some cases simplify the call result
> further.
> 
> In the below testcase the result of get() during evaluation of a's
> initializer is the self-referential CONSTRUCTOR:
> 
>   {._M_p=(char *) &<retval>._M_local_buf}
> 
> which after replacing RESULT_DECL with ctx->object (aka *D.2603, where
> the D.2603 temporary points to the current element of _M_elems under
> construction) becomes:
> 
>   {._M_p=(char *) &D.2603->_M_local_buf}
> 
> but what we really want is:
> 
>   {._M_p=(char *) &a._M_elems[0]._M_local_buf}.
> 
> so that the value of _M_p is independent of the value of the mutable
> D.2603 temporary.
> 
> So to that end, it seems we should constexpr evaluate the result again
> after RESULT_DECL replacement, which is what this patch implements.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR libstdc++/105440
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (cxx_eval_call_expression): If any RESULT_DECLs get
> 	replaced in the call result, try further evaluating the result.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/constexpr-dtor16.C: New test.
> ---
>  gcc/cp/constexpr.cc                           | 12 +++++-
>  gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C | 39 +++++++++++++++++++
>  2 files changed, 49 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index d1097764b10..22a1609e664 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -3213,7 +3213,12 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
>  		&& CLASS_TYPE_P (TREE_TYPE (res))
>  		&& !is_empty_class (TREE_TYPE (res)))
>  	      if (replace_decl (&result, res, ctx->object))
> -		cacheable = false;
> +		{
> +		  cacheable = false;
> +		  result = cxx_eval_constant_expression (ctx, result, lval,
> +							 non_constant_p,
> +							 overflow_p);
> +		}
>  	}
>        else
>  	/* Couldn't get a function copy to evaluate.  */
> @@ -5988,9 +5993,12 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>  	    object = probe;
>  	  else
>  	    {
> +	      tree orig_probe = probe;
>  	      probe = cxx_eval_constant_expression (ctx, probe, vc_glvalue,
>  						    non_constant_p, overflow_p);
>  	      evaluated = true;
> +	      if (orig_probe == target)
> +		target = probe;

Whoops, thanks to an accidental git commit --amend this patch contains
an alternative approach that I considered: in cxx_eval_store_expression,
ensure that we always set ctx->object to a fully reduced result (so
&a._M_elems[0] instead of of *D.2603 in this case), which means later
RESULT_DECL replacement with ctx->object should yield an already reduced
result as well.  But with this approach I ran into a bogus "modifying
const object" error on cpp1y/constexpr-tracking-const23.C so I gave up
on it :(

Here's the correct patch:

	PR libstdc++/105440

gcc/cp/ChangeLog:

	* constexpr.cc (cxx_eval_call_expression): If any RESULT_DECLs get
	replaced in the call result, try further evaluating the result.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/constexpr-dtor16.C: New test.
---
 gcc/cp/constexpr.cc                           |  7 +++-
 gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C | 39 +++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index d1097764b10..9dbbf6eec03 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3213,7 +3213,12 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 		&& CLASS_TYPE_P (TREE_TYPE (res))
 		&& !is_empty_class (TREE_TYPE (res)))
 	      if (replace_decl (&result, res, ctx->object))
-		cacheable = false;
+		{
+		  cacheable = false;
+		  result = cxx_eval_constant_expression (ctx, result, lval,
+							 non_constant_p,
+							 overflow_p);
+		}
 	}
       else
 	/* Couldn't get a function copy to evaluate.  */
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C
new file mode 100644
index 00000000000..707a3e025b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C
@@ -0,0 +1,39 @@
+// PR c++/105440
+// { dg-do compile { target c++20 } }
+
+struct basic_string {
+  char _M_local_buf[32];
+  char* _M_p;
+  constexpr basic_string() : _M_p{_M_local_buf} { }
+  constexpr void f() { if (_M_p) { } }
+  constexpr ~basic_string() { if (_M_p) { } }
+};
+
+template<int N>
+struct array {
+  basic_string _M_elems[N];
+};
+
+constexpr basic_string get() { return {}; }
+
+constexpr bool f1() {
+  array<1> a{get()};
+  a._M_elems[0].f();
+
+  return true;
+}
+
+constexpr bool f2() {
+  array<2> a2{get(), get()};
+  array<3> a3{get(), get(), get()};
+
+  for (basic_string& e : a2._M_elems)
+    e.f();
+  for (basic_string& e : a3._M_elems)
+    e.f();
+
+  return true;
+}
+
+static_assert(f1());
+static_assert(f2());
-- 
2.40.1.445.gf85cd430b1


>  	      if (*non_constant_p)
>  		return t;
>  	    }
> @@ -6154,7 +6162,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>    if (!empty_base && !(same_type_ignoring_top_level_qualifiers_p
>  		       (initialized_type (init), type)))
>      {
> -      gcc_assert (is_empty_class (TREE_TYPE (target)));
> +      gcc_assert (is_empty_class (TREE_TYPE (TREE_OPERAND (t, 0))));
>        empty_base = true;
>      }
>  
> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C
> new file mode 100644
> index 00000000000..707a3e025b1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C
> @@ -0,0 +1,39 @@
> +// PR c++/105440
> +// { dg-do compile { target c++20 } }
> +
> +struct basic_string {
> +  char _M_local_buf[32];
> +  char* _M_p;
> +  constexpr basic_string() : _M_p{_M_local_buf} { }
> +  constexpr void f() { if (_M_p) { } }
> +  constexpr ~basic_string() { if (_M_p) { } }
> +};
> +
> +template<int N>
> +struct array {
> +  basic_string _M_elems[N];
> +};
> +
> +constexpr basic_string get() { return {}; }
> +
> +constexpr bool f1() {
> +  array<1> a{get()};
> +  a._M_elems[0].f();
> +
> +  return true;
> +}
> +
> +constexpr bool f2() {
> +  array<2> a2{get(), get()};
> +  array<3> a3{get(), get(), get()};
> +
> +  for (basic_string& e : a2._M_elems)
> +    e.f();
> +  for (basic_string& e : a3._M_elems)
> +    e.f();
> +
> +  return true;
> +}
> +
> +static_assert(f1());
> +static_assert(f2());
> -- 
> 2.40.1.445.gf85cd430b1
> 
>