From 625acd8f6847a156b236b8f11b4b02b11cac3766 Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Wed, 8 Apr 2020 15:08:48 +0200 Subject: [PATCH] [Parser] Improve diagnostic and error recovery when C++ keywords are used as identifiers. Summary: Previously, clang emitted a less-usefull diagnostic and didnt recover well when the keywords is used as identifier in function paramter. ``` void foo(int case, int x); // previously we drop all parameters after `int case`. ``` Reviewers: sammccall Reviewed By: sammccall Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D77633 --- clang/include/clang/Basic/DiagnosticParseKinds.td | 2 ++ clang/lib/Parse/ParseDecl.cpp | 25 +++++++++++++++++++++ clang/test/Parser/cxx-keyword-identifiers.cpp | 27 +++++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 clang/test/Parser/cxx-keyword-identifiers.cpp diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index a1311d7..29497f9 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -591,6 +591,8 @@ def warn_cxx17_compat_for_range_init_stmt : Warning< def warn_empty_init_statement : Warning< "empty initialization statement of '%select{if|switch|range-based for}0' " "has no effect">, InGroup, DefaultIgnore; +def err_keyword_as_parameter : Error < + "invalid parameter name: '%0' is a keyword">; // C++ derived classes def err_dup_virtual : Error<"duplicate 'virtual' in base specifier">; diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index e60d1f6..8bd7571 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -6815,6 +6815,31 @@ void Parser::ParseParameterDeclarationClause( Actions.containsUnexpandedParameterPacks(ParmDeclarator)) DiagnoseMisplacedEllipsisInDeclarator(ConsumeToken(), ParmDeclarator); + // Now we are at the point where declarator parsing is finished. + // + // Try to catch keywords in place of the identifier in a declarator, and + // in particular the common case where: + // 1 identifier comes at the end of the declarator + // 2 if the identifier is dropped, the declarator is valid but anonymous + // (no identifier) + // 3 declarator parsing succeeds, and then we have a trailing keyword, + // which is never valid in a param list (e.g. missing a ',') + // And we can't handle this in ParseDeclarator because in general keywords + // may be allowed to follow the declarator. (And in some cases there'd be + // better recovery like inserting punctuation). ParseDeclarator is just + // treating this as an anonymous parameter, and fortunately at this point + // we've already almost done that. + // + // We care about case 1) where the declarator type should be known, and + // the identifier should be null. + if (!ParmDeclarator.isInvalidType() && !ParmDeclarator.hasName()) { + if (Tok.getIdentifierInfo() && + Tok.getIdentifierInfo()->isKeyword(getLangOpts())) { + Diag(Tok, diag::err_keyword_as_parameter) << PP.getSpelling(Tok); + // Consume the keyword. + ConsumeToken(); + } + } // Inform the actions module about the parameter declarator, so it gets // added to the current scope. Decl *Param = Actions.ActOnParamDeclarator(getCurScope(), ParmDeclarator); diff --git a/clang/test/Parser/cxx-keyword-identifiers.cpp b/clang/test/Parser/cxx-keyword-identifiers.cpp new file mode 100644 index 0000000..4a60ce9 --- /dev/null +++ b/clang/test/Parser/cxx-keyword-identifiers.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s + + +int foo1(int case, int throw, int y) { // expected-error {{invalid parameter name: 'case' is a keyword}} \ + expected-error {{invalid}} + // Trailing parameters should be recovered. + y = 1; +} + +int foo2(int case = 1); // expected-error {{invalid parameter}} +int foo3(int const); // ok: without parameter name. +// ok: override has special meaning when used after method functions. it can be +// used as name. +int foo4(int override); +int foo5(int x const); // expected-error {{expected ')'}} expected-note {{to match this '('}} +// FIXME: bad recovery on the case below, "invalid parameter" is desired, the +// followon diagnostics should be suppressed. +int foo6(int case __attribute((weak))); // expected-error {{invalid parameter}} \ + // expected-error {{expected ')'}} expected-note {{to match this '('}} + +void test() { + // FIXME: we shoud improve the dianostics for the following cases. + int case; // expected-error {{expected unqualified-id}} + struct X { + int case; // expected-error {{expected member name or ';'}} + }; +} -- 2.7.4