On Fri, Feb 22, 2019 at 1:17 PM Marek Polacek <[email protected]> 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 <[email protected]> > > 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
