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?

Jason
From 3022a600514f8adf8706fd286387a5f9f34c06ba Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Wed, 8 Dec 2021 17:24:21 -0500
Subject: [PATCH] handle in cp_parser_template_name
To: gcc-patches@gcc.gnu.org

---
 gcc/cp/parser.c | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ed0bae33fc1..6ec9da646cf 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -18546,6 +18546,8 @@ cp_parser_template_name (cp_parser* parser,
   /* cp_parser_lookup_name clears OBJECT_TYPE.  */
   tree scope = (parser->scope ? parser->scope
 		: parser->context->object_type);
+  bool any_object_scope = (parser->context->object_type
+			   || parser->object_scope);
 
   /* Look up the name.  */
   decl = cp_parser_lookup_name (parser, identifier,
@@ -18625,12 +18627,41 @@ cp_parser_template_name (cp_parser* parser,
 	    found = true;
 	}
 
-      /* "in a type-only context" */
       if (!found && scope
-	  && tag_type != none_type
-	  && dependentish_scope_p (scope)
-	  && cp_parser_nth_token_starts_template_argument_list_p (parser, 1))
-	found = true;
+	  && processing_template_decl
+	  && cp_parser_nth_token_starts_template_argument_list_p (parser, 1)
+	  && dependentish_scope_p (scope))
+	{
+	  /* "in a type-only context" */
+	  if (tag_type != none_type)
+	    found = true;
+	  else if (warn_missing_template_keyword
+		   /* If there's an object scope we know we're in an
+		      expression, not a declarator-id.  FIXME is_declaration
+		      ought to make that distinction for us.  */
+		   && any_object_scope
+		   /* In a template argument list a > could be closing
+		      the enclosing targs.  */
+		   && !parser->in_template_argument_list_p
+		   && !token->error_reported
+		   && warning_enabled_at (token->location,
+					  OPT_Wmissing_template_keyword))
+	    {
+	      saved_token_sentinel toks (parser->lexer, STS_ROLLBACK);
+	      if (cp_parser_skip_entire_template_parameter_list (parser)
+		  /* An operator after the > suggests that the > ends a
+		     template-id; a name or literal suggests that the > is an
+		     operator.  */
+		  && (cp_lexer_peek_token (parser->lexer)->type
+		      <= CPP_LAST_PUNCTUATOR))
+		{
+		  warning_at (token->location, OPT_Wmissing_template_keyword,
+			      "expected %qs keyword before dependent "
+			      "template name", "template");
+		  token->error_reported = true;
+		}
+	    }
+	}
 
       if (!found)
 	{
-- 
2.27.0

Reply via email to