On Fri, Feb 22, 2019 at 1:17 PM Marek Polacek <pola...@redhat.com> wrote:
> On Thu, Feb 21, 2019 at 02:57:28PM -1000, Jason Merrill wrote:
> > On 2/21/19 1:34 PM, Marek Polacek wrote:
> > > On Thu, Feb 21, 2019 at 11:22:41AM -1000, Jason Merrill wrote:
> > > > On 2/21/19 10:56 AM, Marek Polacek wrote:
> > > > > On Wed, Feb 20, 2019 at 01:53:18PM -1000, Jason Merrill wrote:
> > > > > > On 2/20/19 10:31 AM, Marek Polacek wrote:
> > > > > > > Here we ICE when substituting a deferred noexcept-specifier,
> > > > > > > because it
> > > > > > > contains 'this', a PARM_DECL, in an evaluated context. This is
> > > > > > > different
> > > > > > > from "noexcept(noexcept(this))" where the noexcept operator's
> > > > > > > operand is
> > > > > > > an unevaluated operand. We crash within tsubst_copy's PARM_DECL
> > > > > > > handling
> > > > > > > of a 'this' PARM_DECL:
> > > > > > > 15488 gcc_assert (cp_unevaluated_operand != 0)
> > > > > > > It'd be wrong to mess with cp_unevaluated_operand (or
> > > > > > > current_class_*), and
> > > > > > > since we only check the expression's constness after substituting
> > > > > > > in
> > > > > > > maybe_instantiate_noexcept, one fix would be the following.
> > > > > > >
> > > > > > > This is not just about 'this', as the 87844 test shows: here we
> > > > > > > also have
> > > > > > > a parameter whose value we're trying to determine -- it's a
> > > > > > > template arg
> > > > > > > of a late-specified return type. Returning the original value in
> > > > > > > tsubst_copy
> > > > > > > and leaving the later code to complain seems to work here as
> > > > > > > well. Just
> > > > > > > removing the assert should work as well.
> > > > > >
> > > > > > I'm reluctant to mess with this assert, as it catches a lot of
> > > > > > lambda bugs.
> > > > >
> > > > > Makes sense -- I wasn't aware of that.
> > > > >
> > > > > > I think we want to register_parameter_specializations when
> > > > > > instantiating
> > > > > > deferred noexcept, so that tsubst_copy can find the right decls.
> > > > >
> > > > > Thanks for the suggestion -- that works with one catch: we need to
> > > > > replace the
> > > > > fake 'this' in the noexcept- specifier with a real 'this' (the
> > > > > template one),
> > > > > one that has DECL_CONTEXT set. The fake one comes from
> > > > > inject_this_parameter,
> > > > > when we were parsing the noexcept-specifier. The real one was set in
> > > > > grokfndecl.
> > > >
> > > > If you set current_class_ptr appropriately tsubst_copy will use it, so
> > > > you
> > > > shouldn't need to do a walk_tree.
> > >
> > > Unfortunately that broke a lot of libstdc++ tests. I can poke at it more
> > > if
> > > you feel stronly about it.
> >
> > Please do poke a bit more, that surprises me.
>
> Apparently *someone* needs to learn how to properly restore c_c_{ptr,ref}...
>
> noexcept35.C is a new test reduced from something from libstdc++. Since it
> exercises codepaths that nothing in dg.exp does, I think it's worth adding.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2019-02-22 Marek Polacek <pola...@redhat.com>
>
> PR c++/88294 - ICE with non-constant noexcept-specifier.
> * pt.c (maybe_instantiate_noexcept): Set up the list of local
> specializations. Set current_class_{ptr,ref}.
OK, thanks.
Jason
