[PATCH] D77633: [Parser] Improve diagnostic and error recovery when C++ keywords are used as identifiers.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 255988.
hokein marked an inline comment as done.
hokein added a comment.

add FIXME.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77633/new/

https://reviews.llvm.org/D77633

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/test/Parser/cxx-keyword-identifiers.cpp


Index: clang/test/Parser/cxx-keyword-identifiers.cpp
===================================================================
--- /dev/null
+++ 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 ';'}}
+  };
+}
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6815,6 +6815,31 @@
           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);
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -591,6 +591,8 @@
 def warn_empty_init_statement : Warning<
   "empty initialization statement of '%select{if|switch|range-based for}0' "
   "has no effect">, InGroup<EmptyInitStatement>, 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">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77633.255988.patch
Type: text/x-patch
Size: 3933 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200408/bbcd9c0d/attachment-0001.bin>