diff mbox series

c++, v2: Fix parsing [[]][[]];

Message ID ZW9XL+ac5rY9XbF9@tucnak
State Unresolved
Headers
Series c++, v2: Fix parsing [[]][[]]; |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Dec. 5, 2023, 5 p.m. UTC 
  On Tue, Dec 05, 2023 at 09:45:32AM -0500, Marek Polacek wrote:
> > When working on the previous patch I put [[]] [[]] asm (""); into a
> > testcase, but was surprised it wasn't parsed.
> 
> By wasn't parsed you mean we gave an error, right?  I only see an error
> with block-scope [[]] [[]];.

Yeah.
The reason why [[]][[]]; works at namespace scope is that if
  else if (cp_lexer_nth_token_is (parser->lexer,
                                  cp_parser_skip_std_attribute_spec_seq (parser,
                                                                         1),
                                  CPP_SEMICOLON))
which is the case here then even if after parsing the attributes next token
isn't CPP_SEMICOLON (the case here without the patch), it will just return
and another cp_parser_declaration will parse another [[]], that time also
with CPP_SEMICOLON.

> It seems marginally better to me to use void_list_node so that we don't
> need a new parm, like what we do when parsing parameters: ()/(void)/(...),
> but I should let others decide.

Here is a modified version of the patch which does it like that.

2023-12-05  Jakub Jelinek  <jakub@redhat.com>

	* parser.cc (cp_parser_std_attribute_spec): Return void_list_node
	rather than NULL_TREE if token is neither CPP_OPEN_SQUARE nor
	RID_ALIGNAS CPP_KEYWORD.
	(cp_parser_std_attribute_spec_seq): For attr_spec == void_list_node
	break, for attr_spec == NULL_TREE continue.

	* g++.dg/cpp0x/gen-attrs-79.C: New test.



	Jakub

Comments

Marek Polacek Dec. 5, 2023, 5:17 p.m. UTC | #1 
On Tue, Dec 05, 2023 at 06:00:31PM +0100, Jakub Jelinek wrote:
> On Tue, Dec 05, 2023 at 09:45:32AM -0500, Marek Polacek wrote:
> > > When working on the previous patch I put [[]] [[]] asm (""); into a
> > > testcase, but was surprised it wasn't parsed.
> > 
> > By wasn't parsed you mean we gave an error, right?  I only see an error
> > with block-scope [[]] [[]];.
> 
> Yeah.
> The reason why [[]][[]]; works at namespace scope is that if
>   else if (cp_lexer_nth_token_is (parser->lexer,
>                                   cp_parser_skip_std_attribute_spec_seq (parser,
>                                                                          1),
>                                   CPP_SEMICOLON))
> which is the case here then even if after parsing the attributes next token
> isn't CPP_SEMICOLON (the case here without the patch), it will just return
> and another cp_parser_declaration will parse another [[]], that time also
> with CPP_SEMICOLON.
> 
> > It seems marginally better to me to use void_list_node so that we don't
> > need a new parm, like what we do when parsing parameters: ()/(void)/(...),
> > but I should let others decide.
> 
> Here is a modified version of the patch which does it like that.

Thanks, this looks good to me.
 
> 2023-12-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* parser.cc (cp_parser_std_attribute_spec): Return void_list_node
> 	rather than NULL_TREE if token is neither CPP_OPEN_SQUARE nor
> 	RID_ALIGNAS CPP_KEYWORD.
> 	(cp_parser_std_attribute_spec_seq): For attr_spec == void_list_node
> 	break, for attr_spec == NULL_TREE continue.
> 
> 	* g++.dg/cpp0x/gen-attrs-79.C: New test.
> 
> --- gcc/cp/parser.cc.jj	2023-12-05 16:18:32.224909370 +0100
> +++ gcc/cp/parser.cc	2023-12-05 17:07:34.690170639 +0100
> @@ -30244,7 +30244,11 @@ void cp_parser_late_contract_condition (
>       [ [ assert :  contract-mode [opt] : conditional-expression ] ]
>       [ [ pre :  contract-mode [opt] : conditional-expression ] ]
>       [ [ post :  contract-mode [opt] identifier [opt] :
> -	 conditional-expression ] ]  */
> +	 conditional-expression ] ]
> +
> +   Return void_list_node if the current token doesn't start an
> +   attribute-specifier to differentiate from NULL_TREE returned e.g.
> +   for [ [ ] ].  */
>  
>  static tree
>  cp_parser_std_attribute_spec (cp_parser *parser)
> @@ -30324,7 +30328,7 @@ cp_parser_std_attribute_spec (cp_parser
>  
>        if (token->type != CPP_KEYWORD
>  	  || token->keyword != RID_ALIGNAS)
> -	return NULL_TREE;
> +	return void_list_node;
>  
>        cp_lexer_consume_token (parser->lexer);
>        maybe_warn_cpp0x (CPP0X_ATTRIBUTES);
> @@ -30397,8 +30401,12 @@ cp_parser_std_attribute_spec_seq (cp_par
>    while (true)
>      {
>        tree attr_spec = cp_parser_std_attribute_spec (parser);
> -      if (attr_spec == NULL_TREE)
> +      if (attr_spec == void_list_node)
>  	break;
> +      /* Accept [[]][[]]; for which cp_parser_std_attribute_spec
> +	 returns NULL_TREE as there are no attributes.  */
> +      if (attr_spec == NULL_TREE)
> +	continue;
>        if (attr_spec == error_mark_node)
>  	return error_mark_node;
>  
> --- gcc/testsuite/g++.dg/cpp0x/gen-attrs-79.C.jj	2023-12-05 17:04:14.235988879 +0100
> +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-79.C	2023-12-05 17:04:14.235988879 +0100
> @@ -0,0 +1,9 @@
> +// { dg-do compile { target c++11 } }
> +
> +[[]] [[]];
> +
> +[[]] [[]] void
> +foo ()
> +{
> +  [[]] [[]];
> +}

Marek
Jason Merrill Dec. 8, 2023, 7:56 p.m. UTC | #2 
On 12/5/23 12:17, Marek Polacek wrote:
> On Tue, Dec 05, 2023 at 06:00:31PM +0100, Jakub Jelinek wrote:
>> On Tue, Dec 05, 2023 at 09:45:32AM -0500, Marek Polacek wrote:
>>>> When working on the previous patch I put [[]] [[]] asm (""); into a
>>>> testcase, but was surprised it wasn't parsed.
>>>
>>> By wasn't parsed you mean we gave an error, right?  I only see an error
>>> with block-scope [[]] [[]];.
>>
>> Yeah.
>> The reason why [[]][[]]; works at namespace scope is that if
>>    else if (cp_lexer_nth_token_is (parser->lexer,
>>                                    cp_parser_skip_std_attribute_spec_seq (parser,
>>                                                                           1),
>>                                    CPP_SEMICOLON))
>> which is the case here then even if after parsing the attributes next token
>> isn't CPP_SEMICOLON (the case here without the patch), it will just return
>> and another cp_parser_declaration will parse another [[]], that time also
>> with CPP_SEMICOLON.
>>
>>> It seems marginally better to me to use void_list_node so that we don't
>>> need a new parm, like what we do when parsing parameters: ()/(void)/(...),
>>> but I should let others decide.
>>
>> Here is a modified version of the patch which does it like that.
> 
> Thanks, this looks good to me.

Agreed, OK.
diff mbox series

Patch

--- gcc/cp/parser.cc.jj	2023-12-05 16:18:32.224909370 +0100
+++ gcc/cp/parser.cc	2023-12-05 17:07:34.690170639 +0100
@@ -30244,7 +30244,11 @@  void cp_parser_late_contract_condition (
      [ [ assert :  contract-mode [opt] : conditional-expression ] ]
      [ [ pre :  contract-mode [opt] : conditional-expression ] ]
      [ [ post :  contract-mode [opt] identifier [opt] :
-	 conditional-expression ] ]  */
+	 conditional-expression ] ]
+
+   Return void_list_node if the current token doesn't start an
+   attribute-specifier to differentiate from NULL_TREE returned e.g.
+   for [ [ ] ].  */
 
 static tree
 cp_parser_std_attribute_spec (cp_parser *parser)
@@ -30324,7 +30328,7 @@  cp_parser_std_attribute_spec (cp_parser
 
       if (token->type != CPP_KEYWORD
 	  || token->keyword != RID_ALIGNAS)
-	return NULL_TREE;
+	return void_list_node;
 
       cp_lexer_consume_token (parser->lexer);
       maybe_warn_cpp0x (CPP0X_ATTRIBUTES);
@@ -30397,8 +30401,12 @@  cp_parser_std_attribute_spec_seq (cp_par
   while (true)
     {
       tree attr_spec = cp_parser_std_attribute_spec (parser);
-      if (attr_spec == NULL_TREE)
+      if (attr_spec == void_list_node)
 	break;
+      /* Accept [[]][[]]; for which cp_parser_std_attribute_spec
+	 returns NULL_TREE as there are no attributes.  */
+      if (attr_spec == NULL_TREE)
+	continue;
       if (attr_spec == error_mark_node)
 	return error_mark_node;
 
--- gcc/testsuite/g++.dg/cpp0x/gen-attrs-79.C.jj	2023-12-05 17:04:14.235988879 +0100
+++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-79.C	2023-12-05 17:04:14.235988879 +0100
@@ -0,0 +1,9 @@ 
+// { dg-do compile { target c++11 } }
+
+[[]] [[]];
+
+[[]] [[]] void
+foo ()
+{
+  [[]] [[]];
+}