On Fri, 10 Apr 2020, Jason Merrill wrote:
> 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?
Hmm, we correctly compile the analogous non-TREE_STATIC testcase
struct A {
A() = default; A(const A&);
constexpr A(const A *p) : d{this == p} { }
bool d;
};
constexpr A baz(const A *p) { return A(p); }
void foo() {
constexpr A x = baz(&x);
constexpr A y = baz(&x);
static_assert(!y.d);
}
because the calls 'baz(&x)' are considered to have non-constant arguments.
Unfortunately, we can come up with another trick to fool the constexpr_call
cache in the presence of RVO even in the !TREE_STATIC case:
struct B {
B() = default; B(const B&);
constexpr B(int) : q{!this[-1].q} { }
bool q = false;
};
constexpr B baz() { return B(0); }
void foo()
{
constexpr B d[2] = { {}, baz() };
constexpr B e = baz();
}
The initialization of the local variable 'e' should be invalid, but if we
cached the first call to 'baz' then we would wrongly accept it.
