[PATCH] D26503: [Parser][ObjC] Improve diagnostics and recovery when C++ keywords are used as identifiers in Objective-C++

[Manman Ren via cfe-commits]

manmanren added a comment.

Cheers,
Manman



================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:442
+/// Objective-C++ parser diagnostics
+def err_expected_objcxx_keyword : Error<
+  "expected %0; %1 is a keyword in Objective-C++">;
----------------
This name is a little misleading. It sounds like we are expecting a keyword :]


================
Comment at: include/clang/Parse/Parser.h:798
+  ///
+  /// Try to recover from the error by checking if the next token if a C++
+  /// keyword when parsing Objective-C++. Return false if the recovery was
----------------
--> if the next token is a C++ (if to is)


================
Comment at: include/clang/Parse/Parser.h:801
+  /// successful.
+  bool diagnoseExpectedObjCIdentifier();
+
----------------
Same here. The function name is kind of misleading.


================
Comment at: lib/Parse/ParseDecl.cpp:5405
+      // Objective-C++: Detect C++ keywords and try to prevent further errors by
+      // treating these keyword as valid member names.
+      if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus &&
----------------
Does C++ have the same issue? Or is this only needed for Objective-C++?


================
Comment at: lib/Parse/ParseObjc.cpp:153
     MaybeSkipAttributes(tok::objc_class);
-    if (Tok.isNot(tok::identifier)) {
-      Diag(Tok, diag::err_expected) << tok::identifier;
+    if (Tok.isNot(tok::identifier) && diagnoseExpectedObjCIdentifier()) {
       SkipUntil(tok::semi);
----------------
Are we doing this for all occurrences of "isNot(token::identifier)"?
Is it better to wrap this "&&" in the function and rename the function to something like diagnoseNonIdentifier?


Repository:
  rL LLVM

https://reviews.llvm.org/D26503