On Sat, Jan 15, 2022 at 3:28 AM Anthony Sharp <anthonyshar...@gmail.com> wrote:
> Hi Jason, > > Hope you are well. Apologies, I've not had time to sit down and look at > this since last month I quit my old job, then I had family around for the > whole of the Christmas period, and then even more recently I've had to > start my new job. > > In any case happy that you managed to figure it all out. I noticed the > small regression at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025. > To be honest I wouldn't even know how to begin to go about fixing that so > perhaps it's best if I leave that to you? I guess it's not playing well > with the use of warn_missing_template_keyword. Maybe I didn't set that > variable up properly or something. > FYI it seems to be a latent bug that cp_rollback_tokens doesn't also roll back input_location, not a bug in the new code. > Kind regards, > Anthony > > On Thu, 13 Jan 2022 at 21:01, Jason Merrill <ja...@redhat.com> wrote: > >> On 12/9/21 10:51, Jason Merrill wrote: >> > On 12/4/21 12:23, Anthony Sharp wrote: >> >> Hi Jason, >> >> >> >> Hope you are well. Apologies for not coming back sooner. >> >> >> >> >I'd put it just above the definition of saved_token_sentinel in >> >> parser.c. >> >> >> >> Sounds good, done. >> >> >> >> >Maybe cp_parser_require_end_of_template_parameter_list? Either way >> >> is fine. >> >> >> >> Even better, have changed it. >> >> >> >> >Hmm, good point; operators that are member functions must be >> >> non-static, >> >> >so we couldn't be doing a comparison of the address of the function. >> >> >> >> In that case I have set it to return early there. >> >> >> >> >So declarator_p should be true there. I'll fix that. >> >> >> >> Thank you. >> >> >> >> >> + if (next_token->keyword == RID_TEMPLATE) >> >> >> + { >> >> >> + /* But at least make sure it's properly formed (e.g. see >> >> PR19397). */ >> >> >> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == >> >> CPP_NAME) >> >> >> + return 1; >> >> >> + >> >> >> + return -1; >> >> >> + } >> >> >> + >> >> >> + /* Could be a ~ referencing the destructor of a class template. >> >> */ >> >> >> + if (next_token->type == CPP_COMPL) >> >> >> + { >> >> >> + /* It could only be a template. */ >> >> >> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == >> >> CPP_NAME) >> >> >> + return 1; >> >> >> + >> >> >> + return -1; >> >> >> + } >> >> > >> >> >Why don't these check for the < ? >> >> >> >> I think perhaps I could have named the function better; instead of >> >> next_token_begins_template_id, how's about >> >> next_token_begins_template_name? >> >> That's all I intended to check for. >> > >> > You're using it to control whether we try to parse a template-id, and >> > it's used to initialize variables named looks_like_template_id, so I >> > think better to keep the old name. >> > >> >> In the first case, something like "->template some_name" will always be >> >> intended as a template, so no need to check for the <. If there were >> >> default >> >> template arguments you could also validly omit the <> completely, I >> think >> >> (could be wrong). >> > >> > Or if the template arguments can be deduced, yes: >> > >> > template <class T> struct A >> > { >> > template <class U> void f(U u); >> > }; >> > >> > template <class T> void g(A<T> a) >> > { >> > a->template f(42); >> > } >> > >> > But 'f' is still not a template-id. >> > >> > ... >> > >> > Actually, it occurs to me that you might be better off handling this in >> > cp_parser_template_name, something like the below, to avoid the complex >> > duplicate logic in the id-expression handling. >> > >> > Note that in this patch I'm using "any_object_scope" as a proxy for >> "I'm >> > parsing an expression", since !is_declaration doesn't work for that; as >> > a result, this doesn't handle the static member function template case. >> > For that you'd probably still need to pass down a flag so that >> > cp_parser_template_name knows it's being called from >> > cp_parser_id_expression. >> > >> > Your patch has a false positive on >> > >> > template <bool B> struct A { }; >> > template <class T> void f() >> > { >> > A<T::I < T::J>(); >> > }; >> > >> > which my patch checks in_template_argument_list_p to avoid, though >> > checking any_object_scope also currently avoids it. >> > >> > What do you think? >> >> I decided that it made more sense to keep the check in >> cp_parser_id_expression like you had it, but I moved it to the end to >> simplify the logic. Here's what I'm applying, thanks! > >
