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?
PR c++/105440
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_store_expression): Save the original
target in 'orig_target'. Update 'target' after evaluating it in
the 'probe' loop. Use 'orig_target' instead of 'target' when
appropriate.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/constexpr-dtor16.C: New test.
---
gcc/cp/constexpr.cc | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9dbbf6eec03..2939ac89a98 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -5902,8 +5902,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
tree t,
/* Just ignore clobbers. */
return void_node;
+ const tree orig_target = TREE_OPERAND (t, 0);
+
/* First we figure out where we're storing to. */
- tree target = TREE_OPERAND (t, 0);
+ tree target = orig_target;
maybe_simplify_trivial_copy (target, init);
@@ -5993,9 +5995,12 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
object = probe;
else
{
+ bool is_target = (probe == target);
probe = cxx_eval_constant_expression (ctx, probe, vc_glvalue,
non_constant_p, overflow_p);
evaluated = true;
+ if (is_target)
+ target = probe;
if (*non_constant_p)
return t;
}
@@ -6159,7 +6164,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 (orig_target)));
empty_base = true;
}
@@ -6294,14 +6299,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
&& TREE_CODE (*valp) == CONSTRUCTOR
&& TYPE_READONLY (type))
{
- if (INDIRECT_REF_P (target)
+ if (INDIRECT_REF_P (orig_target)
&& (is_this_parameter
- (tree_strip_nop_conversions (TREE_OPERAND (target, 0)))))
+ (tree_strip_nop_conversions (TREE_OPERAND (orig_target, 0)))))
/* We've just initialized '*this' (perhaps via the target
constructor of a delegating constructor). Leave it up to the
caller that set 'this' to set TREE_READONLY appropriately. */
gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p
- (TREE_TYPE (target), type) || empty_base);
+ (TREE_TYPE (orig_target), type) || empty_base);
else
TREE_READONLY (*valp) = 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());
