Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On Tue, 14 Apr 2020, Jason Merrill wrote:
> On 4/14/20 9:01 AM, Patrick Palka wrote:
> > On Tue, 14 Apr 2020, Patrick Palka wrote:
> >
> > > On Mon, 13 Apr 2020, Jason Merrill wrote:
> > >
> > > > On 4/13/20 2:49 PM, Patrick Palka wrote:
> > > > > On Mon, 13 Apr 2020, Jason Merrill wrote:
> > > > >
> > > > > > On 4/12/20 9:43 AM, Patrick Palka wrote:
> > > > > > > On Sat, 11 Apr 2020, Jason Merrill wrote:
> > > > > > >
> > > > > > > > On 4/10/20 5:47 PM, Patrick Palka wrote:
> > > > > > > > > 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.
> > >
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On 4/14/20 9:01 AM, Patrick Palka wrote:
On Tue, 14 Apr 2020, Patrick Palka wrote:
On Mon, 13 Apr 2020, Jason Merrill wrote:
On 4/13/20 2:49 PM, Patrick Palka wrote:
On Mon, 13 Apr 2020, Jason Merrill wrote:
On 4/12/20 9:43 AM, Patrick Palka wrote:
On Sat, 11 Apr 2020, Jason Merrill wrote:
On 4/10/20 5:47 PM, Patrick Palka wrote:
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);
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On 4/14/20 12:29 AM, Patrick Palka wrote:
On Mon, 13 Apr 2020, Jason Merrill wrote:
On 4/13/20 2:49 PM, Patrick Palka wrote:
On Mon, 13 Apr 2020, Jason Merrill wrote:
On 4/12/20 9:43 AM, Patrick Palka wrote:
On Sat, 11 Apr 2020, Jason Merrill wrote:
On 4/10/20 5:47 PM, Patrick Palka wrote:
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);
s
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On Tue, 14 Apr 2020, Patrick Palka wrote:
> On Mon, 13 Apr 2020, Jason Merrill wrote:
>
> > On 4/13/20 2:49 PM, Patrick Palka wrote:
> > > On Mon, 13 Apr 2020, Jason Merrill wrote:
> > >
> > > > On 4/12/20 9:43 AM, Patrick Palka wrote:
> > > > > On Sat, 11 Apr 2020, Jason Merrill wrote:
> > > > >
> > > > > > On 4/10/20 5:47 PM, Patrick Palka wrote:
> > > > > > > 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
> > > > > > > > > > > > > > > whene
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On Mon, 13 Apr 2020, Jason Merrill wrote:
> On 4/13/20 2:49 PM, Patrick Palka wrote:
> > On Mon, 13 Apr 2020, Jason Merrill wrote:
> >
> > > On 4/12/20 9:43 AM, Patrick Palka wrote:
> > > > On Sat, 11 Apr 2020, Jason Merrill wrote:
> > > >
> > > > > On 4/10/20 5:47 PM, Patrick Palka wrote:
> > > > > > 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
> > > > > > > > > > > > > RES
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On 4/13/20 2:49 PM, Patrick Palka wrote:
On Mon, 13 Apr 2020, Jason Merrill wrote:
On 4/12/20 9:43 AM, Patrick Palka wrote:
On Sat, 11 Apr 2020, Jason Merrill wrote:
On 4/10/20 5:47 PM, Patrick Palka wrote:
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.
Unfor
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On Mon, 13 Apr 2020, Jason Merrill wrote:
> On 4/12/20 9:43 AM, Patrick Palka wrote:
> > On Sat, 11 Apr 2020, Jason Merrill wrote:
> >
> > > On 4/10/20 5:47 PM, Patrick Palka wrote:
> > > > 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 reso
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On 4/12/20 9:43 AM, Patrick Palka wrote:
On Sat, 11 Apr 2020, Jason Merrill wrote:
On 4/10/20 5:47 PM, Patrick Palka wrote:
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
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On Sat, 11 Apr 2020, Jason Merrill wrote:
> On 4/10/20 5:47 PM, Patrick Palka wrote:
> > 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
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
On 4/10/20 5:47 PM, Patrick Palka wrote:
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} {
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
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 o
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
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
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
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 initial
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
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} { }
long d = 0;
};
constexpr A baz(const A *q) { return A(p); };
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'.
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
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?
Jason
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
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?
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
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.
Re: [PATCH] c++: Stray RESULT_DECLs in result of constexpr function call [PR94034]
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.
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?
Jason
