| Message ID | eb8db7d6-757d-cbd0-87b0-2410310ec930@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Improve cp_parser_class_head error recovery | expand |
On 10/18/19 11:35 AM, Paolo Carlini wrote: > Hi, > > a few days ago I noticed that for, say, g++.dg/parse/qualified2.C we > were issuing two additional misleading errors after the first one, > mentioning in particular a certain "unnamed class" (I'm reproducing only > the error messages proper): > > namespace Glib { > template <typename> class Value {}; > template <> class Glib::Value<int> {}; // { dg-error "" } > } > > qualified2.C:3:29: error: extra qualification not allowed [\-fpermissive\] > qualified2.C:3:46: error: explicit specialization of non-template > ‘Glib::<unnamed class>’ > qualified2.C:3:47: error: abstract declarator ‘Glib::<unnamed class>’ > used as declaration > > Let's see if I can explain clearly enough what I think it's going on. > > In cp_parser_class_head, upon the permerror about the extra > qualification, we try to do error recovery, which is particularly > tricky, because we are dealing with a permerror thus we have to make > sure that in case of -fpermissive everything remains internally > consistent anyway. In this context, clearing 'nested_name_specifier' and > 'num_templates' doesn't seem a good idea because it does *not* give us > an internal state similar to the one normally obtained when the nested > name specifier is not there, the reason being that, earlier in the > function, when a nested name specifier really isn't there we try > cp_parser_template_id or in case cp_parser_identifier, which set the > locale 'id' and possibly 'template_id' and 'num_templates', whereas > during error recovery we remain so to speak completely empty handed. > Thus, what about not clearing anything? That seems to work at least for > the two testcases below and doesn't cause regressions. > > Thanks, Paolo. > > ///////////////////////////// > > > OK. Jason
Index: cp/parser.c =================================================================== --- cp/parser.c (revision 277149) +++ cp/parser.c (working copy) @@ -24178,12 +24178,8 @@ cp_parser_class_head (cp_parser* parser, ... [or] the definition or explicit instantiation of a class member of a namespace outside of its namespace. */ if (scope == nested_name_specifier) - { - permerror (nested_name_specifier_token_start->location, - "extra qualification not allowed"); - nested_name_specifier = NULL_TREE; - num_templates = 0; - } + permerror (nested_name_specifier_token_start->location, + "extra qualification not allowed"); } /* An explicit-specialization must be preceded by "template <>". If it is not, try to recover gracefully. */ Index: testsuite/g++.dg/parse/qualified2.C =================================================================== --- testsuite/g++.dg/parse/qualified2.C (revision 277144) +++ testsuite/g++.dg/parse/qualified2.C (working copy) @@ -1,4 +1,4 @@ namespace Glib { template <typename> class Value {}; - template <> class Glib::Value<int> {}; // { dg-error "" } + template <> class Glib::Value<int> {}; // { dg-error "29:extra qualification" } } Index: testsuite/g++.old-deja/g++.other/decl5.C =================================================================== --- testsuite/g++.old-deja/g++.other/decl5.C (revision 277144) +++ testsuite/g++.old-deja/g++.other/decl5.C (working copy) @@ -12,7 +12,6 @@ struct A { int A::m; // { dg-error "extra qualification" } struct e; struct A::e {int i;}; // { dg-error "extra qualification" "qual" } - // { dg-error "anonymous struct" "anon" { target *-*-* } .-1 } struct A::expand { // { dg-error "qualified name" } int m; };
Hi, a few days ago I noticed that for, say, g++.dg/parse/qualified2.C we were issuing two additional misleading errors after the first one, mentioning in particular a certain "unnamed class" (I'm reproducing only the error messages proper): namespace Glib { template <typename> class Value {}; template <> class Glib::Value<int> {}; // { dg-error "" } } qualified2.C:3:29: error: extra qualification not allowed [\-fpermissive\] qualified2.C:3:46: error: explicit specialization of non-template ‘Glib::<unnamed class>’ qualified2.C:3:47: error: abstract declarator ‘Glib::<unnamed class>’ used as declaration Let's see if I can explain clearly enough what I think it's going on. In cp_parser_class_head, upon the permerror about the extra qualification, we try to do error recovery, which is particularly tricky, because we are dealing with a permerror thus we have to make sure that in case of -fpermissive everything remains internally consistent anyway. In this context, clearing 'nested_name_specifier' and 'num_templates' doesn't seem a good idea because it does *not* give us an internal state similar to the one normally obtained when the nested name specifier is not there, the reason being that, earlier in the function, when a nested name specifier really isn't there we try cp_parser_template_id or in case cp_parser_identifier, which set the locale 'id' and possibly 'template_id' and 'num_templates', whereas during error recovery we remain so to speak completely empty handed. Thus, what about not clearing anything? That seems to work at least for the two testcases below and doesn't cause regressions. Thanks, Paolo. ///////////////////////////// /cp 2019-10-18 Paolo Carlini <paolo.carlini@oracle.com> * parser.c (cp_parser_class_head): Improve error recovery upon extra qualification error. /testsuite 2019-10-18 Paolo Carlini <paolo.carlini@oracle.com> * g++.dg/parse/qualified2.C: Tighten dg-error directive. * g++.old-deja/g++.other/decl5.C: Don't expect redundant error.