diff mbox series

[C++] Fix up lambda decl specifier parsing ICE (PR c++/90842)

Message ID 20191119234642.GQ4650@tucnak
State New
Headers show
Series [C++] Fix up lambda decl specifier parsing ICE (PR c++/90842) | expand

Commit Message

Jakub Jelinek Nov. 19, 2019, 11:46 p.m. UTC 
Hi!

In lambdas, the only valid decl specifiers are mutable, constexpr or
consteval.  For various other simple specifiers it is fine to parse them
and reject afterwards if the parsing is simple consuming of a single token
and setting some flags, but as the testcase shows, especially allowing
type specifiers, including new type definitions in there can cause ICEs.
The following patch punts for the cases where the parsing isn't that simple,
which I think is concept, typedef (there we e.g. commit tentative parsing)
or the type specifiers.

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

2019-11-19  Jakub Jelinek  <jakub@redhat.com>

	PR c++/90842
	* parser.c (cp_parser_decl_specifier_seq): For concept, typedef
	or type specifier with CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR
	don't try to parse them at all.

	* g++.dg/cpp1y/lambda-generic-90842.C: New test.
	* g++.dg/cpp0x/lambda/lambda-86550.C: Adjust expected diagnostics.


	Jakub

Comments

Jason Merrill Nov. 20, 2019, 4:35 a.m. UTC | #1 
On 11/19/19 11:46 PM, Jakub Jelinek wrote:
> Hi!
> 
> In lambdas, the only valid decl specifiers are mutable, constexpr or
> consteval.  For various other simple specifiers it is fine to parse them
> and reject afterwards if the parsing is simple consuming of a single token
> and setting some flags, but as the testcase shows, especially allowing
> type specifiers, including new type definitions in there can cause ICEs.
> The following patch punts for the cases where the parsing isn't that simple,
> which I think is concept, typedef (there we e.g. commit tentative parsing)
> or the type specifiers.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok of trunk?
> 
> 2019-11-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/90842
> 	* parser.c (cp_parser_decl_specifier_seq): For concept, typedef
> 	or type specifier with CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR
> 	don't try to parse them at all.
> 
> 	* g++.dg/cpp1y/lambda-generic-90842.C: New test.
> 	* g++.dg/cpp0x/lambda/lambda-86550.C: Adjust expected diagnostics.
> 
> --- gcc/cp/parser.c.jj	2019-11-14 09:13:24.356104252 +0100
> +++ gcc/cp/parser.c	2019-11-19 17:47:24.776014270 +0100
> @@ -14094,6 +14094,12 @@ cp_parser_decl_specifier_seq (cp_parser*
>   	  break;
>   
>           case RID_CONCEPT:
> +          if (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
> +            {
> +	      found_decl_spec = false;
> +	      break;
> +            }
> +
>             ds = ds_concept;
>             cp_lexer_consume_token (parser->lexer);

It would seem better to break after consuming the token, so we just skip 
the extra processing and still give the same error.

>   
> @@ -14136,6 +14142,12 @@ cp_parser_decl_specifier_seq (cp_parser*
>   	  /* decl-specifier:
>   	       typedef  */
>   	case RID_TYPEDEF:
> +          if (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
> +            {
> +	      found_decl_spec = false;
> +	      break;
> +            }
> +
>   	  ds = ds_typedef;
>   	  /* Consume the token.  */
>   	  cp_lexer_consume_token (parser->lexer);
> @@ -14229,7 +14241,9 @@ cp_parser_decl_specifier_seq (cp_parser*
>   
>         /* If we don't have a DECL_SPEC yet, then we must be looking at
>   	 a type-specifier.  */
> -      if (!found_decl_spec && !constructor_p)
> +      if (!found_decl_spec
> +	  && !constructor_p
> +	  && (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR) == 0)

And instead of this, maybe set CP_PARSER_FLAGS_NO_TYPE_DEFINITIONS so we 
keep the same diagnostic for other type-specifiers?

Jason
Jakub Jelinek Nov. 20, 2019, 9:16 a.m. UTC | #2 
On Tue, Nov 19, 2019 at 11:35:02PM -0500, Jason Merrill wrote:
> It would seem better to break after consuming the token, so we just skip the
> extra processing and still give the same error.
> 
> And instead of this, maybe set CP_PARSER_FLAGS_NO_TYPE_DEFINITIONS so we
> keep the same diagnostic for other type-specifiers?

That seems to work well.
So like this if it passes bootstrap/regtest?

2019-11-20  Jakub Jelinek  <jakub@redhat.com>
	    Jason Merrill  <jason@redhat.com>

	PR c++/90842
	* parser.c (cp_parser_decl_specifier_seq): For concept or typedef
	break early if CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR.
	For type specifiers, set CP_PARSER_FLAGS_NO_TYPE_DEFINITIONS
	if CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR is set.

	* g++.dg/cpp1y/lambda-generic-90842.C: New test.

--- gcc/cp/parser.c.jj	2019-11-19 22:26:46.171295670 +0100
+++ gcc/cp/parser.c	2019-11-20 10:05:27.243333499 +0100
@@ -14097,6 +14097,9 @@ cp_parser_decl_specifier_seq (cp_parser*
           ds = ds_concept;
           cp_lexer_consume_token (parser->lexer);
 
+	  if (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
+	    break;
+
           /* Warn for concept as a decl-specifier. We'll rewrite these as
              concept declarations later.  */
           if (!flag_concepts_ts)
@@ -14139,6 +14142,10 @@ cp_parser_decl_specifier_seq (cp_parser*
 	  ds = ds_typedef;
 	  /* Consume the token.  */
 	  cp_lexer_consume_token (parser->lexer);
+
+	  if (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
+	    break;
+
 	  /* A constructor declarator cannot appear in a typedef.  */
 	  constructor_possible_p = false;
 	  /* The "typedef" keyword can only occur in a declaration; we
@@ -14235,6 +14242,9 @@ cp_parser_decl_specifier_seq (cp_parser*
 	  bool is_cv_qualifier;
 	  tree type_spec;
 
+	  if (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
+	    flags |= CP_PARSER_FLAGS_NO_TYPE_DEFINITIONS;
+
 	  type_spec
 	    = cp_parser_type_specifier (parser, flags,
 					decl_specs,
--- gcc/testsuite/g++.dg/cpp1y/lambda-generic-90842.C.jj	2019-11-20 09:36:32.434390276 +0100
+++ gcc/testsuite/g++.dg/cpp1y/lambda-generic-90842.C	2019-11-20 10:08:18.037769485 +0100
@@ -0,0 +1,10 @@
+// PR c++/90842
+// { dg-do compile { target c++14 } }
+
+auto a = [](auto x) struct C { void foo (); } {};	// { dg-error "expected" }
+							// { dg-error "type-specifier invalid in lambda" "" { target *-*-* } .-1 }
+auto b = [](auto x) mutable typedef {};			// { dg-error "'typedef' invalid in lambda" }
+#if __cpp_concepts >= 201907L
+auto c = [](auto x) constexpr concept {};		// { dg-error "'concept' invalid in lambda" "" { target c++2a } }
+#endif
+auto d = [](auto x) mutable friend {};			// { dg-error "'friend' invalid in lambda" }


	Jakub
Jason Merrill Nov. 20, 2019, 6:48 p.m. UTC | #3 
On 11/20/19 9:16 AM, Jakub Jelinek wrote:
> On Tue, Nov 19, 2019 at 11:35:02PM -0500, Jason Merrill wrote:
>> It would seem better to break after consuming the token, so we just skip the
>> extra processing and still give the same error.
>>
>> And instead of this, maybe set CP_PARSER_FLAGS_NO_TYPE_DEFINITIONS so we
>> keep the same diagnostic for other type-specifiers?
> 
> That seems to work well.
> So like this if it passes bootstrap/regtest?

OK.

Jason
diff mbox series

Patch

--- gcc/cp/parser.c.jj	2019-11-14 09:13:24.356104252 +0100
+++ gcc/cp/parser.c	2019-11-19 17:47:24.776014270 +0100
@@ -14094,6 +14094,12 @@  cp_parser_decl_specifier_seq (cp_parser*
 	  break;
 
         case RID_CONCEPT:
+          if (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
+            {
+	      found_decl_spec = false;
+	      break;
+            }
+
           ds = ds_concept;
           cp_lexer_consume_token (parser->lexer);
 
@@ -14136,6 +14142,12 @@  cp_parser_decl_specifier_seq (cp_parser*
 	  /* decl-specifier:
 	       typedef  */
 	case RID_TYPEDEF:
+          if (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
+            {
+	      found_decl_spec = false;
+	      break;
+            }
+
 	  ds = ds_typedef;
 	  /* Consume the token.  */
 	  cp_lexer_consume_token (parser->lexer);
@@ -14229,7 +14241,9 @@  cp_parser_decl_specifier_seq (cp_parser*
 
       /* If we don't have a DECL_SPEC yet, then we must be looking at
 	 a type-specifier.  */
-      if (!found_decl_spec && !constructor_p)
+      if (!found_decl_spec
+	  && !constructor_p
+	  && (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR) == 0)
 	{
 	  int decl_spec_declares_class_or_enum;
 	  bool is_cv_qualifier;
@@ -14288,9 +14302,6 @@  cp_parser_decl_specifier_seq (cp_parser*
 	      found_decl_spec = true;
 	      if (!is_cv_qualifier)
 		decl_specs->any_type_specifiers_p = true;
-
-	      if ((flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR) != 0)
-		error_at (token->location, "type-specifier invalid in lambda");
 	    }
 	}
 
--- gcc/testsuite/g++.dg/cpp1y/lambda-generic-90842.C.jj	2019-11-19 17:53:37.682440002 +0100
+++ gcc/testsuite/g++.dg/cpp1y/lambda-generic-90842.C	2019-11-19 17:53:01.815976144 +0100
@@ -0,0 +1,4 @@ 
+// PR c++/90842
+// { dg-do compile { target c++14 } }
+
+auto a = [](auto x) struct C { void foo (); } {};	// { dg-error "expected" }
--- gcc/testsuite/g++.dg/cpp0x/lambda/lambda-86550.C.jj	2018-07-18 23:01:22.824082949 +0200
+++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-86550.C	2019-11-19 17:54:26.978703112 +0100
@@ -4,6 +4,6 @@ 
 void
 foo ()
 {
-  auto a = []() bool {};			// { dg-error "type-specifier invalid in lambda" }
-  auto b = []() bool bool bool bool int {};	// { dg-error "type-specifier invalid in lambda" }
+  auto a = []() bool {};			// { dg-error "expected" }
+  auto b = []() bool bool bool bool int {};	// { dg-error "expected" }
 }