Re: C++ PATCH to implement deferred parsing of noexcept-specifiers (c++/86476, c++/52869)

[Jason Merrill <jason at redhat dot com>]

On 5/10/19 3:21 PM, Marek Polacek wrote:
> Coming back to this.  I didn't think this was suitable for GCC 9.
>
> On Mon, Jan 07, 2019 at 10:44:37AM -0500, Jason Merrill wrote:
> > On 12/19/18 3:27 PM, Marek Polacek wrote:
> > > Prompted by Jon's observation in 52869, I noticed that we don't treat
> > > a noexcept-specifier as a complete-class context of a class ([class.mem]/6).
> > > As with member function bodies, default arguments, and NSDMIs, names used in
> > > a noexcept-specifier of a member-function can be declared later in the class
> > > body, so we need to wait and parse them at the end of the class.
> > > For that, I've made use of DEFAULT_ARG (now best to be renamed to UNPARSED_ARG).
> >
> > Or DEFERRED_PARSE, yes.
>
> I didn't change the name but I'm happy to do it as a follow up.
>
> > > +  /* We can't compare unparsed noexcept-specifiers.  Save the old decl
> > > +     and check this again after we've parsed the noexcept-specifiers
> > > +     for real.  */
> > > +  if (UNPARSED_NOEXCEPT_SPEC_P (new_exceptions))
> > > +    {
> > > +      vec_safe_push (DEFARG_INSTANTIATIONS (TREE_PURPOSE (new_exceptions)),
> > > +		     copy_decl (old_decl));
> > > +      return;
> > > +    }
> >
> > Why copy_decl?
>
> This is so that we don't lose the diagnostic in noexcept46.C.  If I don't use
> copy_decl then the tree is shared and subsequent changes to it make us not
> detect discrepancies like noexcept(false) vs. noexcept(true) on the same decl.

OK, fair enough.

> @@ -20515,7 +20524,13 @@ cp_parser_init_declarator (cp_parser* parser,
>   			/*asmspec=*/NULL_TREE,
>   			attr_chainon (attributes, prefix_attributes));
>         if (decl && TREE_CODE (decl) == FUNCTION_DECL)
> -	cp_parser_save_default_args (parser, decl);
> +	{
> +	  cp_parser_save_default_args (parser, decl);
> +	  /* Remember if there is a noexcept-specifier to post process.  */
> +	  tree spec = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (decl));
> +	  if (UNPARSED_NOEXCEPT_SPEC_P (spec))
> +	    vec_safe_push (unparsed_noexcepts, decl);

Can we handle this in cp_parser_save_default_args rather than all its 
callers?

> +/* Make sure that any member-function parameters are in scope.
> +   For instance, a function's noexcept-specifier can use the function's
> +   parameters:
> +
> +   struct S {
> +     void fn (int p) noexcept(noexcept(p));
> +   };
> +
> +   so we need to make sure name lookup can find them.  This is used
> +   when we delay parsing of the noexcept-specifier.  */
> +
> +static void
> +inject_parm_decls (tree decl)
> +{
> +  begin_scope (sk_function_parms, decl);
> +  tree args = DECL_ARGUMENTS (decl);
> +  args = nreverse (args);
> +
> +  tree next;
> +  for (tree parm = args; parm; parm = next)
> +    {
> +      next = DECL_CHAIN (parm);
> +      if (TREE_CODE (parm) == PARM_DECL)
> +	pushdecl (parm);
> +    }
> +  /* Get the decls in their original chain order and record in the
> +     function.  This is all and only the PARM_DECLs that were
> +     pushed into scope by the loop above.  */
> +  DECL_ARGUMENTS (decl) = get_local_decls ();
> +}

Can we share this code with store_parm_decls instead of having two copies?

> @@ -25227,6 +25431,18 @@ cp_parser_noexcept_specification_opt (cp_parser* parser,
> +      /* [class.mem]/6 says that a noexcept-specifer (within the
> +	 member-specification of the class) is a complete-class context of
> +	 a class.  So, if the noexcept-specifier has the optional expression,
> +	 just save the tokens, and reparse this after we're done with the
> +	 class.  */
> +      if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_OPEN_PAREN
> +	  && at_class_scope_p ()
> +	  && TYPE_BEING_DEFINED (current_class_type)
> +	  && !LAMBDA_TYPE_P (current_class_type))
> +	return cp_parser_save_noexcept (parser);

We might optimize the noexcept(<literal>) case, which should be pretty 
common.

> +maybe_check_throw_specifier (tree overrider, tree basefn)

maybe_check_overriding_exception_spec

> diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
> index df002a1664c..8cbc48fb44f 100644
> --- gcc/cp/typeck2.c
> +++ gcc/cp/typeck2.c
> @@ -2393,6 +2393,12 @@ merge_exception_specifiers (tree list, tree add)
>     if (list == error_mark_node || add == error_mark_node)
>       return error_mark_node;
>   
> +  /* We don't want to lose the unparsed operand lest we miss diagnostics.  */
> +  if (UNPARSED_NOEXCEPT_SPEC_P (list))
> +    return list;
> +  else if (UNPARSED_NOEXCEPT_SPEC_P (add))
> +    return add;

Here you're throwing away the other side, which seems wrong.

Jason