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);
> > > > > > > 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.
> > > > > >
> > > > > > Right.
> > > > > >
> > > > > > We ought to be able to distinguish between uses of the RESULT_DECL
> > > > > > only
> > > > > > for
> > > > > > storing to it (cacheable) and any other use, either reading from it
> > > > > > or
> > > > > > messing
> > > > > > with its address. But I think that's a future direction, and we
> > > > > > should
> > > > > > stick
> > > > > > with your patch that almost works for GCC 10.
> > > > >
> > > > > Sounds good. Does the following then look OK to commit?
> > > > >
> > > > > -- >8 --
> > > > >
> > > > > Subject: [PATCH] c++: Stray RESULT_DECLs in result of constexpr call
> > > > > [PR94034]
> > > > >
> > > > > When evaluating the initializer of 'a' in the following example
> > > > >
> > > > > struct A {
> > > > > A() = default; A(const 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'.
> > > > >
> > > > > Fixing this properly by immediately resolving 'this' and
> > > > > PLACEHOLDER_EXPRs
> > > > > to
> > > > > the ultimate object under construction would in general mean that we
> > > > > would
> > > > > no
> > > > > longer be able to cache constexpr calls for which RVO possibly
> > > > > applies,
> > > > > because
> > > > > the result of the call may now depend on the ultimate object under
> > > > > construction.
> > > > >
> > > > > So as a mostly correct stopgap solution that retains cachability of
> > > > > RVO'd
> > > > > constexpr calls, this patch fixes this issue by rewriting all
> > > > > occurrences of
> > > > > the
> > > > > RESULT_DECL in the result of a constexpr function call with the
> > > > > current
> > > > > object
> > > > > under construction, after the call returns. This means the 'this'
> > > > > pointer
> > > > > during construction of the temporary will still point to the temporary
> > > > > object
> > > > > instead of the ultimate object, but besides that this approach seems
> > > > > functionally equivalent to the proper approach.
> > > > >
> > > > > Successfully bootstrapped and regtested on x86_64-pc-linux-gnu, does
> > > > > this
> > > > > look
> > > > > OK to commit?
> > > > >
> > > > > gcc/cp/ChangeLog:
> > > > >
> > > > > PR c++/94034
> > > > > * constexpr.c (replace_result_decl_data): New struct.
> > > > > (replace_result_decl_data_r): New function.
> > > > > (replace_result_decl): New function.
> > > > > (cxx_eval_call_expression): Use it.
> > > > > * tree.c (build_aggr_init_expr): Propagate the location of the
> > > > > initializer to the AGGR_INIT_EXPR.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > > PR c++/94034
> > > > > * g++.dg/cpp1y/constexpr-nsdmi6a.C: New test.
> > > > > * g++.dg/cpp1y/constexpr-nsdmi6b.C: New test.
> > > > > * g++.dg/cpp1y/constexpr-nsdmi7a.C: New test.
> > > > > * g++.dg/cpp1y/constexpr-nsdmi7b.C: New test.
> > > > > ---
> > > > > gcc/cp/constexpr.c | 51
> > > > > +++++++++++++++++++
> > > > > gcc/cp/tree.c | 3 ++
> > > > > .../g++.dg/cpp1y/constexpr-nsdmi6a.C | 26 ++++++++++
> > > > > .../g++.dg/cpp1y/constexpr-nsdmi6b.C | 27 ++++++++++
> > > > > .../g++.dg/cpp1y/constexpr-nsdmi7a.C | 49
> > > > > ++++++++++++++++++
> > > > > .../g++.dg/cpp1y/constexpr-nsdmi7b.C | 48
> > > > > +++++++++++++++++
> > > > > 6 files changed, 204 insertions(+)
> > > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C
> > > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C
> > > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C
> > > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
> > > > >
> > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > > > > index 5793430c88d..4cf5812e8a5 100644
> > > > > --- a/gcc/cp/constexpr.c
> > > > > +++ b/gcc/cp/constexpr.c
> > > > > @@ -2029,6 +2029,49 @@ cxx_eval_dynamic_cast_fn (const constexpr_ctx
> > > > > *ctx,
> > > > > tree call,
> > > > > return cp_build_addr_expr (obj, complain);
> > > > > }
> > > > > +/* Data structure used by replace_result_decl and
> > > > > replace_result_decl_r.
> > > > > */
> > > > > +
> > > > > +struct replace_result_decl_data
> > > > > +{
> > > > > + /* The RESULT_DECL we want to replace. */
> > > > > + tree decl;
> > > > > + /* The replacement for DECL. */
> > > > > + tree replacement;
> > > > > + /* Whether we've performed any replacements. */
> > > > > + bool changed;
> > > > > +};
> > > > > +
> > > > > +/* Helper function for replace_result_decl, called through
> > > > > cp_walk_tree.
> > > > > */
> > > > > +
> > > > > +static tree
> > > > > +replace_result_decl_r (tree *tp, int *walk_subtrees, void *data)
> > > > > +{
> > > > > + replace_result_decl_data *d = (replace_result_decl_data *) data;
> > > > > +
> > > > > + if (*tp == d->decl)
> > > > > + {
> > > > > + *tp = unshare_expr (d->replacement);
> > > > > + d->changed = true;
> > > > > + *walk_subtrees = 0;
> > > > > + }
> > > > > + else if (TYPE_P (*tp))
> > > > > + *walk_subtrees = 0;
> > > > > +
> > > > > + return NULL_TREE;
> > > > > +}
> > > > > +
> > > > > +/* Replace every occurrence of DECL, a RESULT_DECL, with (an unshared
> > > > > copy
> > > > > of)
> > > > > + REPLACEMENT within *TP. Returns true iff a replacement was
> > > > > performed.
> > > > > */
> > > > > +
> > > > > +static bool
> > > > > +replace_result_decl (tree *tp, tree decl, tree replacement)
> > > > > +{
> > > > > + gcc_assert (TREE_CODE (decl) == RESULT_DECL);
> > > > > + replace_result_decl_data data = { decl, replacement, false };
> > > > > + cp_walk_tree_without_duplicates (tp, replace_result_decl_r, &data);
> > > > > + return data.changed;
> > > > > +}
> > > > > +
> > > > > /* Subroutine of cxx_eval_constant_expression.
> > > > > Evaluate the call expression tree T in the context of OLD_CALL
> > > > > expression
> > > > > evaluation. */
> > > > > @@ -2536,6 +2579,14 @@ cxx_eval_call_expression (const constexpr_ctx
> > > > > *ctx,
> > > > > tree t,
> > > > > break;
> > > > > }
> > > > > }
> > > > > +
> > > > > + /* Rewrite all occurrences of the function's RESULT_DECL
> > > > > with the
> > > > > + current object under construction. */
> > > > > + if (ctx->object
> > > > > + && (same_type_ignoring_top_level_qualifiers_p
> > > > > + (TREE_TYPE (res), TREE_TYPE (ctx->object))))
> > > >
> > > > When can they have different types?
> > >
> > > During prior testing I tried replacing the same_type_p check with an
> > > assert,
> > > i.e.:
> > >
> > > if (ctx->object
> > > && AGGREGATE_TYPE_P (TREE_TYPE (res)))
> > > {
> > > gcc_assert (same_type_ignoring_top_level_qualifiers_p
> > > (TREE_TYPE (res), TREE_TYPE (ctx->object));
> > > ...
> > > }
> > >
> > > and IIRC I was able to trigger the assert only when the type of
> > > ctx->object and of the RESULT_DECL are distinct empty class types.
> > > Here's a testcase:
> > >
> > > struct empty1 { };
> > > constexpr empty1 foo1() { return {}; }
> > >
> > > struct empty2 { };
> > > constexpr empty2 foo2(empty1) { return {}; }
> > >
> > > constexpr empty2 a = foo2(foo1());
> > >
> > > The initializer of 'a' has the form
> > > TARGET_EXPR <D.2389, foo2 (foo1 ();, <<< Unknown tree:
> > > empty_class_expr
> > > >>>;)>;
> > > and so during evaluation of 'foo1()', ctx->object still points to 'a' and
> > > we
> > > trip over the same_type_p assert. (Is it possible that the call to foo1
> > > is missing a TARGET_EXPR?)
> >
> > Hmm, that would be because of
> >
> > > if (is_empty_class (TREE_TYPE (arg))
> > > && simple_empty_class_p (TREE_TYPE (arg), arg, INIT_EXPR))
> > > { > while (TREE_CODE (arg) == TARGET_EXPR)
> > > /* We're disconnecting the initializer from its target,
> > > don't create a temporary. */
> > > arg = TARGET_EXPR_INITIAL (arg);
> >
> > in build_call_a.
>
> Ah okay.
>
> >
> > > Either way, it would be safe to skip empty class types. Should we change
> > > the
> > > condition to
> > >
> > > if (ctx->object
> > > && AGGREGATE_TYPE_P (TREE_TYPE (res))
> > > && !is_empty_class (TREE_TYPE (res)))
> > > {
> > > ...
> > > }
> > >
> > > and turn the same_type_p check into an assert?
> >
> > Sounds good. Let's say a checking_assert.
>
> Great, I'm bootstrapping/regtesting the following overnight, does it
> look OK to commit if successful?
>
> -- >8 --
>
> Subject: [PATCH] c++: Stray RESULT_DECLs in result of constexpr call [PR94034]
>
> gcc/cp/ChangeLog:
>
> PR c++/94034
> * constexpr.c (replace_result_decl_data): New struct.
> (replace_result_decl_data_r): New function.
> (replace_result_decl): New function.
> (cxx_eval_call_expression): Use it.
> * tree.c (build_aggr_init_expr): Set the location of the AGGR_INIT_EXPR
> to that of its initializer.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/94034
> * g++.dg/cpp0x/constexpr-empty15.C: New test.
> * g++.dg/cpp1y/constexpr-nsdmi6a.C: New test.
> * g++.dg/cpp1y/constexpr-nsdmi6b.C: New test.
> * g++.dg/cpp1y/constexpr-nsdmi7a.C: New test.
> * g++.dg/cpp1y/constexpr-nsdmi7b.C: New test.
> ---
> gcc/cp/constexpr.c | 53 +++++++++++++++++++
> gcc/cp/tree.c | 3 ++
> .../g++.dg/cpp0x/constexpr-empty15.C | 9 ++++
> .../g++.dg/cpp1y/constexpr-nsdmi6a.C | 26 +++++++++
> .../g++.dg/cpp1y/constexpr-nsdmi6b.C | 27 ++++++++++
> .../g++.dg/cpp1y/constexpr-nsdmi7a.C | 49 +++++++++++++++++
> .../g++.dg/cpp1y/constexpr-nsdmi7b.C | 48 +++++++++++++++++
> 7 files changed, 215 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C
> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C
> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C
> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C
> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
>
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index d8636ddb92f..00e313178f9 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -2029,6 +2029,51 @@ cxx_eval_dynamic_cast_fn (const constexpr_ctx *ctx,
> tree call,
> return cp_build_addr_expr (obj, complain);
> }
>
> +/* Data structure used by replace_result_decl and replace_result_decl_r. */
> +
> +struct replace_result_decl_data
> +{
> + /* The RESULT_DECL we want to replace. */
> + tree decl;
> + /* The replacement for DECL. */
> + tree replacement;
> + /* Whether we've performed any replacements. */
> + bool changed;
> +};
> +
> +/* Helper function for replace_result_decl, called through cp_walk_tree. */
> +
> +static tree
> +replace_result_decl_r (tree *tp, int *walk_subtrees, void *data)
> +{
> + replace_result_decl_data *d = (replace_result_decl_data *) data;
> +
> + if (*tp == d->decl)
> + {
> + *tp = unshare_expr (d->replacement);
> + d->changed = true;
> + *walk_subtrees = 0;
> + }
> + else if (TYPE_P (*tp))
> + *walk_subtrees = 0;
> +
> + return NULL_TREE;
> +}
> +
> +/* Replace every occurrence of DECL, a RESULT_DECL, with (an unshared copy
> of)
> + REPLACEMENT within *TP. Returns true iff a replacement was performed. */
> +
> +static bool
> +replace_result_decl (tree *tp, tree decl, tree replacement)
> +{
> + gcc_checking_assert (TREE_CODE (decl) == RESULT_DECL
> + && (same_type_ignoring_top_level_qualifiers_p
> + (TREE_TYPE (decl), TREE_TYPE (replacement))));
> + replace_result_decl_data data = { decl, replacement, false };
> + cp_walk_tree_without_duplicates (tp, replace_result_decl_r, &data);
> + return data.changed;
> +}
> +
> /* Subroutine of cxx_eval_constant_expression.
> Evaluate the call expression tree T in the context of OLD_CALL expression
> evaluation. */
> @@ -2536,6 +2581,14 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> tree t,
> break;
> }
> }
> +
> + /* Rewrite all occurrences of the function's RESULT_DECL with the
> + current object under construction. */
> + if (ctx->object
> + && AGGREGATE_TYPE_P (TREE_TYPE (res))
> + && !is_empty_class (TREE_TYPE (res)))
> + if (replace_result_decl (&result, res, ctx->object))
> + cacheable = false;
We should probably require !*non_constant_p here before doing the
replacement because it doesn't make sense to do this transformation for
non-constant calls to constexpr functions. And I think it also might be
unsafe to mutate the result when !*non_constant_p because we might be
modifying part of the DECL_SAVED_TREE of the function in that case,
although I haven't been able to come up with a testcase for this.
And requiring !*non_constant_p means that the result must be a reduced
constant expression, which then allows replace_result_decl to make some
assumptions about where within the result it could possibly find a
RESULT_DECL, a minor optimization.
I am testing the following (ChangeLog is the same as before):
-- >8 --
---
gcc/cp/constexpr.c | 62 +++++++++++++++++++
gcc/cp/tree.c | 3 +
.../g++.dg/cpp0x/constexpr-empty15.C | 9 +++
.../g++.dg/cpp1y/constexpr-nsdmi6a.C | 26 ++++++++
.../g++.dg/cpp1y/constexpr-nsdmi6b.C | 27 ++++++++
.../g++.dg/cpp1y/constexpr-nsdmi7a.C | 49 +++++++++++++++
.../g++.dg/cpp1y/constexpr-nsdmi7b.C | 48 ++++++++++++++
7 files changed, 224 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index d8636ddb92f..387ee033214 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2029,6 +2029,59 @@ cxx_eval_dynamic_cast_fn (const constexpr_ctx *ctx, tree
call,
return cp_build_addr_expr (obj, complain);
}
+/* Data structure used by replace_result_decl and replace_result_decl_r. */
+
+struct replace_result_decl_data
+{
+ /* The RESULT_DECL we want to replace. */
+ tree decl;
+ /* The replacement for DECL. */
+ tree replacement;
+ /* Whether we've performed any replacements. */
+ bool changed;
+};
+
+/* Helper function for replace_result_decl, called through cp_walk_tree. */
+
+static tree
+replace_result_decl_r (tree *tp, int *walk_subtrees, void *data)
+{
+ replace_result_decl_data *d = (replace_result_decl_data *) data;
+
+ if (*tp == d->decl)
+ {
+ *tp = unshare_expr (d->replacement);
+ d->changed = true;
+ *walk_subtrees = 0;
+ }
+ else if (TYPE_P (*tp))
+ *walk_subtrees = 0;
+ else if (TREE_CODE (*tp) != ADDR_EXPR
+ && !POINTER_TYPE_P (TREE_TYPE (*tp))
+ && !AGGREGATE_TYPE_P (TREE_TYPE (*tp)))
+ /* Since *TP is part of a reduced constant expression, we know that a
+ RESULT_DECL can appear within it only as the operand of an ADDR_EXPR
+ within some aggregate. */
+ *walk_subtrees = 0;
+
+ return NULL_TREE;
+}
+
+/* Replace every occurrence of DECL, a RESULT_DECL, with (an unshared copy of)
+ REPLACEMENT within the reduced constant expression *TP. Returns true iff a
+ replacement was performed. */
+
+static bool
+replace_result_decl (tree *tp, tree decl, tree replacement)
+{
+ gcc_checking_assert (TREE_CODE (decl) == RESULT_DECL
+ && (same_type_ignoring_top_level_qualifiers_p
+ (TREE_TYPE (decl), TREE_TYPE (replacement))));
+ replace_result_decl_data data = { decl, replacement, false };
+ cp_walk_tree_without_duplicates (tp, replace_result_decl_r, &data);
+ return data.changed;
+}
+
/* Subroutine of cxx_eval_constant_expression.
Evaluate the call expression tree T in the context of OLD_CALL expression
evaluation. */
@@ -2536,6 +2589,15 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
break;
}
}
+
+ /* Rewrite all occurrences of the function's RESULT_DECL with the
+ current object under construction. */
+ if (!*non_constant_p
+ && ctx->object
+ && AGGREGATE_TYPE_P (TREE_TYPE (res))
+ && !is_empty_class (TREE_TYPE (res)))
+ if (replace_result_decl (&result, res, ctx->object))
+ cacheable = false;
}
else
/* Couldn't get a function copy to evaluate. */
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 1d311b0fe61..8e4934c0009 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -669,6 +669,9 @@ build_aggr_init_expr (tree type, tree init)
else
rval = init;
+ if (location_t loc = EXPR_LOCATION (init))
+ SET_EXPR_LOCATION (rval, loc);
+
return rval;
}
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C
new file mode 100644
index 00000000000..97863d4e1a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target c++11 } }
+
+struct empty1 { };
+constexpr empty1 foo1() { return {}; }
+
+struct empty2 { };
+constexpr empty2 foo2(empty1) { return {}; }
+
+constexpr empty2 a = foo2(foo1());
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C
new file mode 100644
index 00000000000..bb844b952e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C
@@ -0,0 +1,26 @@
+// PR c++/94034
+// { dg-do compile { target c++14 } }
+
+struct A {
+ A *ap = this;
+};
+
+constexpr A foo()
+{
+ return {};
+}
+
+constexpr A bar()
+{
+ return foo();
+}
+
+void
+baz()
+{
+ constexpr A a = foo(); // { dg-error ".A..& a... is not a constant
expression" }
+ constexpr A b = bar(); // { dg-error ".A..& b... is not a constant
expression" }
+}
+
+constexpr A a = foo();
+constexpr A b = bar();
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C
new file mode 100644
index 00000000000..f847fe9809f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C
@@ -0,0 +1,27 @@
+// PR c++/94034
+// { dg-do compile { target c++14 } }
+
+struct A {
+ A() = default; A(const A&);
+ A *ap = this;
+};
+
+constexpr A foo()
+{
+ return {};
+}
+
+constexpr A bar()
+{
+ return foo();
+}
+
+void
+baz()
+{
+ constexpr A a = foo(); // { dg-error ".A..& a... is not a constant
expression" }
+ constexpr A b = bar(); // { dg-error ".A..& b... is not a constant
expression" }
+}
+
+constexpr A a = foo();
+constexpr A b = bar();
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C
new file mode 100644
index 00000000000..5a40cd0b845
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C
@@ -0,0 +1,49 @@
+// PR c++/94034
+// { dg-do compile { target c++14 } }
+
+struct A
+{
+ A *p = this;
+ int n = 2;
+ int m = p->n++;
+};
+
+constexpr A
+foo()
+{
+ return {};
+}
+
+constexpr A
+bar()
+{
+ A a = foo();
+ a.p->n = 5;
+ return a;
+}
+
+static_assert(bar().n == 5, "");
+
+constexpr int
+baz()
+{
+ A b = foo();
+ b.p->n = 10;
+ A c = foo();
+ if (c.p->n != 3 || c.p->m != 2)
+ __builtin_abort();
+ bar();
+ return 0;
+}
+
+static_assert(baz() == 0, "");
+
+constexpr int
+quux()
+{
+ const A d = foo();
+ d.p->n++; // { dg-error "const object" }
+ return 0;
+}
+
+static_assert(quux() == 0, ""); // { dg-error "non-constant" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
new file mode 100644
index 00000000000..86d8ab4e759
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
@@ -0,0 +1,48 @@
+// PR c++/94034
+// { dg-do compile { target c++14 } }
+
+struct A
+{
+ A() = default; A(const A&);
+ A *p = this;
+ int n = 2;
+ int m = p->n++;
+};
+
+constexpr A
+foo()
+{
+ return {};
+}
+
+constexpr A
+bar()
+{
+ A a = foo();
+ a.p->n = 5;
+ return a; // { dg-error "non-.constexpr." }
+}
+
+constexpr int
+baz()
+{
+ A b = foo();
+ b.p->n = 10;
+ A c = foo();
+ if (c.p->n != 3 || c.p->m != 2)
+ __builtin_abort();
+ foo();
+ return 0;
+}
+
+static_assert(baz() == 0, "");
+
+constexpr int
+quux()
+{
+ const A d = foo();
+ d.p->n++; // { dg-error "const object" }
+ return 0;
+}
+
+static_assert(quux() == 0, ""); // { dg-error "non-constant" }
--
2.26.0.106.g9fadedd637
