diff mbox series

c++: Disallow this specifier except for parameter declarations [PR113788]

Message ID ZcKYmBgorieFShB4@tucnak
State Unresolved
Headers
Series c++: Disallow this specifier except for parameter declarations [PR113788] |

Checks

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

Commit Message

Jakub Jelinek Feb. 6, 2024, 8:37 p.m. UTC 
  Hi!

The deducing this patchset added parsing of this specifier to
cp_parser_decl_specifier_seq unconditionally, but in the C++ grammar
this[opt] only appears in the parameter-declaration non-terminal, so
rather than checking in all the callers of cp_parser_decl_specifier_seq
except for cp_parser_parameter_declaration that this specifier didn't
appear I think it is far easier and closer to what the standard says
to only parse this specifier when called from
cp_parser_parameter_declaration.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-02-06  Jakub Jelinek  <jakub@redhat.com>

	PR c++/113788
	* parser.cc (CP_PARSER_FLAGS_PARAMETER): New enumerator.
	(cp_parser_decl_specifier_seq): Parse RID_THIS only if
	CP_PARSER_FLAGS_PARAMETER is set in flags.
	(cp_parser_parameter_declaration): Or in CP_PARSER_FLAGS_PARAMETER
	when calling cp_parser_decl_specifier_seq.

	* g++.dg/parse/pr113788.C: New test.


	Jakub

Comments

Marek Polacek Feb. 6, 2024, 8:45 p.m. UTC | #1 
On Tue, Feb 06, 2024 at 09:37:44PM +0100, Jakub Jelinek wrote:
> Hi!
> 
> The deducing this patchset added parsing of this specifier to
> cp_parser_decl_specifier_seq unconditionally, but in the C++ grammar
> this[opt] only appears in the parameter-declaration non-terminal, so
> rather than checking in all the callers of cp_parser_decl_specifier_seq
> except for cp_parser_parameter_declaration that this specifier didn't
> appear I think it is far easier and closer to what the standard says
> to only parse this specifier when called from
> cp_parser_parameter_declaration.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

FWIW, the patch looks good to me.

> 2024-02-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/113788
> 	* parser.cc (CP_PARSER_FLAGS_PARAMETER): New enumerator.
> 	(cp_parser_decl_specifier_seq): Parse RID_THIS only if
> 	CP_PARSER_FLAGS_PARAMETER is set in flags.
> 	(cp_parser_parameter_declaration): Or in CP_PARSER_FLAGS_PARAMETER
> 	when calling cp_parser_decl_specifier_seq.
> 
> 	* g++.dg/parse/pr113788.C: New test.
> 
> --- gcc/cp/parser.cc.jj	2024-01-17 10:34:45.337660930 +0100
> +++ gcc/cp/parser.cc	2024-02-06 18:31:35.587193903 +0100
> @@ -2088,7 +2088,9 @@ enum
>    /* When parsing of the noexcept-specifier should be delayed.  */
>    CP_PARSER_FLAGS_DELAY_NOEXCEPT = 0x40,
>    /* When parsing a consteval declarator.  */
> -  CP_PARSER_FLAGS_CONSTEVAL = 0x80
> +  CP_PARSER_FLAGS_CONSTEVAL = 0x80,
> +  /* When parsing a parameter declaration.  */
> +  CP_PARSER_FLAGS_PARAMETER = 0x100
>  };
>  
>  /* This type is used for parameters and variables which hold
> @@ -16342,7 +16344,7 @@ cp_parser_decl_specifier_seq (cp_parser*
>        /* Special case for "this" specifier, indicating a parm is an xobj parm.
>  	 The "this" specifier must be the first specifier in the declaration,
>  	 after any attributes.  */
> -      if (token->keyword == RID_THIS)
> +      if (token->keyword == RID_THIS && (flags & CP_PARSER_FLAGS_PARAMETER))
>  	{
>  	  cp_lexer_consume_token (parser->lexer);
>  	  if (token != first_specifier)
> @@ -25607,7 +25609,7 @@ cp_parser_parameter_declaration (cp_pars
>    /* Parse the declaration-specifiers.  */
>    cp_token *decl_spec_token_start = cp_lexer_peek_token (parser->lexer);
>    cp_parser_decl_specifier_seq (parser,
> -				flags,
> +				flags | CP_PARSER_FLAGS_PARAMETER,
>  				&decl_specifiers,
>  				&declares_class_or_enum);
>  
> --- gcc/testsuite/g++.dg/parse/pr113788.C.jj	2024-02-06 18:40:29.553791028 +0100
> +++ gcc/testsuite/g++.dg/parse/pr113788.C	2024-02-06 18:41:23.326045703 +0100
> @@ -0,0 +1,20 @@
> +// PR c++/113788
> +// { dg-do compile { target c++11 } }
> +
> +struct S { int a, b; };
> +struct U {
> +  void foo () { this int g = 1; }	// { dg-error "expected ';' before 'int'" }
> +};
> +this auto h = 1;			// { dg-error "expected unqualified-id before 'this'" }
> +
> +int
> +main ()
> +{
> +  S s = { 1, 2 };
> +  short t[3] = { 3, 4, 5 };
> +  this auto &[a, b] = s;		// { dg-error "invalid use of 'this' in non-member function" }
> +  this auto &[c, d, e] = t;		// { dg-error "invalid use of 'this' in non-member function" }
> +  this int f = 1;			// { dg-error "invalid use of 'this' in non-member function" }
> +  for (this auto &i : t)		// { dg-error "invalid use of 'this' in non-member function" }
> +    ;					// { dg-error "expected" }
> +}					// { dg-error "expected" }
> 
> 	Jakub
> 

Marek
Jason Merrill Feb. 6, 2024, 9:32 p.m. UTC | #2 
On 2/6/24 15:45, Marek Polacek wrote:
> On Tue, Feb 06, 2024 at 09:37:44PM +0100, Jakub Jelinek wrote:
>> Hi!
>>
>> The deducing this patchset added parsing of this specifier to
>> cp_parser_decl_specifier_seq unconditionally, but in the C++ grammar
>> this[opt] only appears in the parameter-declaration non-terminal, so
>> rather than checking in all the callers of cp_parser_decl_specifier_seq
>> except for cp_parser_parameter_declaration that this specifier didn't
>> appear I think it is far easier and closer to what the standard says
>> to only parse this specifier when called from
>> cp_parser_parameter_declaration.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> FWIW, the patch looks good to me.

Agreed, OK.

Jason
diff mbox series

Patch

--- gcc/cp/parser.cc.jj	2024-01-17 10:34:45.337660930 +0100
+++ gcc/cp/parser.cc	2024-02-06 18:31:35.587193903 +0100
@@ -2088,7 +2088,9 @@  enum
   /* When parsing of the noexcept-specifier should be delayed.  */
   CP_PARSER_FLAGS_DELAY_NOEXCEPT = 0x40,
   /* When parsing a consteval declarator.  */
-  CP_PARSER_FLAGS_CONSTEVAL = 0x80
+  CP_PARSER_FLAGS_CONSTEVAL = 0x80,
+  /* When parsing a parameter declaration.  */
+  CP_PARSER_FLAGS_PARAMETER = 0x100
 };
 
 /* This type is used for parameters and variables which hold
@@ -16342,7 +16344,7 @@  cp_parser_decl_specifier_seq (cp_parser*
       /* Special case for "this" specifier, indicating a parm is an xobj parm.
 	 The "this" specifier must be the first specifier in the declaration,
 	 after any attributes.  */
-      if (token->keyword == RID_THIS)
+      if (token->keyword == RID_THIS && (flags & CP_PARSER_FLAGS_PARAMETER))
 	{
 	  cp_lexer_consume_token (parser->lexer);
 	  if (token != first_specifier)
@@ -25607,7 +25609,7 @@  cp_parser_parameter_declaration (cp_pars
   /* Parse the declaration-specifiers.  */
   cp_token *decl_spec_token_start = cp_lexer_peek_token (parser->lexer);
   cp_parser_decl_specifier_seq (parser,
-				flags,
+				flags | CP_PARSER_FLAGS_PARAMETER,
 				&decl_specifiers,
 				&declares_class_or_enum);
 
--- gcc/testsuite/g++.dg/parse/pr113788.C.jj	2024-02-06 18:40:29.553791028 +0100
+++ gcc/testsuite/g++.dg/parse/pr113788.C	2024-02-06 18:41:23.326045703 +0100
@@ -0,0 +1,20 @@ 
+// PR c++/113788
+// { dg-do compile { target c++11 } }
+
+struct S { int a, b; };
+struct U {
+  void foo () { this int g = 1; }	// { dg-error "expected ';' before 'int'" }
+};
+this auto h = 1;			// { dg-error "expected unqualified-id before 'this'" }
+
+int
+main ()
+{
+  S s = { 1, 2 };
+  short t[3] = { 3, 4, 5 };
+  this auto &[a, b] = s;		// { dg-error "invalid use of 'this' in non-member function" }
+  this auto &[c, d, e] = t;		// { dg-error "invalid use of 'this' in non-member function" }
+  this int f = 1;			// { dg-error "invalid use of 'this' in non-member function" }
+  for (this auto &i : t)		// { dg-error "invalid use of 'this' in non-member function" }
+    ;					// { dg-error "expected" }
+}					// { dg-error "expected" }