On 1/15/25 10:13 AM, Patrick Palka wrote:
On Mon, 1 May 2023, Jason Merrill wrote:
On 4/28/23 15:40, Patrick Palka wrote:
On Fri, 28 Apr 2023, Patrick Palka wrote:
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 :(
Ah, the problem was that later in cxx_eval_store_expression we were
suppressing a TREE_READONLY update via pattern matching on 'target',
but if we are now updating 'target' to its reduced value the pattern
matching needs to consider the shape of the original 'target' instead.
Here's an alternative fix for this PR that passes regression testing,
not sure which approach would be preferable.
With this patch I think we'd still miss the case where the target is a
subobject (so probe != target).
And we would miss clearing cacheable in the case where we the value depends on
ctx->object. OTOH, that seems like it might already be an issue when we go
through here:
if (lval == vc_glvalue)
{
/* If we want to return a reference to the target, we need to evaluate
it as a whole;
otherwise, only evaluate the innermost piece to avoid
building up unnecessary *_REFs. */
target = cxx_eval_constant_expression (ctx, target, lval,
non_constant_p, overflow_p);
So I think it makes sense to try to avoid the need for the replace_decl at all
by doing the replacement earlier along the lines of this patch, and replace
the replace_decl with looking for references to ctx->object in the result?
Sorry, I lost track of this patch...
That does seem better but admittedly tricky to get right. Would the
original approach (i.e. the second patch in this thread, that just
re-evaluates the result after doing replace_decl) be OK to go in for
GCC 15? While not ideal, it seems like a safe improvement to our
current mechanism.
OK.
Jason
