Re: C++ PATCH to fix ICE in replace_placeholders_r (PR c++/79937)

[Marek Polacek <polacek at redhat dot com>]

On Tue, Mar 14, 2017 at 02:34:30PM -0400, Jason Merrill wrote:
> On Tue, Mar 14, 2017 at 2:33 PM, Jason Merrill <jason@redhat.com> wrote:
> > On Tue, Mar 7, 2017 at 12:10 PM, Marek Polacek <polacek@redhat.com> wrote:
> >> In this testcase we have
> >> C c = bar (X{1});
> >> which store_init_value sees as
> >> c = TARGET_EXPR <D.2332, bar (TARGET_EXPR <D.2298, {.i=1, .n=(&<PLACEHOLDER_EXPR struct X>)->i}>)>
> >> i.e. we're initializing "c" with a TARGET_EXPR.  We call replace_placeholders
> >> that walks the whole tree to substitute the placeholders.  Eventually we find
> >> the nested <PLACEHOLDER_EXPR struct X> but that's for another object, so we
> >> crash.  Seems that we shouldn't have stepped into the second TARGET_EXPR at
> >> all; it has nothing to with "c", it's bar's argument.
> >>
> >> It occurred to me that we shouldn't step into CALL_EXPRs and leave the
> >> placeholders in function arguments to cp_gimplify_init_expr which calls
> >> replace_placeholders for constructors.  Not sure if it's enough to handle
> >> CALL_EXPRs like this, anything else?
> >
> > Hmm, we might have a DMI containing a call with an argument referring
> > to *this, i.e.
> >
> > struct A
> > {
> >   int i;
> >   int j = frob (this->i);
> > };
> >
> > The TARGET_EXPR seems like a more likely barrier, but even there we
> > could have something like
> >
> > struct A { int i; };
> > struct B
> > {
> >   int i;
> >   A a = A{this->i};
> > };
> >
> > I think we need replace_placeholders to keep a stack of objects, so
> > that when we see a TARGET_EXPR we add it to the stack and therefore
> > can properly replace a PLACEHOLDER_EXPR of its type.
> 
> Or actually, avoid replacing such a PLACEHOLDER_EXPR, but rather leave
> it for later when we lower the TARGET_EXPR.

Sorry, I don't really follow.  I have a patch that puts TARGET_EXPRs on
a stack, but I don't know how that helps.  E.g. with nsdmi-aggr3.C
we have
B b = TARGET_EXPR <D1, {.a = TARGET_EXPR <D2, (struct A *) &<PLACEHOLDER_EXPR struct B>>}>
so when we get to that PLACEHOLDER_EXPR, on the stack there's
TARGET_EXPR with type struct A
TARGET_EXPR with type struct B
so the type of the PLACEHOLDER_EXPR doesn't match the type of the current
TARGET_EXPR, but we still want to replace it in this case.

So -- could you expand a bit on what you had in mind, please?

Thanks,

	Marek