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.
///////////////////////////
/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.
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 () {}
+};
