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?

Reply via email to