[C++] Fix lambda error recovery (PR c++/84446)

Message ID 20180219190540.GQ5867@tucnak
State New
Headers show
Series
  • [C++] Fix lambda error recovery (PR c++/84446)
Related show

Commit Message

Jakub Jelinek Feb. 19, 2018, 7:05 p.m.
Hi!

In this case, because the corresponding variable is errorneous, we end up
with error_mark_node in LAMBDA_TYPE_EXTRA_SCOPE.  This patch just makes sure
we won't crash on it.  Not 100% sure if this is the best fix though.

Bootstrapped/regtested on x86_64-linux and i686-linux.

2018-02-19  Jakub Jelinek  <jakub@redhat.com>

	PR c++/84446
	* pt.c (template_class_depth): Don't crash if LAMBDA_TYPE_EXTRA_SCOPE
	is error_mark_node.

	* g++.dg/cpp0x/lambda/lambda-ice27.C: New test.


	Jakub

Comments

Paolo Carlini Feb. 19, 2018, 7:12 p.m. | #1
Hi,

On 19/02/2018 20:05, Jakub Jelinek wrote:
> Hi!

>

> In this case, because the corresponding variable is errorneous, we end up

> with error_mark_node in LAMBDA_TYPE_EXTRA_SCOPE.  This patch just makes sure

> we won't crash on it.  Not 100% sure if this is the best fix though.

IMHO something like the below - which just completed testing on 
x86_64-linux - could also make sense: among other things, we would catch 
the problem earlier - no need to check for error_mark_node as part of a 
loop - and the diagnostic would be more terse and identical to the 
non-template case. I should also add that normally when we use 
start_lambd_scope (decl) we *know* one way or the other that decl != 
error_mark_node, and that isn't the case here (in fact we check decl != 
error_mark_node in a couple of other places nearby)

Thanks!
Paolo.

//////////////////////
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 257817)
+++ cp/parser.c	(working copy)
@@ -19644,12 +19644,12 @@ cp_parser_init_declarator (cp_parser* parser,
 	     member templates.  The former involves deferring
 	     parsing of the initializer until end of class as with default
 	     arguments.  So right here we only handle the latter.  */
-	  if (!member_p && processing_template_decl)
+	  if (!member_p && processing_template_decl && decl != error_mark_node)
 	    start_lambda_scope (decl);
 	  initializer = cp_parser_initializer (parser,
 					       &is_direct_init,
 					       &is_non_constant_init);
-	  if (!member_p && processing_template_decl)
+	  if (!member_p && processing_template_decl && decl != error_mark_node)
 	    finish_lambda_scope ();
 	  if (initializer == error_mark_node)
 	    cp_parser_skip_to_end_of_statement (parser);
Index: testsuite/g++.dg/cpp0x/lambda/lambda-ice26.C
===================================================================
--- testsuite/g++.dg/cpp0x/lambda/lambda-ice26.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/lambda/lambda-ice26.C	(working copy)
@@ -0,0 +1,8 @@
+// PR c++/84446
+// { dg-do compile { target c++11 } }
+
+template<int> void foo()
+{
+  int i,
+  i = [] { virtual }();  // { dg-error "redefinition|expected" }
+}
Jakub Jelinek Feb. 19, 2018, 10:13 p.m. | #2
On Mon, Feb 19, 2018 at 08:12:48PM +0100, Paolo Carlini wrote:
> On 19/02/2018 20:05, Jakub Jelinek wrote:

> > Hi!

> > 

> > In this case, because the corresponding variable is errorneous, we end up

> > with error_mark_node in LAMBDA_TYPE_EXTRA_SCOPE.  This patch just makes sure

> > we won't crash on it.  Not 100% sure if this is the best fix though.

> IMHO something like the below - which just completed testing on x86_64-linux

> - could also make sense: among other things, we would catch the problem

> earlier - no need to check for error_mark_node as part of a loop - and the

> diagnostic would be more terse and identical to the non-template case. I

> should also add that normally when we use start_lambd_scope (decl) we *know*

> one way or the other that decl != error_mark_node, and that isn't the case

> here (in fact we check decl != error_mark_node in a couple of other places

> nearby)


Looks better to me indeed.

> Index: cp/parser.c

> ===================================================================

> --- cp/parser.c	(revision 257817)

> +++ cp/parser.c	(working copy)

> @@ -19644,12 +19644,12 @@ cp_parser_init_declarator (cp_parser* parser,

>  	     member templates.  The former involves deferring

>  	     parsing of the initializer until end of class as with default

>  	     arguments.  So right here we only handle the latter.  */

> -	  if (!member_p && processing_template_decl)

> +	  if (!member_p && processing_template_decl && decl != error_mark_node)

>  	    start_lambda_scope (decl);

>  	  initializer = cp_parser_initializer (parser,

>  					       &is_direct_init,

>  					       &is_non_constant_init);

> -	  if (!member_p && processing_template_decl)

> +	  if (!member_p && processing_template_decl && decl != error_mark_node)

>  	    finish_lambda_scope ();

>  	  if (initializer == error_mark_node)

>  	    cp_parser_skip_to_end_of_statement (parser);

> Index: testsuite/g++.dg/cpp0x/lambda/lambda-ice26.C

> ===================================================================

> --- testsuite/g++.dg/cpp0x/lambda/lambda-ice26.C	(nonexistent)

> +++ testsuite/g++.dg/cpp0x/lambda/lambda-ice26.C	(working copy)

> @@ -0,0 +1,8 @@

> +// PR c++/84446

> +// { dg-do compile { target c++11 } }

> +

> +template<int> void foo()

> +{

> +  int i,

> +  i = [] { virtual }();  // { dg-error "redefinition|expected" }

> +}



	Jakub
Jason Merrill Feb. 20, 2018, 3:10 a.m. | #3
OK.

On Mon, Feb 19, 2018 at 2:12 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,

>

> On 19/02/2018 20:05, Jakub Jelinek wrote:

>>

>> Hi!

>>

>> In this case, because the corresponding variable is errorneous, we end up

>> with error_mark_node in LAMBDA_TYPE_EXTRA_SCOPE.  This patch just makes

>> sure

>> we won't crash on it.  Not 100% sure if this is the best fix though.

>

> IMHO something like the below - which just completed testing on x86_64-linux

> - could also make sense: among other things, we would catch the problem

> earlier - no need to check for error_mark_node as part of a loop - and the

> diagnostic would be more terse and identical to the non-template case. I

> should also add that normally when we use start_lambd_scope (decl) we *know*

> one way or the other that decl != error_mark_node, and that isn't the case

> here (in fact we check decl != error_mark_node in a couple of other places

> nearby)

>

> Thanks!

> Paolo.

>

> //////////////////////

>

>

Patch

--- gcc/cp/pt.c.jj	2018-02-19 10:48:00.328184302 +0100
+++ gcc/cp/pt.c	2018-02-19 13:07:00.680855104 +0100
@@ -389,7 +389,11 @@  template_class_depth (tree type)
       if (DECL_P (type))
 	type = CP_DECL_CONTEXT (type);
       else if (LAMBDA_TYPE_P (type))
-	type = LAMBDA_TYPE_EXTRA_SCOPE (type);
+	{
+	  type = LAMBDA_TYPE_EXTRA_SCOPE (type);
+	  if (type == error_mark_node)
+	    break;
+	}
       else
 	type = CP_TYPE_CONTEXT (type);
     }
--- gcc/testsuite/g++.dg/cpp0x/lambda/lambda-ice27.C.jj	2018-02-19 13:11:15.292869216 +0100
+++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-ice27.C	2018-02-19 13:10:45.186870424 +0100
@@ -0,0 +1,12 @@ 
+// PR c++/84446
+// { dg-do compile { target c++11 } }
+
+template <int>
+void
+foo ()
+{
+  int i;
+  auto i = [] { virtual; }();	// { dg-error "conflicting declaration" }
+				// { dg-error "templates may not be 'virtual'" "" { target *-*-* } .-1 }
+				// { dg-error "declaration does not declare anything" "" { target *-*-* } .-2 }
+}