[PATCH] c++: bogus error w/ variadic concept-id as if cond [PR98394]

Jason Merrill jason@redhat.com
Tue Nov 9 05:13:23 GMT 2021


On 11/8/21 19:54, Patrick Palka wrote:
> On Mon, 8 Nov 2021, Jason Merrill wrote:
> 
>> On 11/8/21 10:35, Patrick Palka wrote:
>>> Here when tentatively parsing the if condition as a declaration, we try
>>> to treat C<1> as the start of a constrained placeholder type, which we
>>> quickly reject because C doesn't accept a type as its first argument.
>>> But since we're parsing tentatively, we shouldn't emit an error in this
>>> case.
>>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk/11?
>>>
>>> 	PR c++/98394
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* parser.c (cp_parser_placeholder_type_specifier): Don't emit
>>> 	a "does not constrain a type" error when parsing tentatively.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp2a/concepts-pr98394.C: New test.
>>> ---
>>>    gcc/cp/parser.c                               |  7 +++++--
>>>    gcc/testsuite/g++.dg/cpp2a/concepts-pr98394.C | 14 ++++++++++++++
>>>    2 files changed, 19 insertions(+), 2 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98394.C
>>>
>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>>> index 4c2075742d6..f1498e28da4 100644
>>> --- a/gcc/cp/parser.c
>>> +++ b/gcc/cp/parser.c
>>> @@ -19909,8 +19909,11 @@ cp_parser_placeholder_type_specifier (cp_parser
>>> *parser, location_t loc,
>>>          if (!flag_concepts_ts
>>>    	  || !processing_template_parmlist)
>>>    	{
>>> -	  error_at (loc, "%qE does not constrain a type", DECL_NAME (con));
>>> -	  inform (DECL_SOURCE_LOCATION (con), "concept defined here");
>>> +	  if (!tentative)
>>> +	    {
>>> +	      error_at (loc, "%qE does not constrain a type", DECL_NAME
>>> (con));
>>> +	      inform (DECL_SOURCE_LOCATION (con), "concept defined here");
>>> +	    }
>>
>> We probably want to cp_parser_simulate_error in the tentative case?
> 
> It seems the only caller, cp_parser_simple_type_specifier, is
> responsible for that currently.
> 
>>
>> I also wonder why this code uses a "tentative" parameter instead of checking
>> cp_parser_parsing_tentatively itself.
> 
> During the first call to cp_parser_placeholder_type_specifier from
> cp_parser_simple_type_specifier, we're always parsing tentatively so
> cp_parser_parsing_tentatively would always be true whereas 'tentative'
> could be false if there's also an outer tentative parse.  So some
> reworking of the caller would also be needed in order to avoid the
> 'tentative' parameter.  I tried, but I wasn't able to make it work
> without introducing diagnostic regressions :/
> 
> While poking at the idea though, I was reminded of the related PR85846
> which is about the concept-id C<> being rejected due to a stray error
> being emitted during the tentative type parse.  The below patch
> additionally fixes this PR since it just requires a one-line change in
> cp_parser_placeholder_type_specifier.
> 
> -- >8 --
> 
> Subject: [PATCH] c++: bogus error w/ variadic concept-id as if cond [PR98394]
> 
> Here when tentatively parsing the if condition as a declaration, we try
> to treat C<1> as the start of a constrained placeholder type, which we
> quickly reject because C doesn't accept a type as its first argument.
> But since we're parsing tentatively, we shouldn't emit an error in this
> case.
> 
> In passing, also fix PR85846 by only overriding tentative to false when
> given a concept-name, and not also when given a concept-id with empty
> argument list.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/11?

OK for both.

> 	PR c++/98394
> 	PR c++/85846
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.c (cp_parser_placeholder_type_specifier): Declare
> 	static.  Don't override tentative to false when tmpl is a
> 	concept-id with empty argument list.  Don't emit a "does not
> 	constrain a type" error when tentative.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-pr98394.C: New test.
> 	* g++.dg/cpp2a/concepts-pr85846.C: New test.
> ---
>   gcc/cp/parser.c                               | 11 +++++++----
>   gcc/testsuite/g++.dg/cpp2a/concepts-pr85846.C |  5 +++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-pr98394.C | 14 ++++++++++++++
>   3 files changed, 26 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr85846.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98394.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 4c2075742d6..71f782468ef 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -19855,7 +19855,7 @@ cp_parser_simple_type_specifier (cp_parser* parser,
>     Note that the Concepts TS allows the auto or decltype(auto) to be
>     omitted in a constrained-type-specifier.  */
>   
> -tree
> +static tree
>   cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
>   				      tree tmpl, bool tentative)
>   {
> @@ -19871,7 +19871,7 @@ cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
>         args = TREE_OPERAND (tmpl, 1);
>         tmpl = TREE_OPERAND (tmpl, 0);
>       }
> -  if (args == NULL_TREE)
> +  else if (args == NULL_TREE)
>       /* A concept-name with no arguments can't be an expression.  */
>       tentative = false;
>   
> @@ -19909,8 +19909,11 @@ cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
>         if (!flag_concepts_ts
>   	  || !processing_template_parmlist)
>   	{
> -	  error_at (loc, "%qE does not constrain a type", DECL_NAME (con));
> -	  inform (DECL_SOURCE_LOCATION (con), "concept defined here");
> +	  if (!tentative)
> +	    {
> +	      error_at (loc, "%qE does not constrain a type", DECL_NAME (con));
> +	      inform (DECL_SOURCE_LOCATION (con), "concept defined here");
> +	    }
>   	  return error_mark_node;
>   	}
>       }
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr85846.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr85846.C
> new file mode 100644
> index 00000000000..1507c604920
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr85846.C
> @@ -0,0 +1,5 @@
> +// PR c++/85846
> +// { dg-do compile { target c++20 } }
> +
> +template<int = 0> concept Foo = true;
> +bool f(Foo<>);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr98394.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98394.C
> new file mode 100644
> index 00000000000..c8407cdf7cd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98394.C
> @@ -0,0 +1,14 @@
> +// PR c++/98394
> +// { dg-do compile { target c++20 } }
> +
> +template<int...>
> +concept C = true;
> +
> +template<int, int>
> +concept D = true;
> +
> +int main() {
> +  if (C<1>); // { dg-bogus "does not constrain a type" }
> +  if (D<1>); // { dg-error "wrong number of template arguments" }
> +	     // { dg-bogus "does not constrain a type" "" { target *-*-* } .-1 }
> +}
> 



More information about the Gcc-patches mailing list