On 4/10/20 2:15 PM, Patrick Palka wrote:
On Fri, 10 Apr 2020, Patrick Palka wrote:
On Fri, 10 Apr 2020, Jason Merrill wrote:
On 4/10/20 1:04 PM, Patrick Palka wrote:
On Thu, 9 Apr 2020, Patrick Palka wrote:
On Thu, 9 Apr 2020, Jason Merrill wrote:
On 4/8/20 7:49 PM, Patrick Palka wrote:
When evaluating the initializer of 'a' in the following example
struct A { A *p = this; };
constexpr A foo() { return {}; }
constexpr A a = foo();
the PLACEHOLDER_EXPR for 'this' in the aggregate initializer returned
by foo
gets resolved to the RESULT_DECL of foo. But due to guaranteed RVO,
the
'this'
should really be resolved to '&a'.
It seems to me that the right approach would be to immediately resolve
the
PLACEHOLDER_EXPR to the correct target object during evaluation of
'foo()',
so
that we could use 'this' to access objects adjacent to the current
object in
the
ultimate storage location. (I think #c2 of PR c++/94537 is an example
of
such
usage of 'this', which currently doesn't work. But as #c1 shows we
don't
seem
to handle this case correctly in non-constexpr initialization either.)
As I commented in the PR, the standard doesn't require this to work
because A
is trivially copyable, and our ABI makes it impossible. But there's
still a
constexpr bug when we add
A() = default; A(const A&);
clang doesn't require the constructors to make this work for constant
initialization, but similarly can't make it work for non-constant
initialization.
That makes a lot of sense, thanks for the detailed explanation.
I haven't yet been able to make a solution using the above approach
work --
making sure we use the ultimate object instead of the RESULT_DECL
whenever
we
access ctx->global->values is proving to be tricky and subtle.
Do we need to go through ctx->global->values? Would it work for the
RESULT_DECL case in cxx_eval_constant_expression to go to straight to
ctx->object or ctx->ctor instead?
I attempted that at some point, but IIRC we still end up not resolving
some RESULT_DECLs because not all of them get processed through
cxx_eval_constant_expression before we manipulate ctx->global->values
with them. I'll try this approach more carefully and report back with
more specifics.
It turns out that immediately resolving RESULT_DECLs/'this' to the
ultimate ctx->object would not interact well with constexpr_call
caching:
struct A { A() = default; A(const A&); A *ap = this; };
constexpr A foo() { return {}; }
constexpr A a = foo();
constexpr A b = foo();
static_assert(b.ap == &b); // fails
Evaluation of the first call to foo() returns {&a}, since we resolve
'this' to &a due to guaranteed RVO, and we cache this result.
Evaluation of the second call to foo() just returns the cached result
from the constexpr_call cache, and so we get {&a} again.
So it seems we would have to mark the result of a constexpr call as not
cacheable whenever this RVO applies during its evaluation, even when
doing the RVO has no observable difference in the final result (i.e. the
constructor does not try to save the 'this' pointer).
Would the performance impact of disabling caching whenever RVO applies
during constexpr call evaluation be worth it, or should we go with
something like my first patch which "almost works," and which marks a
constexpr call as not cacheable only when we replace a RESULT_DECL in
the result of the call?
Could we search through the result of the call for ctx->object and cache if we
don't find it?
Hmm, I think the result of the call could still depend on ctx->object
without ctx->object explicitly appearing in the result. Consider the
following testcase:
struct A {
A() = default; A(const A&);
constexpr A(const A *q) : d{this - p} { }
Oops sorry, that 'q' should be a 'p'.
long d = 0;
};
constexpr A baz(const A *q) { return A(p); };
And same here.
constexpr A a = baz(&a);
constexpr A b = baz(&a); // no error
The initialization of 'b' should be ill-formed, but the result of the
first call to baz(&a) would be {0}, so we would cache it and then reuse
the result when initializing 'b'.
Ah, true. Can we still cache if we're initializing something that isn't
TREE_STATIC?
Jason
