From cd0b71242738a1901405f421b352e4f6c30ff7c5 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Tue, 17 Mar 2020 21:21:16 +0100 Subject: [PATCH] c++: Fix parsing of invalid enum specifiers [PR90995] The testcase shows some accepts-invalid (the ones without alignas) and ice-on-invalid-code (the ones with alignas) cases. If the enum doesn't have an underlying type and is not a definition, the caller retries to parse it as elaborated type specifier. E.g. for enum struct S s it will then pedwarn that elaborated type specifier shouldn't have the struct/class keywords. The problem is if the enum specifier is not followed by { when it has underlying type. In that case we have already called cp_parser_parse_definitely to end the tentative parsing started at the beginning of cp_parser_enum_specifier. But the cp_parser_error (parser, "expected %<;%> or %<{%>"); doesn't emit any error because the whole function is called from yet another tentative parse and the caller starts parsing the elaborated type specifier where the cp_parser_enum_specifier stopped (i.e. after the underlying type token(s)). The ultimate caller than commits the tentative parsing (and even if it wouldn't, it wouldn't know what kind of error to report). I think after seeing enum {,struct,class} : type not being followed by { or ;, there is no reason not to report it right away, as it can't be valid C++, which is what the patch does. Not sure if we shouldn't also return error_mark_node instead of NULL_TREE, so that the caller doesn't try to parse it as elaborated type specifier (the patch doesn't do that right now). Furthermore, while reading the code, I've noticed that parser->colon_corrects_to_scope_p is saved and set to false at the start of the function, but not restored back in some cases. Don't have a testcase where this would be a problem, but it just seems wrong. Either we can in the two spots replace return NULL_TREE; with { type = NULL_TREE; goto out; } or we could perhaps abuse warning_sentinel or create a special class with dtor to clean the flag up. And lastly, I've fixed some formatting issues in the function while reading it. 2020-03-17 Jakub Jelinek PR c++/90995 * parser.c (cp_parser_enum_specifier): Use temp_override for parser->colon_corrects_to_scope_p, replace goto out with return. If scoped enum or enum with underlying type is not followed by { or ;, call cp_parser_commit_to_tentative_parse before calling cp_parser_error and make sure to return error_mark_node instead of NULL_TREE. Formatting fixes. * g++.dg/cpp0x/enum40.C: New test. --- gcc/cp/ChangeLog | 10 ++++++ gcc/cp/parser.c | 52 ++++++++++++----------------- gcc/testsuite/ChangeLog | 5 +++ gcc/testsuite/g++.dg/cpp0x/enum40.C | 26 +++++++++++++++ 4 files changed, 62 insertions(+), 31 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/enum40.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 0cdbd027a20..1db1e090e51 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,13 @@ +2020-03-17 Jakub Jelinek + + PR c++/90995 + * parser.c (cp_parser_enum_specifier): Use temp_override for + parser->colon_corrects_to_scope_p, replace goto out with return. + If scoped enum or enum with underlying type is not followed by + { or ;, call cp_parser_commit_to_tentative_parse before calling + cp_parser_error and make sure to return error_mark_node instead of + NULL_TREE. Formatting fixes. + 2020-03-17 Ville Voutilainen PR c++/94197 diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 58a1bea33f3..26e02366645 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -19001,9 +19001,7 @@ cp_parser_enum_specifier (cp_parser* parser) bool is_unnamed = false; tree underlying_type = NULL_TREE; cp_token *type_start_token = NULL; - bool saved_colon_corrects_to_scope_p = parser->colon_corrects_to_scope_p; - - parser->colon_corrects_to_scope_p = false; + temp_override cleanup (parser->colon_corrects_to_scope_p, false); /* Parse tentatively so that we can back up if we don't find a enum-specifier. */ @@ -19043,24 +19041,24 @@ cp_parser_enum_specifier (cp_parser* parser) push_deferring_access_checks (dk_no_check); nested_name_specifier - = cp_parser_nested_name_specifier_opt (parser, - /*typename_keyword_p=*/true, - /*check_dependency_p=*/false, - /*type_p=*/false, - /*is_declaration=*/false); + = cp_parser_nested_name_specifier_opt (parser, + /*typename_keyword_p=*/true, + /*check_dependency_p=*/false, + /*type_p=*/false, + /*is_declaration=*/false); if (nested_name_specifier) { tree name; identifier = cp_parser_identifier (parser); - name = cp_parser_lookup_name (parser, identifier, - enum_type, - /*is_template=*/false, - /*is_namespace=*/false, - /*check_dependency=*/true, - /*ambiguous_decls=*/NULL, - input_location); + name = cp_parser_lookup_name (parser, identifier, + enum_type, + /*is_template=*/false, + /*is_namespace=*/false, + /*check_dependency=*/true, + /*ambiguous_decls=*/NULL, + input_location); if (name && name != error_mark_node) { type = TREE_TYPE (name); @@ -19140,23 +19138,21 @@ cp_parser_enum_specifier (cp_parser* parser) { if (cxx_dialect < cxx11 || (!scoped_enum_p && !underlying_type)) { + if (has_underlying_type) + cp_parser_commit_to_tentative_parse (parser); cp_parser_error (parser, "expected %<{%>"); if (has_underlying_type) - { - type = NULL_TREE; - goto out; - } + return error_mark_node; } /* An opaque-enum-specifier must have a ';' here. */ if ((scoped_enum_p || underlying_type) && cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON)) { + if (has_underlying_type) + cp_parser_commit_to_tentative_parse (parser); cp_parser_error (parser, "expected %<;%> or %<{%>"); if (has_underlying_type) - { - type = NULL_TREE; - goto out; - } + return error_mark_node; } } @@ -19172,9 +19168,7 @@ cp_parser_enum_specifier (cp_parser* parser) push_scope (nested_name_specifier); } else if (TREE_CODE (nested_name_specifier) == NAMESPACE_DECL) - { - push_nested_namespace (nested_name_specifier); - } + push_nested_namespace (nested_name_specifier); } /* Issue an error message if type-definitions are forbidden here. */ @@ -19334,12 +19328,8 @@ cp_parser_enum_specifier (cp_parser* parser) pop_scope (nested_name_specifier); } else if (TREE_CODE (nested_name_specifier) == NAMESPACE_DECL) - { - pop_nested_namespace (nested_name_specifier); - } + pop_nested_namespace (nested_name_specifier); } - out: - parser->colon_corrects_to_scope_p = saved_colon_corrects_to_scope_p; return type; } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 484eef58281..45fb82941cc 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-03-17 Jakub Jelinek + + PR c++/90995 + * g++.dg/cpp0x/enum40.C: New test. + 2020-03-17 Richard Sandiford * gcc.target/aarch64/advsimd-intrinsics/bfcvt-nosimd.c: Skip for diff --git a/gcc/testsuite/g++.dg/cpp0x/enum40.C b/gcc/testsuite/g++.dg/cpp0x/enum40.C new file mode 100644 index 00000000000..cfdf2a4a18a --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/enum40.C @@ -0,0 +1,26 @@ +// PR c++/90995 +// { dg-do compile { target c++11 } } + +void +foo () +{ + enum : int a alignas; // { dg-error "expected" } +} + +void +bar () +{ + enum : int a; // { dg-error "expected" } +} + +void +baz () +{ + enum class a : int b alignas; // { dg-error "expected" } +} + +void +qux () +{ + enum class a : int b; // { dg-error "expected" } +} -- 2.30.2