[C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1922 bytes --]

Hi,

not considering the various rather mild error recovery regressions which 
we have in this area, I have been working on and off on these issues for 
a while, as QoI diagnostic improvements: when something wrong happens 
while parsing the template parameter lists we want anyway to keep track 
of all the parameters, erroneous and not, and we go on handling the body 
of the class (we are mostly considering classes here but not necessarily 
so in the future). Then, very often, during error recovery the hell 
breaks loose and we emit a ton of redundant diagnostic about member 
function calls, overload resolution, etc, all due to the broken template 
parameters. Today finally I had this very simply (but a bit invasive in 
this Stage!?!) idea which, of all those I tried (like, skipping 
completely the body, etc) is the one which so far works best, appears to 
strike a good balance between keeping information useful for error 
recovery and skipping those manipulations which, when the parameters are 
broken, are bound to fail anyway. Thus the idea would be adding a flag 
to struct cp_parser, set it when cp_parser_template_parameter returns an 
error and use it later in cp_parser_class_specifier_1 in order to skip 
parsing the queued function definitions. The solution passes the 
testusite as-is and, while a bit invasive, seems rather safe, because, 
if I'm not badly mistaken, it's about recovery, isn't that we are 
risking rejecting valid code. What do you think? Well, I final thing: I 
briefly wondered today if adding the flag is an issue from the memory 
footprint point of view: I don't think so, but, in case, it seems to me 
that cp_parser could be improved in many ways - even by just reordering 
the members or using bitfields?

Thanks! Paolo.

PS: if we want to do something super-safe about the regressions, we 
still have the Plan B in Comment #5 of c++/71832.

///////////////////////////


[-- Attachment #2: CL_71169_71832 --]
[-- Type: text/plain, Size: 956 bytes --]

/cp
2018-03-06

	PR c++/71169
	PR c++/71832
	* parser.h (struct cp_parser): Add error_in_template_parameter_list_p
	data member.
	* parser.c (cp_debug_parser): Print the latter.
	(cp_parser_new): Initialize it.
	(cp_parser_lambda_expression): Save and restore it.
	(cp_parser_lambda_declarator_opt): Maybe reset it.
	(cp_parser_template_parameter_list): Set it in case of error.
	(cp_parser_explicit_specialization): Maybe reset it.
	(cp_parser_class_specifier_1): Save and restore it; if it's true
	do not parse the bodies of the queued function definitions.
	(cp_parser_class_head): Maybe reset it.
	(cp_parser_constructor_declarator_p): Save and restore it.
	(cp_parser_function_definition_after_declarator): Likewise.
	(cp_parser_template_declaration_after_parameters): Maybe reset it.
	(finish_fully_implicit_template): Likewise.

/testsuite
2018-03-06

	PR c++/71169
	PR c++/71832
	* g++.dg/cpp0x/pr71169.C: New.
	* g++.dg/cpp0x/pr71832.C: Likewise.

[-- Attachment #3: patch_71169_71832 --]
[-- Type: text/plain, Size: 10041 bytes --]

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 258264)
+++ cp/parser.c	(working copy)
@@ -565,6 +565,8 @@ cp_debug_parser (FILE *file, cp_parser *parser)
 			      "local class", parser->in_function_body);
   cp_debug_print_flag (file, "Auto correct a colon to a scope operator",
 			      parser->colon_corrects_to_scope_p);
+  cp_debug_print_flag (file, "Error in template parameter list",
+			      parser->error_in_template_parameter_list_p);
   cp_debug_print_flag (file, "Colon doesn't start a class definition",
 			      parser->colon_doesnt_start_class_def_p);
   if (parser->type_definition_forbidden_message)
@@ -3910,6 +3912,9 @@ cp_parser_new (void)
   /* We can correct until told otherwise.  */
   parser->colon_corrects_to_scope_p = true;
 
+  /* No error so far.  */
+  parser->error_in_template_parameter_list_p = false;
+
   /* The unparsed function queue is empty.  */
   push_unparsed_function_queues (parser);
 
@@ -10151,6 +10156,8 @@ cp_parser_lambda_expression (cp_parser* parser)
     /* Inside the class, surrounding template-parameter-lists do not apply.  */
     unsigned int saved_num_template_parameter_lists
         = parser->num_template_parameter_lists;
+    bool saved_error_in_template_parameter_list_p
+        = parser->error_in_template_parameter_list_p;
     unsigned char in_statement = parser->in_statement;
     bool in_switch_statement_p = parser->in_switch_statement_p;
     bool fully_implicit_function_template_p
@@ -10161,6 +10168,7 @@ cp_parser_lambda_expression (cp_parser* parser)
         = parser->auto_is_implicit_function_template_parm_p;
 
     parser->num_template_parameter_lists = 0;
+    parser->error_in_template_parameter_list_p = false;
     parser->in_statement = 0;
     parser->in_switch_statement_p = false;
     parser->fully_implicit_function_template_p = false;
@@ -10199,6 +10207,8 @@ cp_parser_lambda_expression (cp_parser* parser)
     type = finish_struct (type, /*attributes=*/NULL_TREE);
 
     parser->num_template_parameter_lists = saved_num_template_parameter_lists;
+    parser->error_in_template_parameter_list_p
+       = saved_error_in_template_parameter_list_p;
     parser->in_statement = in_statement;
     parser->in_switch_statement_p = in_switch_statement_p;
     parser->fully_implicit_function_template_p
@@ -10624,7 +10634,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser
       {
 	fco = finish_member_template_decl (fco);
 	finish_template_decl (template_param_list);
-	--parser->num_template_parameter_lists;
+	if (--parser->num_template_parameter_lists == 0)
+	  parser->error_in_template_parameter_list_p = false;
       }
     else if (parser->fully_implicit_function_template_p)
       fco = finish_fully_implicit_template (parser, fco);
@@ -15065,6 +15076,11 @@ cp_parser_template_parameter_list (cp_parser* pars
          parameter_list = chainon (parameter_list, err_parm);
        }
 
+      if (parameter == error_mark_node
+	  || error_operand_p (TREE_VALUE (parameter))
+	  || error_operand_p (TREE_PURPOSE (parameter)))
+	parser->error_in_template_parameter_list_p = true;
+
       /* If the next token is not a `,', we're done.  */
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
 	break;
@@ -16673,7 +16689,8 @@ cp_parser_explicit_specialization (cp_parser* pars
   if (need_lang_pop)
     pop_lang_context ();
   /* We're done with this parameter list.  */
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 }
 
 /* Parse a type-specifier.
@@ -22438,6 +22455,7 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   tree attributes = NULL_TREE;
   bool nested_name_specifier_p;
   unsigned saved_num_template_parameter_lists;
+  bool saved_error_in_template_parameter_list_p;
   bool saved_in_function_body;
   unsigned char in_statement;
   bool in_switch_statement_p;
@@ -22480,6 +22498,9 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   saved_num_template_parameter_lists
     = parser->num_template_parameter_lists;
   parser->num_template_parameter_lists = 0;
+  saved_error_in_template_parameter_list_p
+    = parser->error_in_template_parameter_list_p;
+  parser->error_in_template_parameter_list_p = false;
   /* We are not in a function body.  */
   saved_in_function_body = parser->in_function_body;
   parser->in_function_body = false;
@@ -22658,7 +22679,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
        struct A::B { void f() { } };
 
      there is no need to delay the parsing of `A::B::f'.  */
-  if (--parser->num_classes_being_defined == 0)
+  if (--parser->num_classes_being_defined == 0
+      && !saved_error_in_template_parameter_list_p)
     {
       tree decl;
       tree class_type = NULL_TREE;
@@ -22752,6 +22774,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   parser->in_function_body = saved_in_function_body;
   parser->num_template_parameter_lists
     = saved_num_template_parameter_lists;
+  parser->error_in_template_parameter_list_p
+    = saved_error_in_template_parameter_list_p;
   parser->in_unbraced_linkage_specification_p
     = saved_in_unbraced_linkage_specification_p;
 
@@ -23253,7 +23277,8 @@ cp_parser_class_head (cp_parser* parser,
   if (invalid_explicit_specialization_p)
     {
       end_specialization ();
-      --parser->num_template_parameter_lists;
+      if (--parser->num_template_parameter_lists == 0)
+	parser->error_in_template_parameter_list_p = false;
     }
 
   if (type)
@@ -26616,6 +26641,7 @@ cp_parser_constructor_declarator_p (cp_parser *par
 	  tree type;
 	  tree pushed_scope = NULL_TREE;
 	  unsigned saved_num_template_parameter_lists;
+	  bool saved_error_in_template_parameter_list_p;
 
 	  /* Names appearing in the type-specifier should be looked up
 	     in the scope of the class.  */
@@ -26642,6 +26668,9 @@ cp_parser_constructor_declarator_p (cp_parser *par
 	  saved_num_template_parameter_lists
 	    = parser->num_template_parameter_lists;
 	  parser->num_template_parameter_lists = 0;
+	  saved_error_in_template_parameter_list_p
+	    = parser->error_in_template_parameter_list_p;
+	  parser->error_in_template_parameter_list_p = false;
 
 	  /* Look for the type-specifier.  */
 	  cp_parser_type_specifier (parser,
@@ -26653,6 +26682,8 @@ cp_parser_constructor_declarator_p (cp_parser *par
 
 	  parser->num_template_parameter_lists
 	    = saved_num_template_parameter_lists;
+	  parser->error_in_template_parameter_list_p
+	    = saved_error_in_template_parameter_list_p;
 
 	  /* Leave the scope of the class.  */
 	  if (pushed_scope)
@@ -26751,6 +26782,7 @@ cp_parser_function_definition_after_declarator (cp
   bool saved_in_unbraced_linkage_specification_p;
   bool saved_in_function_body;
   unsigned saved_num_template_parameter_lists;
+  bool saved_error_in_template_parameter_list_p;
   cp_token *token;
   bool fully_implicit_function_template_p
     = parser->fully_implicit_function_template_p;
@@ -26799,6 +26831,9 @@ cp_parser_function_definition_after_declarator (cp
   saved_num_template_parameter_lists
     = parser->num_template_parameter_lists;
   parser->num_template_parameter_lists = 0;
+  saved_error_in_template_parameter_list_p
+    = parser->error_in_template_parameter_list_p;
+  parser->error_in_template_parameter_list_p = false;
 
   /* If the next token is `try', `__transaction_atomic', or
      `__transaction_relaxed`, then we are looking at either function-try-block
@@ -26824,6 +26859,8 @@ cp_parser_function_definition_after_declarator (cp
     = saved_in_unbraced_linkage_specification_p;
   parser->num_template_parameter_lists
     = saved_num_template_parameter_lists;
+  parser->error_in_template_parameter_list_p
+    = saved_error_in_template_parameter_list_p;
   parser->in_function_body = saved_in_function_body;
 
   parser->fully_implicit_function_template_p
@@ -26893,7 +26930,8 @@ cp_parser_template_declaration_after_parameters (c
 			   /*complain=*/true);
     }
   /* We are done with the current parameter list.  */
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 
   pop_deferring_access_checks ();
 
@@ -39249,7 +39287,8 @@ finish_fully_implicit_template (cp_parser *parser,
   end_template_decl ();
 
   parser->fully_implicit_function_template_p = false;
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 
   return member_decl_opt;
 }
Index: cp/parser.h
===================================================================
--- cp/parser.h	(revision 258264)
+++ cp/parser.h	(working copy)
@@ -346,6 +346,9 @@ struct GTY(()) cp_parser {
      is terminated by colon.  */
   bool colon_doesnt_start_class_def_p;
 
+  /* TRUE if there was an error in a template parameter list.  */
+  bool error_in_template_parameter_list_p;
+
   /* If non-NULL, then we are parsing a construct where new type
      definitions are not permitted.  The string stored here will be
      issued as an error message if a type is defined.  */
Index: testsuite/g++.dg/cpp0x/pr71169.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
Index: testsuite/g++.dg/cpp0x/pr71832.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71832.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71832.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template < typename decltype (0) > struct A  // { dg-error "expected|two or more" }
+{ 
+  void foo () { baz (); }
+  template < typename ... S > void baz () {}
+};

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Paolo Carlini]

.. grrrr, consider the patch withdrawn in its detailed form, I have a 
new testcase which add some random code after that ill-formed class and 
it's mishandled, it ICEs again.

Well, I would still appreciate some feedback about the general idea...

Paolo.

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

Hi again,

On 06/03/2018 19:58, Paolo Carlini wrote:
> .. grrrr, consider the patch withdrawn in its detailed form, I have a 
> new testcase which add some random code after that ill-formed class 
> and it's mishandled, it ICEs again.
The below avoids the idiotic thinko (I added a testcase for that) and is 
likewise passing testing.

Thanks,
Paolo.

/////////////////////////

[-- Attachment #2: patch_71169_71832_a --]
[-- Type: text/plain, Size: 10729 bytes --]

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 258264)
+++ cp/parser.c	(working copy)
@@ -565,6 +565,8 @@ cp_debug_parser (FILE *file, cp_parser *parser)
 			      "local class", parser->in_function_body);
   cp_debug_print_flag (file, "Auto correct a colon to a scope operator",
 			      parser->colon_corrects_to_scope_p);
+  cp_debug_print_flag (file, "Error in template parameter list",
+			      parser->error_in_template_parameter_list_p);
   cp_debug_print_flag (file, "Colon doesn't start a class definition",
 			      parser->colon_doesnt_start_class_def_p);
   if (parser->type_definition_forbidden_message)
@@ -3910,6 +3912,9 @@ cp_parser_new (void)
   /* We can correct until told otherwise.  */
   parser->colon_corrects_to_scope_p = true;
 
+  /* No error so far.  */
+  parser->error_in_template_parameter_list_p = false;
+
   /* The unparsed function queue is empty.  */
   push_unparsed_function_queues (parser);
 
@@ -10151,6 +10156,8 @@ cp_parser_lambda_expression (cp_parser* parser)
     /* Inside the class, surrounding template-parameter-lists do not apply.  */
     unsigned int saved_num_template_parameter_lists
         = parser->num_template_parameter_lists;
+    bool saved_error_in_template_parameter_list_p
+        = parser->error_in_template_parameter_list_p;
     unsigned char in_statement = parser->in_statement;
     bool in_switch_statement_p = parser->in_switch_statement_p;
     bool fully_implicit_function_template_p
@@ -10161,6 +10168,7 @@ cp_parser_lambda_expression (cp_parser* parser)
         = parser->auto_is_implicit_function_template_parm_p;
 
     parser->num_template_parameter_lists = 0;
+    parser->error_in_template_parameter_list_p = false;
     parser->in_statement = 0;
     parser->in_switch_statement_p = false;
     parser->fully_implicit_function_template_p = false;
@@ -10199,6 +10207,8 @@ cp_parser_lambda_expression (cp_parser* parser)
     type = finish_struct (type, /*attributes=*/NULL_TREE);
 
     parser->num_template_parameter_lists = saved_num_template_parameter_lists;
+    parser->error_in_template_parameter_list_p
+       = saved_error_in_template_parameter_list_p;
     parser->in_statement = in_statement;
     parser->in_switch_statement_p = in_switch_statement_p;
     parser->fully_implicit_function_template_p
@@ -10624,7 +10634,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser
       {
 	fco = finish_member_template_decl (fco);
 	finish_template_decl (template_param_list);
-	--parser->num_template_parameter_lists;
+	if (--parser->num_template_parameter_lists == 0)
+	  parser->error_in_template_parameter_list_p = false;
       }
     else if (parser->fully_implicit_function_template_p)
       fco = finish_fully_implicit_template (parser, fco);
@@ -15065,6 +15076,11 @@ cp_parser_template_parameter_list (cp_parser* pars
          parameter_list = chainon (parameter_list, err_parm);
        }
 
+      if (parameter == error_mark_node
+	  || error_operand_p (TREE_VALUE (parameter))
+	  || error_operand_p (TREE_PURPOSE (parameter)))
+	parser->error_in_template_parameter_list_p = true;
+
       /* If the next token is not a `,', we're done.  */
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
 	break;
@@ -16673,7 +16689,8 @@ cp_parser_explicit_specialization (cp_parser* pars
   if (need_lang_pop)
     pop_lang_context ();
   /* We're done with this parameter list.  */
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 }
 
 /* Parse a type-specifier.
@@ -22438,6 +22455,7 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   tree attributes = NULL_TREE;
   bool nested_name_specifier_p;
   unsigned saved_num_template_parameter_lists;
+  bool saved_error_in_template_parameter_list_p;
   bool saved_in_function_body;
   unsigned char in_statement;
   bool in_switch_statement_p;
@@ -22480,6 +22498,9 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   saved_num_template_parameter_lists
     = parser->num_template_parameter_lists;
   parser->num_template_parameter_lists = 0;
+  saved_error_in_template_parameter_list_p
+    = parser->error_in_template_parameter_list_p;
+  parser->error_in_template_parameter_list_p = false;
   /* We are not in a function body.  */
   saved_in_function_body = parser->in_function_body;
   parser->in_function_body = false;
@@ -22658,8 +22679,13 @@ cp_parser_class_specifier_1 (cp_parser* parser)
        struct A::B { void f() { } };
 
      there is no need to delay the parsing of `A::B::f'.  */
-  if (--parser->num_classes_being_defined == 0)
+  if (saved_error_in_template_parameter_list_p)
     {
+      /* Do nothing, we skip the bodies in order to improve error
+	 recovery (c++/71169, c++/71832, etc).  */;
+    }
+  else if (--parser->num_classes_being_defined == 0)
+    {
       tree decl;
       tree class_type = NULL_TREE;
       tree pushed_scope = NULL_TREE;
@@ -22752,6 +22778,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   parser->in_function_body = saved_in_function_body;
   parser->num_template_parameter_lists
     = saved_num_template_parameter_lists;
+  parser->error_in_template_parameter_list_p
+    = saved_error_in_template_parameter_list_p;
   parser->in_unbraced_linkage_specification_p
     = saved_in_unbraced_linkage_specification_p;
 
@@ -23253,7 +23281,8 @@ cp_parser_class_head (cp_parser* parser,
   if (invalid_explicit_specialization_p)
     {
       end_specialization ();
-      --parser->num_template_parameter_lists;
+      if (--parser->num_template_parameter_lists == 0)
+	parser->error_in_template_parameter_list_p = false;
     }
 
   if (type)
@@ -26616,6 +26645,7 @@ cp_parser_constructor_declarator_p (cp_parser *par
 	  tree type;
 	  tree pushed_scope = NULL_TREE;
 	  unsigned saved_num_template_parameter_lists;
+	  bool saved_error_in_template_parameter_list_p;
 
 	  /* Names appearing in the type-specifier should be looked up
 	     in the scope of the class.  */
@@ -26642,6 +26672,9 @@ cp_parser_constructor_declarator_p (cp_parser *par
 	  saved_num_template_parameter_lists
 	    = parser->num_template_parameter_lists;
 	  parser->num_template_parameter_lists = 0;
+	  saved_error_in_template_parameter_list_p
+	    = parser->error_in_template_parameter_list_p;
+	  parser->error_in_template_parameter_list_p = false;
 
 	  /* Look for the type-specifier.  */
 	  cp_parser_type_specifier (parser,
@@ -26653,6 +26686,8 @@ cp_parser_constructor_declarator_p (cp_parser *par
 
 	  parser->num_template_parameter_lists
 	    = saved_num_template_parameter_lists;
+	  parser->error_in_template_parameter_list_p
+	    = saved_error_in_template_parameter_list_p;
 
 	  /* Leave the scope of the class.  */
 	  if (pushed_scope)
@@ -26751,6 +26786,7 @@ cp_parser_function_definition_after_declarator (cp
   bool saved_in_unbraced_linkage_specification_p;
   bool saved_in_function_body;
   unsigned saved_num_template_parameter_lists;
+  bool saved_error_in_template_parameter_list_p;
   cp_token *token;
   bool fully_implicit_function_template_p
     = parser->fully_implicit_function_template_p;
@@ -26799,6 +26835,9 @@ cp_parser_function_definition_after_declarator (cp
   saved_num_template_parameter_lists
     = parser->num_template_parameter_lists;
   parser->num_template_parameter_lists = 0;
+  saved_error_in_template_parameter_list_p
+    = parser->error_in_template_parameter_list_p;
+  parser->error_in_template_parameter_list_p = false;
 
   /* If the next token is `try', `__transaction_atomic', or
      `__transaction_relaxed`, then we are looking at either function-try-block
@@ -26824,6 +26863,8 @@ cp_parser_function_definition_after_declarator (cp
     = saved_in_unbraced_linkage_specification_p;
   parser->num_template_parameter_lists
     = saved_num_template_parameter_lists;
+  parser->error_in_template_parameter_list_p
+    = saved_error_in_template_parameter_list_p;
   parser->in_function_body = saved_in_function_body;
 
   parser->fully_implicit_function_template_p
@@ -26893,7 +26934,8 @@ cp_parser_template_declaration_after_parameters (c
 			   /*complain=*/true);
     }
   /* We are done with the current parameter list.  */
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 
   pop_deferring_access_checks ();
 
@@ -39249,7 +39291,8 @@ finish_fully_implicit_template (cp_parser *parser,
   end_template_decl ();
 
   parser->fully_implicit_function_template_p = false;
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 
   return member_decl_opt;
 }
Index: cp/parser.h
===================================================================
--- cp/parser.h	(revision 258264)
+++ cp/parser.h	(working copy)
@@ -346,6 +346,9 @@ struct GTY(()) cp_parser {
      is terminated by colon.  */
   bool colon_doesnt_start_class_def_p;
 
+  /* TRUE if there was an error in a template parameter list.  */
+  bool error_in_template_parameter_list_p;
+
   /* If non-NULL, then we are parsing a construct where new type
      definitions are not permitted.  The string stored here will be
      issued as an error message if a type is defined.  */
Index: testsuite/g++.dg/cpp0x/pr71169-2.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169-2.C	(working copy)
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
+
+template<typename>
+struct B
+{
+  int f() { return 0; }
+};
+
+int main()
+{
+  B<int> b;
+  return b.f();
+}
Index: testsuite/g++.dg/cpp0x/pr71169.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
Index: testsuite/g++.dg/cpp0x/pr71832.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71832.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71832.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template < typename decltype (0) > struct A  // { dg-error "expected|two or more" }
+{ 
+  void foo () { baz (); }
+  template < typename ... S > void baz () {}
+};

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Jason Merrill]

Interesting, that seems like a promising idea.  I'm not sure we need
to do this based on an error in a default template arg, though; can we
drop

+  || error_operand_p (TREE_PURPOSE (parameter)))

?

Jason

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

Hi,

On 06/03/2018 21:33, Jason Merrill wrote:
> Interesting, that seems like a promising idea.  I'm not sure we need
> to do this based on an error in a default template arg, though; can we
> drop
>
> +  || error_operand_p (TREE_PURPOSE (parameter)))
>
> ?
Good point, yes, I believe we can, isn't necessary for all the snippets 
I have around neither apparently to pass the testsuite (but this is 
rather straightforward, right?). As I said, I tried many different 
things, some also fiddled with TREE_PURPOSE, in pt.c too, but in what I 
sent I only added the check out of a reflex. Anyway, the below is in 
libstdc++, so far so good.

Thanks,
Paolo.

//////////////////

[-- Attachment #2: patch_71169_71832_b --]
[-- Type: text/plain, Size: 10679 bytes --]

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 258264)
+++ cp/parser.c	(working copy)
@@ -565,6 +565,8 @@ cp_debug_parser (FILE *file, cp_parser *parser)
 			      "local class", parser->in_function_body);
   cp_debug_print_flag (file, "Auto correct a colon to a scope operator",
 			      parser->colon_corrects_to_scope_p);
+  cp_debug_print_flag (file, "Error in template parameter list",
+			      parser->error_in_template_parameter_list_p);
   cp_debug_print_flag (file, "Colon doesn't start a class definition",
 			      parser->colon_doesnt_start_class_def_p);
   if (parser->type_definition_forbidden_message)
@@ -3910,6 +3912,9 @@ cp_parser_new (void)
   /* We can correct until told otherwise.  */
   parser->colon_corrects_to_scope_p = true;
 
+  /* No error so far.  */
+  parser->error_in_template_parameter_list_p = false;
+
   /* The unparsed function queue is empty.  */
   push_unparsed_function_queues (parser);
 
@@ -10151,6 +10156,8 @@ cp_parser_lambda_expression (cp_parser* parser)
     /* Inside the class, surrounding template-parameter-lists do not apply.  */
     unsigned int saved_num_template_parameter_lists
         = parser->num_template_parameter_lists;
+    bool saved_error_in_template_parameter_list_p
+        = parser->error_in_template_parameter_list_p;
     unsigned char in_statement = parser->in_statement;
     bool in_switch_statement_p = parser->in_switch_statement_p;
     bool fully_implicit_function_template_p
@@ -10161,6 +10168,7 @@ cp_parser_lambda_expression (cp_parser* parser)
         = parser->auto_is_implicit_function_template_parm_p;
 
     parser->num_template_parameter_lists = 0;
+    parser->error_in_template_parameter_list_p = false;
     parser->in_statement = 0;
     parser->in_switch_statement_p = false;
     parser->fully_implicit_function_template_p = false;
@@ -10199,6 +10207,8 @@ cp_parser_lambda_expression (cp_parser* parser)
     type = finish_struct (type, /*attributes=*/NULL_TREE);
 
     parser->num_template_parameter_lists = saved_num_template_parameter_lists;
+    parser->error_in_template_parameter_list_p
+       = saved_error_in_template_parameter_list_p;
     parser->in_statement = in_statement;
     parser->in_switch_statement_p = in_switch_statement_p;
     parser->fully_implicit_function_template_p
@@ -10624,7 +10634,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser
       {
 	fco = finish_member_template_decl (fco);
 	finish_template_decl (template_param_list);
-	--parser->num_template_parameter_lists;
+	if (--parser->num_template_parameter_lists == 0)
+	  parser->error_in_template_parameter_list_p = false;
       }
     else if (parser->fully_implicit_function_template_p)
       fco = finish_fully_implicit_template (parser, fco);
@@ -15065,6 +15076,10 @@ cp_parser_template_parameter_list (cp_parser* pars
          parameter_list = chainon (parameter_list, err_parm);
        }
 
+      if (parameter == error_mark_node
+	  || error_operand_p (TREE_VALUE (parameter)))
+	parser->error_in_template_parameter_list_p = true;
+
       /* If the next token is not a `,', we're done.  */
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
 	break;
@@ -16673,7 +16688,8 @@ cp_parser_explicit_specialization (cp_parser* pars
   if (need_lang_pop)
     pop_lang_context ();
   /* We're done with this parameter list.  */
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 }
 
 /* Parse a type-specifier.
@@ -22438,6 +22454,7 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   tree attributes = NULL_TREE;
   bool nested_name_specifier_p;
   unsigned saved_num_template_parameter_lists;
+  bool saved_error_in_template_parameter_list_p;
   bool saved_in_function_body;
   unsigned char in_statement;
   bool in_switch_statement_p;
@@ -22480,6 +22497,9 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   saved_num_template_parameter_lists
     = parser->num_template_parameter_lists;
   parser->num_template_parameter_lists = 0;
+  saved_error_in_template_parameter_list_p
+    = parser->error_in_template_parameter_list_p;
+  parser->error_in_template_parameter_list_p = false;
   /* We are not in a function body.  */
   saved_in_function_body = parser->in_function_body;
   parser->in_function_body = false;
@@ -22658,8 +22678,13 @@ cp_parser_class_specifier_1 (cp_parser* parser)
        struct A::B { void f() { } };
 
      there is no need to delay the parsing of `A::B::f'.  */
-  if (--parser->num_classes_being_defined == 0)
+  if (saved_error_in_template_parameter_list_p)
     {
+      /* Do nothing, we skip the bodies in order to improve error
+	 recovery (c++/71169, c++/71832, etc).  */;
+    }
+  else if (--parser->num_classes_being_defined == 0)
+    {
       tree decl;
       tree class_type = NULL_TREE;
       tree pushed_scope = NULL_TREE;
@@ -22752,6 +22777,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
   parser->in_function_body = saved_in_function_body;
   parser->num_template_parameter_lists
     = saved_num_template_parameter_lists;
+  parser->error_in_template_parameter_list_p
+    = saved_error_in_template_parameter_list_p;
   parser->in_unbraced_linkage_specification_p
     = saved_in_unbraced_linkage_specification_p;
 
@@ -23253,7 +23280,8 @@ cp_parser_class_head (cp_parser* parser,
   if (invalid_explicit_specialization_p)
     {
       end_specialization ();
-      --parser->num_template_parameter_lists;
+      if (--parser->num_template_parameter_lists == 0)
+	parser->error_in_template_parameter_list_p = false;
     }
 
   if (type)
@@ -26616,6 +26644,7 @@ cp_parser_constructor_declarator_p (cp_parser *par
 	  tree type;
 	  tree pushed_scope = NULL_TREE;
 	  unsigned saved_num_template_parameter_lists;
+	  bool saved_error_in_template_parameter_list_p;
 
 	  /* Names appearing in the type-specifier should be looked up
 	     in the scope of the class.  */
@@ -26642,6 +26671,9 @@ cp_parser_constructor_declarator_p (cp_parser *par
 	  saved_num_template_parameter_lists
 	    = parser->num_template_parameter_lists;
 	  parser->num_template_parameter_lists = 0;
+	  saved_error_in_template_parameter_list_p
+	    = parser->error_in_template_parameter_list_p;
+	  parser->error_in_template_parameter_list_p = false;
 
 	  /* Look for the type-specifier.  */
 	  cp_parser_type_specifier (parser,
@@ -26653,6 +26685,8 @@ cp_parser_constructor_declarator_p (cp_parser *par
 
 	  parser->num_template_parameter_lists
 	    = saved_num_template_parameter_lists;
+	  parser->error_in_template_parameter_list_p
+	    = saved_error_in_template_parameter_list_p;
 
 	  /* Leave the scope of the class.  */
 	  if (pushed_scope)
@@ -26751,6 +26785,7 @@ cp_parser_function_definition_after_declarator (cp
   bool saved_in_unbraced_linkage_specification_p;
   bool saved_in_function_body;
   unsigned saved_num_template_parameter_lists;
+  bool saved_error_in_template_parameter_list_p;
   cp_token *token;
   bool fully_implicit_function_template_p
     = parser->fully_implicit_function_template_p;
@@ -26799,6 +26834,9 @@ cp_parser_function_definition_after_declarator (cp
   saved_num_template_parameter_lists
     = parser->num_template_parameter_lists;
   parser->num_template_parameter_lists = 0;
+  saved_error_in_template_parameter_list_p
+    = parser->error_in_template_parameter_list_p;
+  parser->error_in_template_parameter_list_p = false;
 
   /* If the next token is `try', `__transaction_atomic', or
      `__transaction_relaxed`, then we are looking at either function-try-block
@@ -26824,6 +26862,8 @@ cp_parser_function_definition_after_declarator (cp
     = saved_in_unbraced_linkage_specification_p;
   parser->num_template_parameter_lists
     = saved_num_template_parameter_lists;
+  parser->error_in_template_parameter_list_p
+    = saved_error_in_template_parameter_list_p;
   parser->in_function_body = saved_in_function_body;
 
   parser->fully_implicit_function_template_p
@@ -26893,7 +26933,8 @@ cp_parser_template_declaration_after_parameters (c
 			   /*complain=*/true);
     }
   /* We are done with the current parameter list.  */
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 
   pop_deferring_access_checks ();
 
@@ -39249,7 +39290,8 @@ finish_fully_implicit_template (cp_parser *parser,
   end_template_decl ();
 
   parser->fully_implicit_function_template_p = false;
-  --parser->num_template_parameter_lists;
+  if (--parser->num_template_parameter_lists == 0)
+    parser->error_in_template_parameter_list_p = false;
 
   return member_decl_opt;
 }
Index: cp/parser.h
===================================================================
--- cp/parser.h	(revision 258264)
+++ cp/parser.h	(working copy)
@@ -346,6 +346,9 @@ struct GTY(()) cp_parser {
      is terminated by colon.  */
   bool colon_doesnt_start_class_def_p;
 
+  /* TRUE if there was an error in a template parameter list.  */
+  bool error_in_template_parameter_list_p;
+
   /* If non-NULL, then we are parsing a construct where new type
      definitions are not permitted.  The string stored here will be
      issued as an error message if a type is defined.  */
Index: testsuite/g++.dg/cpp0x/pr71169-2.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169-2.C	(working copy)
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
+
+template<typename>
+struct B
+{
+  int f() { return 0; }
+};
+
+int main()
+{
+  B<int> b;
+  return b.f();
+}
Index: testsuite/g++.dg/cpp0x/pr71169.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
Index: testsuite/g++.dg/cpp0x/pr71832.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71832.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71832.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template < typename decltype (0) > struct A  // { dg-error "expected|two or more" }
+{ 
+  void foo () { baz (); }
+  template < typename ... S > void baz () {}
+};

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Jason Merrill]

On Tue, Mar 6, 2018 at 5:00 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 06/03/2018 21:33, Jason Merrill wrote:
>>
>> Interesting, that seems like a promising idea.  I'm not sure we need
>> to do this based on an error in a default template arg, though; can we
>> drop
>>
>> +  || error_operand_p (TREE_PURPOSE (parameter)))
>>
>> ?
>
> Good point, yes, I believe we can, isn't necessary for all the snippets I
> have around neither apparently to pass the testsuite (but this is rather
> straightforward, right?). As I said, I tried many different things, some
> also fiddled with TREE_PURPOSE, in pt.c too, but in what I sent I only added
> the check out of a reflex. Anyway, the below is in libstdc++, so far so
> good.

What if, instead of adding another flag to cp_parser, we look for
errors in the template parms for a particular member before we do any
late parsing for it?

Jason

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Paolo Carlini]

Hi,

On 07/03/2018 16:38, Jason Merrill wrote:
> On Tue, Mar 6, 2018 at 5:00 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> On 06/03/2018 21:33, Jason Merrill wrote:
>>> Interesting, that seems like a promising idea.  I'm not sure we need
>>> to do this based on an error in a default template arg, though; can we
>>> drop
>>>
>>> +  || error_operand_p (TREE_PURPOSE (parameter)))
>>>
>>> ?
>> Good point, yes, I believe we can, isn't necessary for all the snippets I
>> have around neither apparently to pass the testsuite (but this is rather
>> straightforward, right?). As I said, I tried many different things, some
>> also fiddled with TREE_PURPOSE, in pt.c too, but in what I sent I only added
>> the check out of a reflex. Anyway, the below is in libstdc++, so far so
>> good.
> What if, instead of adding another flag to cp_parser, we look for
> errors in the template parms for a particular member before we do any
> late parsing for it?
Thus do the check before:

     FOR_EACH_VEC_SAFE_ELT (unparsed_funs_with_definitions, ix, decl)
       cp_parser_late_parsing_for_member (parser, decl);

and therefore skip the whole set of unparsed_funs? Because the template 
parameters to be considered is the same for all of them, right? 
(otherwise something is seriously wrong with my very idea!) And what 
about the other entities in the 'else if 
(--parser->num_classes_being_defined == 0)' body? On one hand I'm 
certainly concerned that we may be too heavy handed, on the other hand 
we may risk inconsistencies, with some definitions available during 
error recovery other not, where all of them have broken outer template 
parameters anyway. Well, in general, I must say that - assuming the 
possibly erroneous template parameters are shared by all the entities in 
the 'else if (--parser->num_classes_being_define == 0)' body - it would 
be too bad going through again all the template parameters where with 
just a bool we could save the work that we already did anyway, if see 
what I mean...

Paolo.

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Jason Merrill]

On Wed, Mar 7, 2018 at 10:55 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 07/03/2018 16:38, Jason Merrill wrote:
>>
>> On Tue, Mar 6, 2018 at 5:00 PM, Paolo Carlini <paolo.carlini@oracle.com>
>> wrote:
>>>
>>> On 06/03/2018 21:33, Jason Merrill wrote:
>>>>
>>>> Interesting, that seems like a promising idea.  I'm not sure we need
>>>> to do this based on an error in a default template arg, though; can we
>>>> drop
>>>>
>>>> +  || error_operand_p (TREE_PURPOSE (parameter)))
>>>>
>>>> ?
>>>
>>> Good point, yes, I believe we can, isn't necessary for all the snippets I
>>> have around neither apparently to pass the testsuite (but this is rather
>>> straightforward, right?). As I said, I tried many different things, some
>>> also fiddled with TREE_PURPOSE, in pt.c too, but in what I sent I only
>>> added
>>> the check out of a reflex. Anyway, the below is in libstdc++, so far so
>>> good.
>>
>> What if, instead of adding another flag to cp_parser, we look for
>> errors in the template parms for a particular member before we do any
>> late parsing for it?
>
> Thus do the check before:
>
>     FOR_EACH_VEC_SAFE_ELT (unparsed_funs_with_definitions, ix, decl)
>       cp_parser_late_parsing_for_member (parser, decl);
>
> and therefore skip the whole set of unparsed_funs? Because the template
> parameters to be considered is the same for all of them, right?

No; member templates have more template parameters; the error might be
in a nested class, and not affect functions from an enclosing class.

> (otherwise something is seriously wrong with my very idea!)

Not that seriously; once we've seen an error all bets are off, and
we're free to decide that enclosing class functions aren't worth
parsing in that case, either.

> And what about the other
> entities in the 'else if (--parser->num_classes_being_defined == 0)' body?

They also have associated decls where we can look at template parameters.

> On one hand I'm certainly concerned that we may be too heavy handed, on the
> other hand we may risk inconsistencies, with some definitions available
> during error recovery other not, where all of them have broken outer
> template parameters anyway. Well, in general, I must say that - assuming the
> possibly erroneous template parameters are shared by all the entities in the
> 'else if (--parser->num_classes_being_define == 0)' body - it would be too
> bad going through again all the template parameters where with just a bool
> we could save the work that we already did anyway, if see what I mean...

My thought was that this patch adds a lot of managing of the flag in
different places in the parser, whereas looking for error_mark_node in
the template parms here would be just in one place.  But if you prefer
the current approach, that's fine, it's straightforward enough.

Jason

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Paolo Carlini]

Hi,

[snip the various clarifications]

Il 7 Marzo 2018 17:57:07 CET, Jason Merrill <jason@redhat.com> ha scritto:
>My thought was that this patch adds a lot of managing of the flag in
>different places in the parser, whereas looking for error_mark_node in
>the template parms here would be just in one place.  But if you prefer
>the current approach, that's fine, it's straightforward enough.

Thanks a lot for the various clarifications above, where essentially turns out that some details of my patch are correct essentially by chance ;) Seriously, I'm thinking the following: since 8 is getting real close, what if, for 8, for the known mild regressions, we go ahead with my super safe Plan B which I mentioned at beginning of the thread, then as soon as trunk branches we immediately apply my patch and we give it a serious spin, say we rebuild distros with it, and see what happens?

Thanks,
Paolo

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

On Wed, Mar 7, 2018 at 12:18 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> [snip the various clarifications]
>
> Il 7 Marzo 2018 17:57:07 CET, Jason Merrill <jason@redhat.com> ha scritto:
>>My thought was that this patch adds a lot of managing of the flag in
>>different places in the parser, whereas looking for error_mark_node in
>>the template parms here would be just in one place.  But if you prefer
>>the current approach, that's fine, it's straightforward enough.
>
> Thanks a lot for the various clarifications above, where essentially turns out that some details of my patch are correct essentially by chance ;) Seriously, I'm thinking the following: since 8 is getting real close, what if, for 8, for the known mild regressions, we go ahead with my super safe Plan B which I mentioned at beginning of the thread, then as soon as trunk branches we immediately apply my patch and we give it a serious spin, say we rebuild distros with it, and see what happens?

This is what I was suggesting, what do you think?

[-- Attachment #2: 71832.diff --]
[-- Type: text/x-patch, Size: 3045 bytes --]

commit 706f372b52e65694161b9dff0272481d23fa898a
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Mar 7 14:45:19 2018 -0500

            PR c++/71832 - ICE with ill-formed template parameter.
    
            * pt.c (any_erroneous_template_args_p): New.
            * parser.c (cp_parser_class_specifier_1): Use it.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 268be0fd543..8f3ec86e8ce 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6558,6 +6558,7 @@ extern int processing_template_parmlist;
 extern bool dependent_type_p			(tree);
 extern bool dependent_scope_p			(tree);
 extern bool any_dependent_template_arguments_p  (const_tree);
+extern bool any_erroneous_template_args_p	(const_tree);
 extern bool dependent_template_p		(tree);
 extern bool dependent_template_id_p		(tree, tree);
 extern bool type_dependent_expression_p		(tree);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index a19bbe1e1d0..f7a8be50b79 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -22682,6 +22682,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       FOR_EACH_VEC_SAFE_ELT (unparsed_funs_with_default_args, ix, e)
 	{
 	  decl = e->decl;
+	  if (any_erroneous_template_args_p (decl))
+	    continue;
 	  /* If there are default arguments that have not yet been processed,
 	     take care of them now.  */
 	  if (class_type != e->class_type)
@@ -22704,6 +22706,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       save_ccr = current_class_ref;
       FOR_EACH_VEC_SAFE_ELT (unparsed_nsdmis, ix, decl)
 	{
+	  if (any_erroneous_template_args_p (decl))
+	    continue;
 	  if (class_type != DECL_CONTEXT (decl))
 	    {
 	      if (pushed_scope)
@@ -27642,6 +27646,9 @@ cp_parser_late_parsing_for_member (cp_parser* parser, tree member_function)
   if (DECL_FUNCTION_TEMPLATE_P (member_function))
     member_function = DECL_TEMPLATE_RESULT (member_function);
 
+  if (any_erroneous_template_args_p (member_function))
+    return;
+
   /* There should not be any class definitions in progress at this
      point; the bodies of members are only parsed outside of all class
      definitions.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 89024c10fe2..1ce04aaabc7 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -25048,6 +25048,38 @@ any_dependent_template_arguments_p (const_tree args)
   return false;
 }
 
+/* Returns true if ARGS contains any errors.  */
+
+bool
+any_erroneous_template_args_p (const_tree args)
+{
+  int i;
+  int j;
+
+  if (args && TREE_CODE (args) != TREE_VEC)
+    {
+      if (tree ti = get_template_info (args))
+	args = TI_ARGS (ti);
+      else
+	args = NULL_TREE;
+    }
+
+  if (!args)
+    return false;
+  if (args == error_mark_node)
+    return true;
+
+  for (i = 0; i < TMPL_ARGS_DEPTH (args); ++i)
+    {
+      const_tree level = TMPL_ARGS_LEVEL (args, i + 1);
+      for (j = 0; j < TREE_VEC_LENGTH (level); ++j)
+	if (error_operand_p (TREE_VEC_ELT (level, j)))
+	  return true;
+    }
+
+  return false;
+}
+
 /* Returns TRUE if the template TMPL is type-dependent.  */
 
 bool

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Paolo Carlini]

Hi,

On 07/03/2018 21:23, Jason Merrill wrote:
> On Wed, Mar 7, 2018 at 12:18 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Hi,
>>
>> [snip the various clarifications]
>>
>> Il 7 Marzo 2018 17:57:07 CET, Jason Merrill <jason@redhat.com> ha scritto:
>>> My thought was that this patch adds a lot of managing of the flag in
>>> different places in the parser, whereas looking for error_mark_node in
>>> the template parms here would be just in one place.  But if you prefer
>>> the current approach, that's fine, it's straightforward enough.
>> Thanks a lot for the various clarifications above, where essentially turns out that some details of my patch are correct essentially by chance ;) Seriously, I'm thinking the following: since 8 is getting real close, what if, for 8, for the known mild regressions, we go ahead with my super safe Plan B which I mentioned at beginning of the thread, then as soon as trunk branches we immediately apply my patch and we give it a serious spin, say we rebuild distros with it, and see what happens?
> This is what I was suggesting, what do you think?
Eh, eh, certainly I don't have anything to say from the correctness 
point of view. As I already tried to explain, what I find annoying in 
this kind of approach, no matter how well is implemented, is that at 
parsing time we have to go anyway over all the parameters of all the 
template lists and then we know once and for all, for the corresponding 
class, whether there are erroneous parameters or not. In this kind of 
approach we go again through all the parameters, and, for example, 
multiple times when there are nested classes for example, I'm pretty 
sure in some (other) cases too (I should think more about that to be 
more specific). If you ask my opinion, at the moment I still believe 
that the best solution would be doing something at parsing time, save 
the information, in a more elegant way, maybe adding a special 
"erroneous TREE_VEC" flag to the TREE_VECs. I don't know exactly. Even 
better a unique flag for all the template parameter lists of each class. 
That said, if you don't foresee immediate performance-related issues, 
it's of course your call ;) If/when I will have a more concrete 
alternate proposal I will speak up ;) Nice anyway that we agree about 
the basic idea :-)

Paolo.

Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

Hi again,

On 07/03/2018 21:49, Paolo Carlini wrote:
> ... If you ask my opinion, at the moment I still believe that the best 
> solution would be doing something at parsing time, save the 
> information, in a more elegant way, maybe adding a special "erroneous 
> TREE_VEC" flag to the TREE_VECs. I don't know exactly. Even better a 
> unique flag for all the template parameter lists of each class.
Just to explain more *concretely* what I had in mind, I quickly hacked 
the below, which would be a *minimal* implementation of my idea. It 
should be also possible to consolidate the flags of the various 
TMPL_ARGS_LEVELs to the flag of the TREE_VEC returned by TI_ARGS 
(get_template_info). That would be fantastic: ideally I would like to 
see an O(0) any_erroneous_template_args_p. Maybe we can do that when we 
use SET_TMPL_ARGS_LEVEL? Probably it would take you 5 mins to implement 
all of that. Or, I was thinking, something completely different, instead 
of fiddling with TREE_VECs, have something in DECL_TEMPLATE_INFO? (see 
again my remark about the 5 mins ;)

Paolo.

//////////////////

[-- Attachment #2: 71832_p.diff --]
[-- Type: text/x-patch, Size: 3370 bytes --]

Index: cp-tree.h
===================================================================
--- cp-tree.h	(revision 258355)
+++ cp-tree.h	(working copy)
@@ -592,6 +592,8 @@ struct GTY(()) ptrmem_cst {
 };
 typedef struct ptrmem_cst * ptrmem_cst_t;
 
+#define ERRONEOUS_TREE_VEC(NODE) ((NODE)->base.u.bits.lang_flag_0)
+
 #define CLEANUP_P(NODE)		TREE_LANG_FLAG_0 (TRY_BLOCK_CHECK (NODE))
 
 #define BIND_EXPR_TRY_BLOCK(NODE) \
@@ -6558,6 +6560,7 @@ extern int processing_template_parmlist;
 extern bool dependent_type_p			(tree);
 extern bool dependent_scope_p			(tree);
 extern bool any_dependent_template_arguments_p  (const_tree);
+extern bool any_erroneous_template_args_p	(const_tree);
 extern bool dependent_template_p		(tree);
 extern bool dependent_template_id_p		(tree, tree);
 extern bool type_dependent_expression_p		(tree);
Index: parser.c
===================================================================
--- parser.c	(revision 258355)
+++ parser.c	(working copy)
@@ -22682,6 +22682,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       FOR_EACH_VEC_SAFE_ELT (unparsed_funs_with_default_args, ix, e)
 	{
 	  decl = e->decl;
+	  if (any_erroneous_template_args_p (decl))
+	    continue;
 	  /* If there are default arguments that have not yet been processed,
 	     take care of them now.  */
 	  if (class_type != e->class_type)
@@ -22704,6 +22706,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       save_ccr = current_class_ref;
       FOR_EACH_VEC_SAFE_ELT (unparsed_nsdmis, ix, decl)
 	{
+	  if (any_erroneous_template_args_p (decl))
+	    continue;
 	  if (class_type != DECL_CONTEXT (decl))
 	    {
 	      if (pushed_scope)
@@ -27642,6 +27646,9 @@ cp_parser_late_parsing_for_member (cp_parser* pars
   if (DECL_FUNCTION_TEMPLATE_P (member_function))
     member_function = DECL_TEMPLATE_RESULT (member_function);
 
+  if (any_erroneous_template_args_p (member_function))
+    return;
+
   /* There should not be any class definitions in progress at this
      point; the bodies of members are only parsed outside of all class
      definitions.  */
Index: pt.c
===================================================================
--- pt.c	(revision 258355)
+++ pt.c	(working copy)
@@ -4459,6 +4459,8 @@ end_template_parm_list (tree parms)
     {
       next = TREE_CHAIN (parm);
       TREE_VEC_ELT (saved_parmlist, nparms) = parm;
+      if (error_operand_p (parm))
+	ERRONEOUS_TREE_VEC (saved_parmlist) = true;
       TREE_CHAIN (parm) = NULL_TREE;
     }
 
@@ -25048,6 +25050,42 @@ any_dependent_template_arguments_p (const_tree arg
   return false;
 }
 
+/* Returns true if ARGS contains any errors.  */
+
+bool
+any_erroneous_template_args_p (const_tree args)
+{
+  int i;
+  int j;
+
+  if (args && TREE_CODE (args) != TREE_VEC)
+    {
+      if (tree ti = get_template_info (args))
+	args = TI_ARGS (ti);
+      else
+	args = NULL_TREE;
+    }
+
+  if (!args)
+    return false;
+  if (args == error_mark_node)
+    return true;
+
+  for (i = 0; i < TMPL_ARGS_DEPTH (args); ++i)
+    {
+      const_tree level = TMPL_ARGS_LEVEL (args, i + 1);
+      if (ERRONEOUS_TREE_VEC (level))
+	return true;
+      /*
+      for (j = 0; j < TREE_VEC_LENGTH (level); ++j)
+	if (error_operand_p (TREE_VEC_ELT (level, j)))
+	  return true;
+      */
+    }
+
+  return false;
+}
+
 /* Returns TRUE if the template TMPL is type-dependent.  */
 
 bool