arphaman updated this revision to Diff 78218.
arphaman marked 4 inline comments as done.
arphaman added a comment.

Addressed review comments by renaming the diagnostic and simplifying the name 
and the use of the `expectIdentifier ` method.


Repository:
  rL LLVM

https://reviews.llvm.org/D26503

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/IdentifierTable.h
  include/clang/Parse/Parser.h
  lib/Basic/IdentifierTable.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseObjc.cpp
  lib/Parse/Parser.cpp
  test/Parser/objc-cxx-keyword-identifiers.mm

Index: test/Parser/objc-cxx-keyword-identifiers.mm
===================================================================
--- /dev/null
+++ test/Parser/objc-cxx-keyword-identifiers.mm
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wno-objc-root-class -Wno-incomplete-implementation -triple x86_64-apple-macosx10.10.0 -verify %s
+
+// rdar://20626062
+
+struct S {
+  int throw; // expected-error {{expected member name or ';' after declaration specifiers; 'throw' is a keyword in Objective-C++}}
+};
+
+@interface class // expected-error {{expected identifier; 'class' is a keyword in Objective-C++}}
+@end
+
+@interface Bar: class // expected-error {{expected identifier; 'class' is a keyword in Objective-C++}}
+@end
+
+@protocol P // ok
+@end
+
+@protocol new // expected-error {{expected identifier; 'new' is a keyword in Objective-C++}}
+@end
+
+@protocol P2, delete; // expected-error {{expected identifier; 'delete' is a keyword in Objective-C++}}
+
+@class Foo, try; // expected-error {{expected identifier; 'try' is a keyword in Objective-C++}}
+
+@interface Foo
+
+@property (readwrite, nonatomic) int a, b, throw; // expected-error {{expected member name or ';' after declaration specifiers; 'throw' is a keyword in Objective-C++}}
+
+-foo:(int)class; // expected-error {{expected identifier; 'class' is a keyword in Objective-C++}}
++foo:(int)constexpr; // expected-error {{expected identifier; 'constexpr' is a keyword in Objective-C++}}
+
+@end
+
+@interface Foo () <P, new> // expected-error {{expected identifier; 'new' is a keyword in Objective-C++}}
+@end
+
+@implementation Foo
+
+@synthesize a = _a; // ok
+@synthesize b = virtual; // expected-error {{expected identifier; 'virtual' is a keyword in Objective-C++}}
+
+@dynamic throw; // expected-error {{expected identifier; 'throw' is a keyword in Objective-C++}}
+
+-foo:(int)class { // expected-error {{expected identifier; 'class' is a keyword in Objective-C++}}
+}
+
+@end
+
+@implementation class // expected-error {{expected identifier; 'class' is a keyword in Objective-C++}}
+@end
+
+@implementation Bar: class // expected-error {{expected identifier; 'class' is a keyword in Objective-C++}}
+@end
+
+@compatibility_alias C Foo; // ok
+@compatibility_alias const_cast Bar; // expected-error {{expected identifier; 'const_cast' is a keyword in Objective-C++}}
+@compatibility_alias C2 class; // expected-error {{expected identifier; 'class' is a keyword in Objective-C++}}
+
+void func() {
+  (void)@protocol(P); // ok
+  (void)@protocol(delete); // expected-error {{expected identifier; 'delete' is a keyword in Objective-C++}}
+}
Index: lib/Parse/Parser.cpp
===================================================================
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -232,6 +232,21 @@
       << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
 }
 
+bool Parser::expectIdentifier() {
+  if (Tok.is(tok::identifier))
+    return false;
+  if (const auto *II = Tok.getIdentifierInfo()) {
+    if (II->isCPlusPlusKeyword(getLangOpts())) {
+      Diag(Tok, diag::err_expected_token_instead_of_objcxx_keyword)
+          << tok::identifier << Tok.getIdentifierInfo();
+      // Objective-C++: Recover by treating this keyword as a valid identifier.
+      return false;
+    }
+  }
+  Diag(Tok, diag::err_expected) << tok::identifier;
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // Error recovery.
 //===----------------------------------------------------------------------===//
Index: lib/Parse/ParseObjc.cpp
===================================================================
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -137,8 +137,7 @@
 
   while (1) {
     MaybeSkipAttributes(tok::objc_class);
-    if (Tok.isNot(tok::identifier)) {
-      Diag(Tok, diag::err_expected) << tok::identifier;
+    if (expectIdentifier()) {
       SkipUntil(tok::semi);
       return Actions.ConvertDeclToDeclGroup(nullptr);
     }
@@ -229,11 +228,8 @@
 
   MaybeSkipAttributes(tok::objc_interface);
 
-  if (Tok.isNot(tok::identifier)) {
-    Diag(Tok, diag::err_expected)
-        << tok::identifier; // missing class or category name.
-    return nullptr;
-  }
+  if (expectIdentifier())
+    return nullptr; // missing class or category name.
 
   // We have a class or category name - consume it.
   IdentifierInfo *nameId = Tok.getIdentifierInfo();
@@ -329,11 +325,8 @@
       return nullptr;
     }
 
-    if (Tok.isNot(tok::identifier)) {
-      Diag(Tok, diag::err_expected)
-          << tok::identifier; // missing super class name.
-      return nullptr;
-    }
+    if (expectIdentifier())
+      return nullptr; // missing super class name.
     superClassId = Tok.getIdentifierInfo();
     superClassLoc = ConsumeToken();
 
@@ -1448,12 +1441,9 @@
       cutOffParsing();
       return nullptr;
     }
-    
-    if (Tok.isNot(tok::identifier)) {
-      Diag(Tok, diag::err_expected)
-          << tok::identifier; // missing argument name.
-      break;
-    }
+
+    if (expectIdentifier())
+      break; // missing argument name.
 
     ArgInfo.Name = Tok.getIdentifierInfo();
     ArgInfo.NameLoc = Tok.getLocation();
@@ -1562,8 +1552,7 @@
       return true;
     }
 
-    if (Tok.isNot(tok::identifier)) {
-      Diag(Tok, diag::err_expected) << tok::identifier;
+    if (expectIdentifier()) {
       SkipUntil(tok::greater, StopAtSemi);
       return true;
     }
@@ -2045,10 +2034,8 @@
 
   MaybeSkipAttributes(tok::objc_protocol);
 
-  if (Tok.isNot(tok::identifier)) {
-    Diag(Tok, diag::err_expected) << tok::identifier; // missing protocol name.
-    return nullptr;
-  }
+  if (expectIdentifier())
+    return nullptr; // missing protocol name.
   // Save the protocol name, then consume it.
   IdentifierInfo *protocolName = Tok.getIdentifierInfo();
   SourceLocation nameLoc = ConsumeToken();
@@ -2068,8 +2055,7 @@
     // Parse the list of forward declarations.
     while (1) {
       ConsumeToken(); // the ','
-      if (Tok.isNot(tok::identifier)) {
-        Diag(Tok, diag::err_expected) << tok::identifier;
+      if (expectIdentifier()) {
         SkipUntil(tok::semi);
         return nullptr;
       }
@@ -2136,11 +2122,8 @@
 
   MaybeSkipAttributes(tok::objc_implementation);
 
-  if (Tok.isNot(tok::identifier)) {
-    Diag(Tok, diag::err_expected)
-        << tok::identifier; // missing class or category name.
-    return nullptr;
-  }
+  if (expectIdentifier())
+    return nullptr; // missing class or category name.
   // We have a class or category name - consume it.
   IdentifierInfo *nameId = Tok.getIdentifierInfo();
   SourceLocation nameLoc = ConsumeToken(); // consume class or category name
@@ -2210,11 +2193,8 @@
     IdentifierInfo *superClassId = nullptr;
     if (TryConsumeToken(tok::colon)) {
       // We have a super class
-      if (Tok.isNot(tok::identifier)) {
-        Diag(Tok, diag::err_expected)
-            << tok::identifier; // missing super class name.
-        return nullptr;
-      }
+      if (expectIdentifier())
+        return nullptr; // missing super class name.
       superClassId = Tok.getIdentifierInfo();
       superClassLoc = ConsumeToken(); // Consume super class name
     }
@@ -2314,16 +2294,12 @@
   assert(Tok.isObjCAtKeyword(tok::objc_compatibility_alias) &&
          "ParseObjCAtAliasDeclaration(): Expected @compatibility_alias");
   ConsumeToken(); // consume compatibility_alias
-  if (Tok.isNot(tok::identifier)) {
-    Diag(Tok, diag::err_expected) << tok::identifier;
+  if (expectIdentifier())
     return nullptr;
-  }
   IdentifierInfo *aliasId = Tok.getIdentifierInfo();
   SourceLocation aliasLoc = ConsumeToken(); // consume alias-name
-  if (Tok.isNot(tok::identifier)) {
-    Diag(Tok, diag::err_expected) << tok::identifier;
+  if (expectIdentifier())
     return nullptr;
-  }
   IdentifierInfo *classId = Tok.getIdentifierInfo();
   SourceLocation classLoc = ConsumeToken(); // consume class-name;
   ExpectAndConsume(tok::semi, diag::err_expected_after, "@compatibility_alias");
@@ -2371,11 +2347,9 @@
         cutOffParsing();
         return nullptr;
       }
-      
-      if (Tok.isNot(tok::identifier)) {
-        Diag(Tok, diag::err_expected) << tok::identifier;
+
+      if (expectIdentifier())
         break;
-      }
       propertyIvar = Tok.getIdentifierInfo();
       propertyIvarLoc = ConsumeToken(); // consume ivar-name
     }
@@ -2433,9 +2407,8 @@
       cutOffParsing();
       return nullptr;
     }
-    
-    if (Tok.isNot(tok::identifier)) {
-      Diag(Tok, diag::err_expected) << tok::identifier;
+
+    if (expectIdentifier()) {
       SkipUntil(tok::semi);
       return nullptr;
     }
@@ -3566,8 +3539,8 @@
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
 
-  if (Tok.isNot(tok::identifier))
-    return ExprError(Diag(Tok, diag::err_expected) << tok::identifier);
+  if (expectIdentifier())
+    return ExprError();
 
   IdentifierInfo *protocolId = Tok.getIdentifierInfo();
   SourceLocation ProtoIdLoc = ConsumeToken();
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -5402,6 +5402,21 @@
     if (Tok.is(tok::l_square))
       return ParseMisplacedBracketDeclarator(D);
     if (D.getContext() == Declarator::MemberContext) {
+      // Objective-C++: Detect C++ keywords and try to prevent further errors by
+      // treating these keyword as valid member names.
+      if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus &&
+          Tok.getIdentifierInfo() &&
+          Tok.getIdentifierInfo()->isCPlusPlusKeyword(getLangOpts())) {
+        Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
+             diag::err_expected_member_name_or_semi_objcxx_keyword)
+            << Tok.getIdentifierInfo()
+            << (D.getDeclSpec().isEmpty() ? SourceRange()
+                                          : D.getDeclSpec().getSourceRange());
+        D.SetIdentifier(Tok.getIdentifierInfo(), Tok.getLocation());
+        D.SetRangeEnd(Tok.getLocation());
+        ConsumeToken();
+        goto PastIdentifier;
+      }
       Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
            diag::err_expected_member_name_or_semi)
           << (D.getDeclSpec().isEmpty() ? SourceRange()
Index: lib/Basic/IdentifierTable.cpp
===================================================================
--- lib/Basic/IdentifierTable.cpp
+++ lib/Basic/IdentifierTable.cpp
@@ -244,7 +244,7 @@
 
 /// \brief Returns true if the identifier represents a keyword in the
 /// specified language.
-bool IdentifierInfo::isKeyword(const LangOptions &LangOpts) {
+bool IdentifierInfo::isKeyword(const LangOptions &LangOpts) const {
   switch (getTokenKwStatus(LangOpts, getTokenID())) {
   case KS_Enabled:
   case KS_Extension:
@@ -254,6 +254,19 @@
   }
 }
 
+/// \brief Returns true if the identifier represents a C++ keyword in the
+/// specified language.
+bool IdentifierInfo::isCPlusPlusKeyword(const LangOptions &LangOpts) const {
+  if (!LangOpts.CPlusPlus || !isKeyword(LangOpts))
+    return false;
+  // This is a C++ keyword if this identifier is not a keyword when checked
+  // using LangOptions without C++ support.
+  LangOptions LangOptsNoCPP = LangOpts;
+  LangOptsNoCPP.CPlusPlus = false;
+  LangOptsNoCPP.CPlusPlus11 = false;
+  return !isKeyword(LangOptsNoCPP);
+}
+
 tok::PPKeywordKind IdentifierInfo::getPPKeywordID() const {
   // We use a perfect hash function here involving the length of the keyword,
   // the first and third character.  For preprocessor ID's there are no
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -793,6 +793,14 @@
   /// \brief Consume any extra semi-colons until the end of the line.
   void ConsumeExtraSemi(ExtraSemiKind Kind, unsigned TST = TST_unspecified);
 
+  /// Return false if the next token is an identifier. An 'expected identifier'
+  /// error is emitted otherwise.
+  ///
+  /// The parser tries to recover from the error by checking if the next token
+  /// is a C++ keyword when parsing Objective-C++. Return false if the recovery
+  /// was successful.
+  bool expectIdentifier();
+
 public:
   //===--------------------------------------------------------------------===//
   // Scope manipulation
Index: include/clang/Basic/IdentifierTable.h
===================================================================
--- include/clang/Basic/IdentifierTable.h
+++ include/clang/Basic/IdentifierTable.h
@@ -272,7 +272,11 @@
   bool isCPlusPlusOperatorKeyword() const { return IsCPPOperatorKeyword; }
 
   /// \brief Return true if this token is a keyword in the specified language.
-  bool isKeyword(const LangOptions &LangOpts);
+  bool isKeyword(const LangOptions &LangOpts) const;
+
+  /// \brief Return true if this token is a C++ keyword in the specified
+  /// language.
+  bool isCPlusPlusKeyword(const LangOptions &LangOpts) const;
 
   /// getFETokenInfo/setFETokenInfo - The language front-end is allowed to
   /// associate arbitrary metadata with this token.
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -438,6 +438,13 @@
   "declaration does not declare a parameter">;
 def err_no_matching_param : Error<"parameter named %0 is missing">;
 
+/// Objective-C++ parser diagnostics
+def err_expected_token_instead_of_objcxx_keyword : Error<
+  "expected %0; %1 is a keyword in Objective-C++">;
+def err_expected_member_name_or_semi_objcxx_keyword : Error<
+  "expected member name or ';' after declaration specifiers; "
+  "%0 is a keyword in Objective-C++">;
+
 /// C++ parser diagnostics
 def err_invalid_operator_on_type : Error<
   "cannot use %select{dot|arrow}0 operator on a type">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to