Better parser recovery in Objective-C containers.

Previously it was possible to get an infinite-loop-on-invalid with a namespace
decl within @interface. Since 'namespace' is normally a safe place to retry
top-level parsing, we just didn't consume the token.

This adds a flag that tracks whether we have temporarily left Objective-C
scope to parse a C-like declaration, and uses that to better recover from
parse problems by stopping at possible method declarations and at @end. To
fix the original problem, we do /not/ stop at 'namespace' when in an
Objective-C @interface or @protocol context (but still do in @implementation).

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@159941 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Parse/Parser.h

@@ -23,6 +23,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include <stack>
 
 namespace clang {
@@ -83,6 +84,7 @@ class Parser : public CodeCompletionHandler {
   friend class ColonProtectionRAIIObject;
   friend class InMessageExpressionRAIIObject;
   friend class PoisonSEHIdentifiersRAIIObject;
+  friend class ObjCDeclContextSwitch;
   friend class ParenBraceBracketBalancer;
   friend class BalancedDelimiterTracker;
 
@@ -203,6 +205,13 @@ class Parser : public CodeCompletionHandler {
 
   IdentifierInfo *getSEHExceptKeyword();
 
+  /// True if we are within an Objective-C container while parsing C-like decls.
+  ///
+  /// This is necessary because Sema thinks we have left the container
+  /// to parse the C-like decls, meaning Actions.getObjCDeclContext() will
+  /// be NULL.
+  bool ParsingInObjCContainer;
+
   bool SkipFunctionBodies;
 
 public:
@@ -556,9 +565,11 @@ private:
   class ObjCDeclContextSwitch {
     Parser &P;
     Decl *DC;
+    SaveAndRestore<bool> WithinObjCContainer;
   public:
-    explicit ObjCDeclContextSwitch(Parser &p) : P(p),
-               DC(p.getObjCDeclContext()) {
+    explicit ObjCDeclContextSwitch(Parser &p)
+      : P(p), DC(p.getObjCDeclContext()),
+        WithinObjCContainer(P.ParsingInObjCContainer, DC != 0) {
       if (DC)
         P.Actions.ActOnObjCTemporaryExitContainerContext(cast<DeclContext>(DC));
     }

lib/Parse/ParseDecl.cpp

@@ -1270,15 +1270,33 @@ void Parser::SkipMalformedDecl() {
 
     case tok::kw_inline:
       // 'inline namespace' at the start of a line is almost certainly
-      // a good place to pick back up parsing.
-      if (Tok.isAtStartOfLine() && NextToken().is(tok::kw_namespace))
+      // a good place to pick back up parsing, except in an Objective-C
+      // @interface context.
+      if (Tok.isAtStartOfLine() && NextToken().is(tok::kw_namespace) &&
+          (!ParsingInObjCContainer || CurParsedObjCImpl))
         return;
       break;
 
     case tok::kw_namespace:
       // 'namespace' at the start of a line is almost certainly a good
-      // place to pick back up parsing.
-      if (Tok.isAtStartOfLine())
+      // place to pick back up parsing, except in an Objective-C
+      // @interface context.
+      if (Tok.isAtStartOfLine() &&
+          (!ParsingInObjCContainer || CurParsedObjCImpl))
+        return;
+      break;
+
+    case tok::at:
+      // @end is very much like } in Objective-C contexts.
+      if (NextToken().isObjCAtKeyword(tok::objc_end) &&
+          ParsingInObjCContainer)
+        return;
+      break;
+
+    case tok::minus:
+    case tok::plus:
+      // - and + probably start new method declarations in Objective-C contexts.
+      if (Tok.isAtStartOfLine() && ParsingInObjCContainer)
         return;
       break;
 

lib/Parse/Parser.cpp

@@ -49,7 +49,7 @@ Parser::Parser(Preprocessor &pp, Sema &actions, bool SkipFunctionBodies)
   : PP(pp), Actions(actions), Diags(PP.getDiagnostics()),
     GreaterThanIsOperator(true), ColonIsSacred(false), 
     InMessageExpression(false), TemplateParameterDepth(0),
-    SkipFunctionBodies(SkipFunctionBodies) {
+    ParsingInObjCContainer(false), SkipFunctionBodies(SkipFunctionBodies) {
   Tok.setKind(tok::eof);
   Actions.CurScope = 0;
   NumCachedScopes = 0;

test/Parser/objc-recover.mm

@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+
+@interface StopAtAtEnd
+// This used to eat the @end
+int 123 // expected-error{{expected unqualified-id}}
+@end
+
+@implementation StopAtAtEnd // no-warning
+int 123 // expected-error{{expected unqualified-id}}
+@end
+
+
+@interface StopAtMethodDecls
+// This used to eat the method declarations
+int 123 // expected-error{{expected unqualified-id}}
+- (void)foo; // expected-note{{here}}
+int 456 // expected-error{{expected unqualified-id}}
++ (void)bar; // expected-note{{here}}
+@end
+
+@implementation StopAtMethodDecls
+int 123 // expected-error{{expected unqualified-id}}
+- (id)foo {} // expected-warning{{conflicting return type}}
+int 456 // expected-error{{expected unqualified-id}}
++ (id)bar {} // expected-warning{{conflicting return type}}
+@end
+
+
+@interface EmbeddedNamespace
+// This used to cause an infinite loop.
+namespace NS { // expected-error{{expected unqualified-id}}
+}
+- (id)test; // expected-note{{here}}
+@end
+
+@implementation EmbeddedNamespace
+int 123 // expected-error{{expected unqualified-id}}
+// We should still stop here and parse this namespace.
+namespace NS {
+  void foo();
+}
+
+// Make sure the declaration of -test was recognized.
+- (void)test { // expected-warning{{conflicting return type}}
+  // Make sure the declaration of NS::foo was recognized.
+  NS::foo();
+}
+
+@end
+
+
+@protocol ProtocolWithEmbeddedNamespace
+namespace NS { // expected-error{{expected unqualified-id}}
+
+}
+- (void)PWEN_foo; // expected-note{{here}}
+@end
+
+@interface ImplementPWEN <ProtocolWithEmbeddedNamespace>
+@end
+
+@implementation ImplementPWEN
+- (id)PWEN_foo {} // expected-warning{{conflicting return type}}
+@end