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

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:6872
 
+      // Recovery if a keyword is used as an identifier.
+      if (Tok.getIdentifierInfo() &&
----------------
As I understand it, we're trying to catch keywords in place of the identifier in a declarator, and in particular the common case where:
 - the identifier comes at the end of the declarator
 - if the identifier is dropped, the declarator is valid but anonymous (no identifier)
 - so declarator parsing succeeds, and then we have a trailing keyword, which is never valid in a param list (need , or ... or `)` )

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).
Our recovery is just treating this as an anonymous parameter, and fortunately we've already almost done that.

So all of this would be good to capture in the comment :-)

We should restrict to these cases as much as possible: the type should be present, and the identifier should be null[1]. 

I don't know how confident we can be all the time that our interpretation (the keyword was meant to be a variable) is right, and the diagnostic might be confusing in that case.
e.g. `void foo(int const)` will trigger this diagnostic, and the user may intend anonymous `void foo(const int)`. `using keyword const as an identifier is not permitted` may be less clear than e.g. the more specific `invalid parameter name: 'const' is a keyword`.

(Note that because of the [1] case, as the patch stands `void foo(int x const)` will trigger `using keyword const as an identifier...`, which is bad!)


================
Comment at: clang/test/Parser/cxx-keyword-identifiers.cpp:12
+  // FIXME: we shoud improve the dianostics for the following cases.
+  int case; // expected-error {{expected unqualified-id}}
+  struct X {
----------------
this seems nice but harder, I think the grammar is more permissive here. I guess we'd be able to whitelist the keywords that can legitimately follow?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77633