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

[Jason Merrill <jason@redhat.com>]

On 6/21/19 5:29 PM, Marek Polacek wrote:
> On Fri, Jun 21, 2019 at 04:47:46PM -0400, Jason Merrill wrote:
>> On 6/14/19 4:54 PM, Marek Polacek wrote:
>>> On Tue, Jun 11, 2019 at 11:46:05PM -0400, Jason Merrill wrote:
>>>> On 6/3/19 9:01 PM, Marek Polacek wrote:
>>>>
>>>>> I sort of ended up going down a rathole, but then I realized we don't need to
>>>>> delay parsing of noexcept-specifiers of member friend function declarations,
>>>>> because they aren't members of the class.
>>>>
>>>> Where are you getting this from?  I'm definitely sympathetic to the idea
>>>> that noexcept-specifiers of friend functions shouldn't need to be
>>>> complete-class contexts, but 10.3 doesn't make that distinction that I can
>>>> see.
>>>
>>> When I tested my patch I noticed that none of the 3 compilers I tried handled
>>> this scenario, so I thought I was missing something.  But if the standard
>>> really doesn't say that noexcept-specifiers of friend functions don't have to
>>> be complete-class contexts, then perhaps it needs to say so.  Should I raise
>>> this on the reflector?
>>
>> Sounds good.
> 
> Will do.
> 
>>>>> This was a huge relief because
>>>>> member friend function declarations can be redeclared, so we'd have to make
>>>>> sure to check if their noexcept-specifiers match.  But member function decls
>>>>> can't be redeclared.  I updated the comment to better reflect why what I'm
>>>>> doing there is correct, along with an assert.
>>>>
>>>> But then why do you still need this:
>>>>
>>>>> +  /* We can't compare unparsed noexcept-specifiers.  Save the decl
>>>>> +     and check this again after we've parsed the noexcept-specifiers
>>>>> +     for real.  */
>>>>> +  if (UNPARSED_NOEXCEPT_SPEC_P (new_exceptions))
>>>>> +    {
>>>>> +      DEFARG_DECL (TREE_PURPOSE (new_exceptions)) = copy_decl (old_decl);
>>>>> +      return;
>>>>> +    }
>>>>
>>>> ?
>>>
>>> Eh... I don't.  The following version is with the DEFARG_DECL junk removed.
>>>
>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>>
>>> 2019-06-14  Marek Polacek  <polacek@redhat.com>
>>>
>>> 	PR c++/86476 - noexcept-specifier is a complete-class context.
>>> 	PR c++/52869
>>> 	* cp-tree.def (DEFAULT_ARG): Update commentary.
>>
>> I'd still like to rename this, can you do that in a follow-up?
> 
> Absolutely.
> 
>>> @@ -25203,6 +25379,26 @@ cp_parser_noexcept_specification_opt (cp_parser* parser,
>>>      if (cp_parser_is_keyword (token, RID_NOEXCEPT))
>>>        {
>>>          tree expr;
>>> +
>>> +      /* [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_nth_token_is (parser->lexer, 2, CPP_OPEN_PAREN)
>>> +	  /* No need to delay parsing for a number literal or true/false.  */
>>> +	  && !cp_lexer_nth_token_is (parser->lexer, 3, CPP_NUMBER)
>>> +	  && !(cp_lexer_nth_token_is (parser->lexer, 3, CPP_KEYWORD)
>>> +	       && (cp_lexer_nth_token_is_keyword (parser->lexer, 3, RID_FALSE)
>>> +		   || cp_lexer_nth_token_is_keyword (parser->lexer, 3,
>>> +						     RID_TRUE)))
>>
>> Maybe do immediate parsing for any keyword, not just true/false?  I can't
>> think of a keyword that delayed parsing would make a difference for.
> 
> Probably true.
> 
>> I think we also need to check that token 4 is close paren, so we still get
>> delayed parsing for noexcept (1 + foo).
> 
> Indeed, fixed, in a way that makes the whole conditional more readable.
> 
> I've renamed some tests, otherwise no changes.
> 
> Bootstrap/regtest running on x86_64-linux, ok for trunk if it passes?

Yes, thanks.

Jason