c++: Improve diagnostics for template args terminated with >= or >>= [PR104319]

  Hi!

As mentioned in the PR, for C++98 we have diagnostics that expect
>> terminating template arguments to be a mistake for > > (C++11
said it has to be treated that way), while if user trying to spare the
spacebar doesn't separate > from following = or >> from following =,
the diagnostics is confusing, while clang suggests adding space in between.

The following patch does that for >= and >>= too.

For some strange reason the error recovery emits further errors,
not really sure what's going on because I overwrite the token->type
like the code does for the C++11 >> case or for the C++98 >> cases,
but at least the first error is nicer (well, for the C++98 nested
template case and >>= I need to overwrite it to > and so the = is lost,
so perhaps some follow-up errors are needed for that case).

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
Or shall it wait for GCC 13?

2022-02-04  Jakub Jelinek  <jakub@redhat.com>

	PR c++/104319
	* parser.cc (cp_parser_template_argument): Treat >= like C++98 >>
	after a type id by setting maybe_type_id and aborting tentative
	parse.
	(cp_parser_enclosed_template_argument_list): Handle
	CPP_GREATER_EQ like misspelled CPP_GREATER CPP_RQ and
	CPP_RSHIFT_EQ like misspelled CPP_GREATER CPP_GREATER_EQ
	or CPP_RSHIFT CPP_EQ or CPP_GREATER CPP_GREATER CPP_EQ.
	(cp_parser_next_token_ends_template_argument_p): Return true
	also for CPP_GREATER_EQ and CPP_RSHIFT_EQ.

	* g++.dg/parse/template28.C: Adjust expected diagnostics.
	* g++.dg/parse/template30.C: New test.


	Jakub
  

On 2/4/22 09:12, Jakub Jelinek wrote:
> Hi!
> 
> As mentioned in the PR, for C++98 we have diagnostics that expect
>>> terminating template arguments to be a mistake for > > (C++11
> said it has to be treated that way), while if user trying to spare the
> spacebar doesn't separate > from following = or >> from following =,
> the diagnostics is confusing, while clang suggests adding space in between.
> 
> The following patch does that for >= and >>= too.
> 
> For some strange reason the error recovery emits further errors,
> not really sure what's going on because I overwrite the token->type
> like the code does for the C++11 >> case or for the C++98 >> cases,
> but at least the first error is nicer (well, for the C++98 nested
> template case and >>= I need to overwrite it to > and so the = is lost,
> so perhaps some follow-up errors are needed for that case).
> 
> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
> Or shall it wait for GCC 13?

Hmm, I lean toward GCC 13; this seems more of a stage 3 change.

I see you test valid uses of >= in template arguments; you should also 
test valid >>= (with another overloaded operator).

> 2022-02-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/104319
> 	* parser.cc (cp_parser_template_argument): Treat >= like C++98 >>
> 	after a type id by setting maybe_type_id and aborting tentative
> 	parse.
> 	(cp_parser_enclosed_template_argument_list): Handle
> 	CPP_GREATER_EQ like misspelled CPP_GREATER CPP_RQ and
> 	CPP_RSHIFT_EQ like misspelled CPP_GREATER CPP_GREATER_EQ
> 	or CPP_RSHIFT CPP_EQ or CPP_GREATER CPP_GREATER CPP_EQ.
> 	(cp_parser_next_token_ends_template_argument_p): Return true
> 	also for CPP_GREATER_EQ and CPP_RSHIFT_EQ.
> 
> 	* g++.dg/parse/template28.C: Adjust expected diagnostics.
> 	* g++.dg/parse/template30.C: New test.
> 
> --- gcc/cp/parser.cc.jj	2022-02-04 14:36:54.765608651 +0100
> +++ gcc/cp/parser.cc	2022-02-04 14:42:14.761139259 +0100
> @@ -18820,8 +18820,13 @@ cp_parser_template_argument (cp_parser*
>        In C++0x, the '>>' will be considered two separate '>'
>        tokens.  */
>     if (!cp_parser_error_occurred (parser)
> -      && cxx_dialect == cxx98
> -      && cp_lexer_next_token_is (parser->lexer, CPP_RSHIFT))
> +      && ((cxx_dialect == cxx98
> +	   && cp_lexer_next_token_is (parser->lexer, CPP_RSHIFT))
> +	  /* Similarly for >= which
> +	     cp_parser_next_token_ends_template_argument_p treats for
> +	     diagnostics purposes as mistyped > =, but can be valid
> +	     after a type-id.  */
> +	  || cp_lexer_next_token_is (parser->lexer, CPP_GREATER_EQ)))
>       {
>         maybe_type_id = true;
>         cp_parser_abort_tentative_parse (parser);
> @@ -32029,7 +32034,9 @@ cp_parser_enclosed_template_argument_lis
>     cp_evaluated ev;
>     /* Parse the template-argument-list itself.  */
>     if (cp_lexer_next_token_is (parser->lexer, CPP_GREATER)
> -      || cp_lexer_next_token_is (parser->lexer, CPP_RSHIFT))
> +      || cp_lexer_next_token_is (parser->lexer, CPP_RSHIFT)
> +      || cp_lexer_next_token_is (parser->lexer, CPP_GREATER_EQ)
> +      || cp_lexer_next_token_is (parser->lexer, CPP_RSHIFT_EQ))
>       arguments = NULL_TREE;
>     else
>       arguments = cp_parser_template_argument_list (parser);
> @@ -32086,6 +32093,38 @@ cp_parser_enclosed_template_argument_lis
>   		    "a template argument list");
>   	}
>       }
> +  /* Similarly for >>= and >=.  */
> +  else if (cp_lexer_next_token_is (parser->lexer, CPP_GREATER_EQ)
> +	   || cp_lexer_next_token_is (parser->lexer, CPP_RSHIFT_EQ))
> +    {
> +      cp_token *token = cp_lexer_consume_token (parser->lexer);
> +      gcc_rich_location richloc (token->location);
> +      enum cpp_ttype new_type;
> +      const char *replacement;
> +      if (token->type == CPP_GREATER_EQ)
> +	{
> +	  replacement = "> =";
> +	  new_type = CPP_EQ;
> +	}
> +      else if (!saved_greater_than_is_operator_p)
> +	{
> +	  if (cxx_dialect != cxx98)
> +	    replacement = ">> =";
> +	  else
> +	    replacement = "> > =";
> +	  new_type = CPP_GREATER;
> +	}
> +      else
> +	{
> +	  replacement = "> >=";
> +	  new_type = CPP_GREATER_EQ;
> +	}
> +      richloc.add_fixit_replace (replacement);
> +      error_at (&richloc, "%qs should be %qs to terminate a template "
> +		"argument list",
> +		cpp_type2name (token->type, token->flags), replacement);
> +      token->type = new_type;
> +    }
>     else
>       cp_parser_require_end_of_template_parameter_list (parser);
>     /* The `>' token might be a greater-than operator again now.  */
> @@ -33163,7 +33202,11 @@ cp_parser_next_token_ends_template_argum
>     return (token->type == CPP_COMMA
>             || token->type == CPP_GREATER
>             || token->type == CPP_ELLIPSIS
> -	  || ((cxx_dialect != cxx98) && token->type == CPP_RSHIFT));
> +	  || ((cxx_dialect != cxx98) && token->type == CPP_RSHIFT)
> +	  /* For better diagnostics, treat >>= like that too, that
> +	     shouldn't appear non-nested in template arguments.  */
> +	  || token->type == CPP_GREATER_EQ
> +	  || token->type == CPP_RSHIFT_EQ);
>   }
>   
>   /* Returns TRUE iff the n-th token is a "<", or the n-th is a "[" and the
> --- gcc/testsuite/g++.dg/parse/template28.C.jj	2020-01-12 11:54:37.228401112 +0100
> +++ gcc/testsuite/g++.dg/parse/template28.C	2022-02-04 14:51:13.387630759 +0100
> @@ -2,8 +2,8 @@
>   
>   template<class> struct A {};
>   
> -template<class T> void foo(A<T>=A<T>()) {} // { dg-error "24:variable or field .foo. declared void" }
> -// { dg-error "template" "" { target *-*-* } .-1 }
> +template<class T> void foo(A<T>=A<T>()) {} // { dg-error "'>=' should be '> =' to terminate a template argument list" }
> +// { dg-error "expected" "" { target *-*-* } .-1 }
>   
>   void bar()
>   {
> --- gcc/testsuite/g++.dg/parse/template30.C.jj	2022-02-04 14:42:14.761139259 +0100
> +++ gcc/testsuite/g++.dg/parse/template30.C	2022-02-04 14:42:14.761139259 +0100
> @@ -0,0 +1,49 @@
> +// PR c++/104319
> +// { dg-do compile }
> +// { dg-options "" }
> +
> +template<typename T> struct A {};
> +template<typename T> int z;	// { dg-warning "variable templates only available with" "" { target c++11_down } }
> +template<int N> int w;		// { dg-warning "variable templates only available with" "" { target c++11_down } }
> +
> +void
> +foo ()
> +{
> +  z<int>=0;			// { dg-error "'>=' should be '> =' to terminate a template argument list" }
> +}				// { dg-error "expected ';' before numeric constant" "" { target *-*-* } .-1 }
> +
> +int
> +bar ()
> +{
> +  return z<int>>0;		// { dg-error "spurious '>>', use '>' to terminate a template argument list" "" { target c++98_only } }
> +}				// { dg-error "expected ';' before numeric constant" "" { target c++98_only } .-1 }
> +
> +int
> +baz ()
> +{
> +  return z<int>>=0;		// { dg-error "'>>=' should be '> >=' to terminate a template argument list" }
> +}				// { dg-error "expected ';' before numeric constant" "" { target *-*-* } .-1 }
> +
> +int
> +qux ()
> +{
> +  return z<A<int>>=0;		// { dg-error "'>>=' should be '>> =' to terminate a template argument list" "" { target c++11 } }
> +}				// { dg-error "'>>=' should be '> > =' to terminate a template argument list" "" { target c++98_only } .-1 }
> +				// { dg-error "parse error in template argument list" "" { target *-*-* } .-2 }
> +				// { dg-error "template argument 1 is invalid" "" { target *-*-* } .-3 }
> +
> +void
> +quux ()
> +{
> +  w<5>=0>=6>=8> = 5;
> +}
> +
> +#if __cplusplus >= 201103L
> +struct B { constexpr bool operator >= (int) { return true; } };
> +
> +void
> +corge ()
> +{
> +  w<B()>=5> = 5;
> +}
> +#endif
> 
> 	Jakub
>
  

On Fri, Feb 04, 2022 at 04:54:39PM -0500, Jason Merrill wrote:
> > Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
> > Or shall it wait for GCC 13?
> 
> Hmm, I lean toward GCC 13; this seems more of a stage 3 change.

Ok.

> I see you test valid uses of >= in template arguments; you should also test
> valid >>= (with another overloaded operator).

Is >>= valid there though?  The comment says:
   Although the standard says "assignment-expression", it forbids
   throw-expressions or assignments in the template argument.
   Therefore, we use "conditional-expression" instead.  */

	Jakub
  

On 2/4/22 17:10, Jakub Jelinek wrote:
> On Fri, Feb 04, 2022 at 04:54:39PM -0500, Jason Merrill wrote:
>>> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
>>> Or shall it wait for GCC 13?
>>
>> Hmm, I lean toward GCC 13; this seems more of a stage 3 change.
> 
> Ok.
> 
>> I see you test valid uses of >= in template arguments; you should also test
>> valid >>= (with another overloaded operator).
> 
> Is >>= valid there though?  The comment says:
>     Although the standard says "assignment-expression", it forbids
>     throw-expressions or assignments in the template argument.
>     Therefore, we use "conditional-expression" instead.  */

Ah, good point.  Incidentally that's out of date; it now says 
"constant-expression", which expands to "conditional-expression".

Jason