Improve recovery when there is a stray ']' or ')' before the ';' at

the end of a statement. Fixes <rdar://problem/6896493>.

llvm-svn: 113202

7 files changed, 53 insertions, 20 deletions

diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 28cd2d98f07..427bae20950 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -124,6 +124,8 @@ def err_expected_while : Error<"expected 'while' in do/while loop">;
 def err_expected_semi_after : Error<"expected ';' after %0">;
 def err_expected_semi_after_stmt : Error<"expected ';' after %0 statement">;
 def err_expected_semi_after_expr : Error<"expected ';' after expression">;
+def err_extraneous_token_before_semi : Error<"extraneous '%0' before ';'">;
+
 def err_expected_semi_after_method_proto : Error<
   "expected ';' after method prototype">;
 def err_expected_semi_after_namespace_name : Error<
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 41a2fb61579..9c1630e899f 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -464,6 +464,13 @@ private:
                         const char *DiagMsg = "",
                         tok::TokenKind SkipToTok = tok::unknown);
 
+  /// \brief The parser expects a semicolon and, if present, will consume it.
+  ///
+  /// If the next token is not a semicolon, this emits the specified diagnostic,
+  /// or, if there's just some closing-delimiter noise (e.g., ')' or ']') prior
+  /// to the semicolon, consumes that extra token.
+  bool ExpectAndConsumeSemi(unsigned DiagID);
+  
   //===--------------------------------------------------------------------===//
   // Scope manipulation
 
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 67d49852395..8bb898591ed 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -313,8 +313,9 @@ Decl *Parser::ParseUsingDirective(unsigned Context,
   // Eat ';'.
   DeclEnd = Tok.getLocation();
   ExpectAndConsume(tok::semi,
-                   GNUAttr ? diag::err_expected_semi_after_attribute_list :
-                   diag::err_expected_semi_after_namespace_name, "", tok::semi);
+                   GNUAttr ? diag::err_expected_semi_after_attribute_list
+                           : diag::err_expected_semi_after_namespace_name, 
+                   "", tok::semi);
 
   return Actions.ActOnUsingDirective(getCurScope(), UsingLoc, NamespcLoc, SS,
                                       IdentLoc, NamespcName, Attr);
@@ -422,7 +423,7 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd){
   MatchRHSPunctuation(tok::r_paren, LParenLoc);
 
   DeclEnd = Tok.getLocation();
-  ExpectAndConsume(tok::semi, diag::err_expected_semi_after_static_assert);
+  ExpectAndConsumeSemi(diag::err_expected_semi_after_static_assert);
 
   return Actions.ActOnStaticAssertDeclaration(StaticAssertLoc,
                                               AssertExpr.take(),
diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index 6861ce940f7..5f5b716a61c 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -1731,7 +1731,7 @@ StmtResult Parser::ParseObjCAtStatement(SourceLocation AtLoc) {
   }
   
   // Otherwise, eat the semicolon.
-  ExpectAndConsume(tok::semi, diag::err_expected_semi_after_expr);
+  ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
   return Actions.ActOnExprStmt(Actions.MakeFullExpr(Res.take()));
 }
 
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 6c240e608c2..3ff940faf9d 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -137,7 +137,7 @@ Parser::ParseStatementOrDeclaration(bool OnlyStatement) {
       return StmtError();
     }
     // Otherwise, eat the semicolon.
-    ExpectAndConsume(tok::semi, diag::err_expected_semi_after_expr);
+    ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
     return Actions.ActOnExprStmt(Actions.MakeFullExpr(Expr.get()));
   }
 
@@ -507,7 +507,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
         // FIXME: Use attributes?
         // Eat the semicolon at the end of stmt and convert the expr into a
         // statement.
-        ExpectAndConsume(tok::semi, diag::err_expected_semi_after_expr);
+        ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
         R = Actions.ActOnExprStmt(Actions.MakeFullExpr(Res.get()));
       }
     }
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 44bd0fbc0c0..ca06ba72f64 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -162,6 +162,25 @@ bool Parser::ExpectAndConsume(tok::TokenKind ExpectedTok, unsigned DiagID,
   return true;
 }
 
+bool Parser::ExpectAndConsumeSemi(unsigned DiagID) {
+  if (Tok.is(tok::semi) || Tok.is(tok::code_completion)) {
+    ConsumeAnyToken();
+    return false;
+  }
+  
+  if ((Tok.is(tok::r_paren) || Tok.is(tok::r_square)) && 
+      NextToken().is(tok::semi)) {
+    Diag(Tok, diag::err_extraneous_token_before_semi)
+      << PP.getSpelling(Tok)
+      << FixItHint::CreateRemoval(Tok.getLocation());
+    ConsumeAnyToken(); // The ')' or ']'.
+    ConsumeToken(); // The ';'.
+    return false;
+  }
+  
+  return ExpectAndConsume(tok::semi, DiagID);
+}
+
 //===----------------------------------------------------------------------===//
 // Error recovery.
 //===----------------------------------------------------------------------===//
diff --git a/clang/test/FixIt/fixit-objc.m b/clang/test/FixIt/fixit-objc.m
index 03f28a1b586..bf704c66a02 100644
--- a/clang/test/FixIt/fixit-objc.m
+++ b/clang/test/FixIt/fixit-objc.m
@@ -1,6 +1,7 @@
+// RUN: %clang_cc1 -pedantic -verify %s
 // RUN: cp %s %t
-// RUN: %clang_cc1 -pedantic -fixit -x objective-c %t
-// RUN: %clang_cc1 -pedantic -verify -x objective-c %t
+// RUN: not %clang_cc1 -pedantic -fixit -x objective-c %t
+// RUN: %clang_cc1 -pedantic -Werror -x objective-c %t
 
 /* This is a test of the various code modification hints that are
    provided as part of warning or extension diagnostics. All of the
@@ -10,28 +11,31 @@
 @protocol X;
 
 void foo() {
-  <X> *P;    // should be fixed to 'id<X>'.
+  <X> *P;    // expected-warning{{protocol qualifiers without 'id' is archaic}}
 }
 
 @class A;
 @class NSString;
 
 @interface Test
-- (void)test:(NSString *)string;
+- (void)test:(NSString *)string; // expected-note{{passing argument to parameter 'string' here}}
 
 @property (copy) NSString *property;
 @end
 
-void g(NSString *a);
-void h(id a);
+void g(NSString *a); // expected-note{{passing argument to parameter 'a' here}}
+void h(id a); // expected-note 2{{passing argument to parameter 'a' here}}
 
 void f(Test *t) {
-  NSString *a = "Foo";
-  id b = "Foo";
-  A* c = "Foo"; // expected-warning {{incompatible pointer types initializing 'A *' with an expression of type 'char [4]'}}
-  g("Foo");
-  h("Foo");
-  h(("Foo"));
-  [t test:"Foo"];
-  t.property = "Foo";
+  NSString *a = "Foo"; // expected-warning {{incompatible pointer types initializing 'NSString *' with an expression of type 'char [4]'}}
+  id b = "Foo"; // expected-warning {{incompatible pointer types initializing 'id' with an expression of type 'char [4]'}}
+  g("Foo"); // expected-warning{{incompatible pointer types passing 'char [4]' to parameter of type 'NSString *'}}
+  h("Foo"); // expected-warning{{incompatible pointer types passing 'char [4]' to parameter of type 'id'}}
+  h(("Foo")); // expected-warning{{incompatible pointer types passing 'char [4]' to parameter of type 'id'}}
+  [t test:"Foo"]; // expected-warning{{incompatible pointer types sending 'char [4]' to parameter of type 'NSString *'}}
+  t.property = "Foo"; // expected-warning{{incompatible pointer types assigning to 'NSString *' from 'char [4]'}}
+
+  // <rdar://problem/6896493>
+  [t test:@"Foo"]]; // expected-error{{extraneous ']' before ';'}}
+  g(@"Foo")); // expected-error{{extraneous ')' before ';'}}
 }