* [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
@ 2018-03-06 18:14 Paolo Carlini
2018-03-06 18:58 ` Paolo Carlini
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Carlini @ 2018-03-06 18:14 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- 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 () {}
+};
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-06 18:14 [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more Paolo Carlini
@ 2018-03-06 18:58 ` Paolo Carlini
2018-03-06 19:44 ` Paolo Carlini
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Carlini @ 2018-03-06 18:58 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
.. 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-06 18:58 ` Paolo Carlini
@ 2018-03-06 19:44 ` Paolo Carlini
2018-03-06 20:33 ` Jason Merrill
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Carlini @ 2018-03-06 19:44 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- 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 () {}
+};
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-06 19:44 ` Paolo Carlini
@ 2018-03-06 20:33 ` Jason Merrill
2018-03-06 22:00 ` Paolo Carlini
0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2018-03-06 20:33 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-06 20:33 ` Jason Merrill
@ 2018-03-06 22:00 ` Paolo Carlini
2018-03-07 15:39 ` Jason Merrill
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Carlini @ 2018-03-06 22:00 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- 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 () {}
+};
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-06 22:00 ` Paolo Carlini
@ 2018-03-07 15:39 ` Jason Merrill
2018-03-07 16:10 ` Paolo Carlini
0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2018-03-07 15:39 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-07 15:39 ` Jason Merrill
@ 2018-03-07 16:10 ` Paolo Carlini
2018-03-07 16:57 ` Jason Merrill
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Carlini @ 2018-03-07 16:10 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-07 16:10 ` Paolo Carlini
@ 2018-03-07 16:57 ` Jason Merrill
2018-03-07 17:18 ` Paolo Carlini
0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2018-03-07 16:57 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-07 16:57 ` Jason Merrill
@ 2018-03-07 17:18 ` Paolo Carlini
2018-03-07 20:24 ` Jason Merrill
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Carlini @ 2018-03-07 17:18 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-07 17:18 ` Paolo Carlini
@ 2018-03-07 20:24 ` Jason Merrill
2018-03-07 20:59 ` Paolo Carlini
0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2018-03-07 20:24 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
[-- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-07 20:24 ` Jason Merrill
@ 2018-03-07 20:59 ` Paolo Carlini
2018-03-08 16:11 ` Paolo Carlini
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Carlini @ 2018-03-07 20:59 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
2018-03-07 20:59 ` Paolo Carlini
@ 2018-03-08 16:11 ` Paolo Carlini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Carlini @ 2018-03-08 16:11 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-03-08 16:11 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-06 18:14 [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more Paolo Carlini
2018-03-06 18:58 ` Paolo Carlini
2018-03-06 19:44 ` Paolo Carlini
2018-03-06 20:33 ` Jason Merrill
2018-03-06 22:00 ` Paolo Carlini
2018-03-07 15:39 ` Jason Merrill
2018-03-07 16:10 ` Paolo Carlini
2018-03-07 16:57 ` Jason Merrill
2018-03-07 17:18 ` Paolo Carlini
2018-03-07 20:24 ` Jason Merrill
2018-03-07 20:59 ` Paolo Carlini
2018-03-08 16:11 ` Paolo Carlini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).