diff mbox series

[v2] c++: concept in default argument [PR109859]

Message ID ZvckA2cYbqL-DcWG@redhat.com
State New
Headers
Series [v2] c++: concept in default argument [PR109859] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Marek Polacek Sept. 27, 2024, 9:30 p.m. UTC 
  On Fri, Sep 27, 2024 at 04:57:58PM -0400, Jason Merrill wrote:
> On 9/18/24 5:06 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > 1) We're hitting the assert in cp_parser_placeholder_type_specifier.
> > It says that if it turns out to be false, we should do error() instead.
> > Do so, then.
> > 
> > 2) lambda-targ8.C should compile fine, though.  The problem was that
> > local_variables_forbidden_p wasn't cleared when we're about to parse
> > the optional template-parameter-list for a lambda in a default argument.
> > 
> > 	PR c++/109859
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* parser.cc (cp_parser_lambda_declarator_opt): Temporarily clear
> > 	local_variables_forbidden_p.
> > 	(cp_parser_placeholder_type_specifier): Turn an assert into an error.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/concepts-defarg3.C: New test.
> > 	* g++.dg/cpp2a/lambda-targ8.C: New test.
> > ---
> >   gcc/cp/parser.cc                              |  9 +++++++--
> >   gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C |  8 ++++++++
> >   gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C     | 10 ++++++++++
> >   3 files changed, 25 insertions(+), 2 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C
> > 
> > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> > index 4dd9474cf60..bdc4fef243a 100644
> > --- a/gcc/cp/parser.cc
> > +++ b/gcc/cp/parser.cc
> > @@ -11891,6 +11891,11 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr)
> >   		 "lambda templates are only available with "
> >   		 "%<-std=c++20%> or %<-std=gnu++20%>");
> > +      /* Even though the whole lambda may be a default argument, its
> > +	 template-parameter-list is a context where it's OK to create
> > +	 new parameters.  */
> > +      auto lvf = make_temp_override (parser->local_variables_forbidden_p, 0u);
> > +
> >         cp_lexer_consume_token (parser->lexer);
> >         template_param_list = cp_parser_template_parameter_list (parser);
> > @@ -20978,8 +20983,8 @@ cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
> >         /* In a default argument we may not be creating new parameters.  */
> >         if (parser->local_variables_forbidden_p & LOCAL_VARS_FORBIDDEN)
> >   	{
> > -	  /* If this assert turns out to be false, do error() instead.  */
> > -	  gcc_assert (tentative);
> > +	  if (!tentative)
> > +	    error_at (loc, "local variables may not appear in this context");
> 
> There's no local variable in the new testcase, the error should talk about a
> concept-name.

Ah sure.  So like this?

Tested dg.exp.

-- >8 --
1) We're hitting the assert in cp_parser_placeholder_type_specifier.
It says that if it turns out to be false, we should do error() instead.
Do so, then.

2) lambda-targ8.C should compile fine, though.  The problem was that
local_variables_forbidden_p wasn't cleared when we're about to parse
the optional template-parameter-list for a lambda in a default argument.

	PR c++/109859

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_lambda_declarator_opt): Temporarily clear
	local_variables_forbidden_p.
	(cp_parser_placeholder_type_specifier): Turn an assert into an error.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-defarg3.C: New test.
	* g++.dg/cpp2a/lambda-targ8.C: New test.
---
 gcc/cp/parser.cc                              |  9 +++++++--
 gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C |  8 ++++++++
 gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C     | 10 ++++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C


base-commit: 7040c207baa6b5d5f6065a47dd3559f3d3974a1b

Comments

Jason Merrill Sept. 30, 2024, 2:53 p.m. UTC | #1 
On 9/27/24 5:30 PM, Marek Polacek wrote:
> On Fri, Sep 27, 2024 at 04:57:58PM -0400, Jason Merrill wrote:
>> On 9/18/24 5:06 PM, Marek Polacek wrote:
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> -- >8 --
>>> 1) We're hitting the assert in cp_parser_placeholder_type_specifier.
>>> It says that if it turns out to be false, we should do error() instead.
>>> Do so, then.
>>>
>>> 2) lambda-targ8.C should compile fine, though.  The problem was that
>>> local_variables_forbidden_p wasn't cleared when we're about to parse
>>> the optional template-parameter-list for a lambda in a default argument.
>>>
>>> 	PR c++/109859
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* parser.cc (cp_parser_lambda_declarator_opt): Temporarily clear
>>> 	local_variables_forbidden_p.
>>> 	(cp_parser_placeholder_type_specifier): Turn an assert into an error.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp2a/concepts-defarg3.C: New test.
>>> 	* g++.dg/cpp2a/lambda-targ8.C: New test.
>>> ---
>>>    gcc/cp/parser.cc                              |  9 +++++++--
>>>    gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C |  8 ++++++++
>>>    gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C     | 10 ++++++++++
>>>    3 files changed, 25 insertions(+), 2 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C
>>>
>>> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
>>> index 4dd9474cf60..bdc4fef243a 100644
>>> --- a/gcc/cp/parser.cc
>>> +++ b/gcc/cp/parser.cc
>>> @@ -11891,6 +11891,11 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr)
>>>    		 "lambda templates are only available with "
>>>    		 "%<-std=c++20%> or %<-std=gnu++20%>");
>>> +      /* Even though the whole lambda may be a default argument, its
>>> +	 template-parameter-list is a context where it's OK to create
>>> +	 new parameters.  */
>>> +      auto lvf = make_temp_override (parser->local_variables_forbidden_p, 0u);
>>> +
>>>          cp_lexer_consume_token (parser->lexer);
>>>          template_param_list = cp_parser_template_parameter_list (parser);
>>> @@ -20978,8 +20983,8 @@ cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
>>>          /* In a default argument we may not be creating new parameters.  */
>>>          if (parser->local_variables_forbidden_p & LOCAL_VARS_FORBIDDEN)
>>>    	{
>>> -	  /* If this assert turns out to be false, do error() instead.  */
>>> -	  gcc_assert (tentative);
>>> +	  if (!tentative)
>>> +	    error_at (loc, "local variables may not appear in this context");
>>
>> There's no local variable in the new testcase, the error should talk about a
>> concept-name.
> 
> Ah sure.  So like this?
> 
> Tested dg.exp.
> 
> -- >8 --
> 1) We're hitting the assert in cp_parser_placeholder_type_specifier.
> It says that if it turns out to be false, we should do error() instead.
> Do so, then.
> 
> 2) lambda-targ8.C should compile fine, though.  The problem was that
> local_variables_forbidden_p wasn't cleared when we're about to parse
> the optional template-parameter-list for a lambda in a default argument.
> 
> 	PR c++/109859
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_lambda_declarator_opt): Temporarily clear
> 	local_variables_forbidden_p.
> 	(cp_parser_placeholder_type_specifier): Turn an assert into an error.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-defarg3.C: New test.
> 	* g++.dg/cpp2a/lambda-targ8.C: New test.
> ---
>   gcc/cp/parser.cc                              |  9 +++++++--
>   gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C |  8 ++++++++
>   gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C     | 10 ++++++++++
>   3 files changed, 25 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index f50534f5f39..a92e6a29ba6 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -11891,6 +11891,11 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr)
>   		 "lambda templates are only available with "
>   		 "%<-std=c++20%> or %<-std=gnu++20%>");
>   
> +      /* Even though the whole lambda may be a default argument, its
> +	 template-parameter-list is a context where it's OK to create
> +	 new parameters.  */
> +      auto lvf = make_temp_override (parser->local_variables_forbidden_p, 0u);
> +
>         cp_lexer_consume_token (parser->lexer);
>   
>         template_param_list = cp_parser_template_parameter_list (parser);
> @@ -20989,8 +20994,8 @@ cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
>         /* In a default argument we may not be creating new parameters.  */
>         if (parser->local_variables_forbidden_p & LOCAL_VARS_FORBIDDEN)
>   	{
> -	  /* If this assert turns out to be false, do error() instead.  */
> -	  gcc_assert (tentative);
> +	  if (!tentative)
> +	    error_at (loc, "concept-name may not appear in this context");

Hmm, actually I expect it can appear in a concept-id.  What if we fall 
through to the next case instead of erroring/returning at this point?

Jason
diff mbox series

Patch

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index f50534f5f39..a92e6a29ba6 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -11891,6 +11891,11 @@  cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr)
 		 "lambda templates are only available with "
 		 "%<-std=c++20%> or %<-std=gnu++20%>");
 
+      /* Even though the whole lambda may be a default argument, its
+	 template-parameter-list is a context where it's OK to create
+	 new parameters.  */
+      auto lvf = make_temp_override (parser->local_variables_forbidden_p, 0u);
+
       cp_lexer_consume_token (parser->lexer);
 
       template_param_list = cp_parser_template_parameter_list (parser);
@@ -20989,8 +20994,8 @@  cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
       /* In a default argument we may not be creating new parameters.  */
       if (parser->local_variables_forbidden_p & LOCAL_VARS_FORBIDDEN)
 	{
-	  /* If this assert turns out to be false, do error() instead.  */
-	  gcc_assert (tentative);
+	  if (!tentative)
+	    error_at (loc, "concept-name may not appear in this context");
 	  return error_mark_node;
 	}
       return build_constrained_parameter (con, proto, args);
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C
new file mode 100644
index 00000000000..1d689dc8f30
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-defarg3.C
@@ -0,0 +1,8 @@ 
+// PR c++/109859
+// { dg-do compile { target c++20 } }
+
+template<class>
+concept C = true;
+
+template <class = C> // { dg-error "may not appear in this context" }
+int f();
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C b/gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C
new file mode 100644
index 00000000000..3685b0ef880
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-targ8.C
@@ -0,0 +1,10 @@ 
+// PR c++/109859
+// { dg-do compile { target c++20 } }
+
+template<typename>
+concept A = true;
+
+template<auto = []<A a> {}>
+int x;
+
+void g() { (void) x<>; }