Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
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.
//
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
Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
Hi, On 07/03/2018 21:23, Jason Merrill wrote: On Wed, Mar 7, 2018 at 12:18 PM, Paolo Carlini wrote: Hi, [snip the various clarifications] Il 7 Marzo 2018 17:57:07 CET, Jason Merrill 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
On Wed, Mar 7, 2018 at 12:18 PM, Paolo Carlini wrote:
> Hi,
>
> [snip the various clarifications]
>
> Il 7 Marzo 2018 17:57:07 CET, Jason Merrill 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?
commit 706f372b52e65694161b9dff0272481d23fa898a
Author: Jason Merrill
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
Hi, [snip the various clarifications] Il 7 Marzo 2018 17:57:07 CET, Jason Merrill 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
On Wed, Mar 7, 2018 at 10:55 AM, Paolo Carlini wrote: > Hi, > > On 07/03/2018 16:38, Jason Merrill wrote: >> >> On Tue, Mar 6, 2018 at 5:00 PM, Paolo Carlini >> 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
Hi, On 07/03/2018 16:38, Jason Merrill wrote: On Tue, Mar 6, 2018 at 5:00 PM, Paolo Carlini 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
On Tue, Mar 6, 2018 at 5:00 PM, Paolo Carlini 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
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.
//
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_parame
Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
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
Hi again,
On 06/03/2018 19:58, Paolo Carlini wrote:
.. g, 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.
/
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_c
Re: [C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
.. g, 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.
[C++ Patch/RFC] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more
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 =
