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. 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!
