Implement bracket insertion for Objective-C instance message sends as

part of parser recovery. For example, given:

  a method1:arg];

we detect after parsing the expression "a" that we have the start of a
message send expression. We pretend we've seen a '[' prior to the a,
then parse the remainder as a message send. We'll then give a
diagnostic+fix-it such as:

fixit-objc-message.m:17:3: error: missing '[' at start of message
      send expression
  a method1:arg];
  ^
  [

The algorithm here is very simple, and always assumes that the open
bracket goes at the beginning of the message send. It also only works
for non-super instance message sends at this time.



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

10 files changed, 124 insertions, 28 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 9aadc1efa1..1ccc89f068 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2302,6 +2302,8 @@ def err_invalid_receiver_to_message_super : Error<
   "'super' is only valid in a method body">;
 def err_invalid_receiver_class_message : Error<
   "receiver type %0 is not an Objective-C class">;
+def err_missing_open_square_message_send : Error<
+  "missing '[' at start of message send expression">;
 def warn_bad_receiver_type : Warning<
   "receiver type %0 is not 'id' or interface pointer, consider "
   "casting it to 'id'">;
diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index a7dab1e667..479032c83f 100644
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -34,7 +34,8 @@ namespace clang {
   class Parser;
   class PragmaUnusedHandler;
   class ColonProtectionRAIIObject;
-
+  class InMessageExpressionRAIIObject;
+  
 /// PrettyStackTraceParserEntry - If a crash happens while the parser is active,
 /// an entry is printed for it.
 class PrettyStackTraceParserEntry : public llvm::PrettyStackTraceEntry {
@@ -75,6 +76,7 @@ namespace prec {
 class Parser : public CodeCompletionHandler {
   friend class PragmaUnusedHandler;
   friend class ColonProtectionRAIIObject;
+  friend class InMessageExpressionRAIIObject;
   friend class ParenBraceBracketBalancer;
   PrettyStackTraceParserEntry CrashInfo;
 
@@ -131,6 +133,13 @@ class Parser : public CodeCompletionHandler {
   /// ColonProtectionRAIIObject RAII object.
   bool ColonIsSacred;
 
+  /// \brief When true, we are directly inside an Ojective-C messsage 
+  /// send expression.
+  ///
+  /// This is managed by the \c InMessageExpressionRAIIObject class, and
+  /// should not be set directly.
+  bool InMessageExpression;
+  
   /// The "depth" of the template parameters currently being parsed.
   unsigned TemplateParameterDepth;
 
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
index 4ee924543a..1f5a6964e3 100644
--- a/lib/Parse/ParseExpr.cpp
+++ b/lib/Parse/ParseExpr.cpp
@@ -576,9 +576,10 @@ ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
     SourceLocation RParenLoc;
     
     {
-      // The inside of the parens don't need to be a colon protected scope.
+      // The inside of the parens don't need to be a colon protected scope, and
+      // isn't immediately a message send.
       ColonProtectionRAIIObject X(*this, false);
-    
+
       Res = ParseParenExpression(ParenExprType, false/*stopIfCastExr*/,
                                  TypeOfCast, CastTy, RParenLoc);
       if (Res.isInvalid()) 
@@ -978,6 +979,19 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
   SourceLocation Loc;
   while (1) {
     switch (Tok.getKind()) {
+    case tok::identifier:
+      // If we see identifier: after an expression, and we're not already in a
+      // message send, then this is probably a message send with a missing
+      // opening bracket '['.
+      if (getLang().ObjC1 && !InMessageExpression && 
+          NextToken().is(tok::colon)) {
+        LHS = ParseObjCMessageExpressionBody(SourceLocation(), SourceLocation(),
+                                             ParsedType(), LHS.get());
+        break;
+      }
+        
+      // Fall through; this isn't a message send.
+                
     default:  // Not a postfix-expression suffix.
       return move(LHS);
     case tok::l_square: {  // postfix-expression: p-e '[' expression ']'
@@ -1008,6 +1022,8 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
     }
 
     case tok::l_paren: {   // p-e: p-e '(' argument-expression-list[opt] ')'
+      InMessageExpressionRAIIObject InMessage(*this, false);
+      
       ExprVector ArgExprs(Actions);
       CommaLocsTy CommaLocs;
 
@@ -1483,8 +1499,13 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr,
       return ParseCXXAmbiguousParenExpression(ExprType, CastTy,
                                               OpenLoc, RParenLoc);
 
-    TypeResult Ty = ParseTypeName();
-
+    TypeResult Ty;
+    
+    {
+      InMessageExpressionRAIIObject InMessage(*this, false);
+      Ty = ParseTypeName();
+    }
+    
     // Match the ')'.
     if (Tok.is(tok::r_paren))
       RParenLoc = ConsumeParen();
@@ -1532,6 +1553,8 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr,
     return ExprError();
   } else if (TypeOfCast) {
     // Parse the expression-list.
+    InMessageExpressionRAIIObject InMessage(*this, false);
+    
     ExprVector ArgExprs(Actions);
     CommaLocsTy CommaLocs;
 
@@ -1541,6 +1564,8 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr,
                                           move_arg(ArgExprs), TypeOfCast);
     }
   } else {
+    InMessageExpressionRAIIObject InMessage(*this, false);
+    
     Result = ParseExpression();
     ExprType = SimpleExpr;
     if (!Result.isInvalid() && Tok.is(tok::r_paren))
diff --git a/lib/Parse/ParseInit.cpp b/lib/Parse/ParseInit.cpp
index 4347294141..c2675f33e7 100644
--- a/lib/Parse/ParseInit.cpp
+++ b/lib/Parse/ParseInit.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Parse/Parser.h"
 #include "clang/Parse/ParseDiagnostic.h"
+#include "RAIIObjectsForParser.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Scope.h"
 #include "llvm/ADT/SmallString.h"
@@ -136,6 +137,8 @@ ExprResult Parser::ParseInitializerWithPotentialDesignator() {
     //   [foo ... bar]     -> array designator
     //   [4][foo bar]      -> obsolete GNU designation with objc message send.
     //
+    InMessageExpressionRAIIObject InMessage(*this, true);
+    
     SourceLocation StartLoc = ConsumeBracket();
     ExprResult Idx;
 
@@ -146,7 +149,8 @@ ExprResult Parser::ParseInitializerWithPotentialDesignator() {
     if  (getLang().ObjC1 && getLang().CPlusPlus) {
       // Send to 'super'.
       if (Tok.is(tok::identifier) && Tok.getIdentifierInfo() == Ident_super &&
-          NextToken().isNot(tok::period) && getCurScope()->isInObjcMethodScope()) {
+          NextToken().isNot(tok::period) && 
+          getCurScope()->isInObjcMethodScope()) {
         CheckArrayDesignatorSyntax(*this, StartLoc, Desig);
         return ParseAssignmentExprWithObjCMessageExprStart(StartLoc,
                                                            ConsumeToken(),
@@ -310,6 +314,8 @@ ExprResult Parser::ParseInitializerWithPotentialDesignator() {
 ///         initializer-list ',' designation[opt] initializer
 ///
 ExprResult Parser::ParseBraceInitializer() {
+  InMessageExpressionRAIIObject InMessage(*this, false);
+  
   SourceLocation LBraceLoc = ConsumeBrace();
 
   /// InitExprs - This is the actual list of expressions contained in the
diff --git a/lib/Parse/ParseObjc.cpp b/lib/Parse/ParseObjc.cpp
index 5f5b716a61..c29c618b01 100644
--- a/lib/Parse/ParseObjc.cpp
+++ b/lib/Parse/ParseObjc.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/Parser.h"
+#include "RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/PrettyDeclStackTrace.h"
 #include "clang/Sema/Scope.h"
@@ -1785,6 +1786,8 @@ ExprResult Parser::ParseObjCAtExpression(SourceLocation AtLoc) {
 ///     simple-type-specifier
 ///     typename-specifier
 bool Parser::ParseObjCXXMessageReceiver(bool &IsExpr, void *&TypeOrExpr) {
+  InMessageExpressionRAIIObject InMessage(*this, true);
+
   if (Tok.is(tok::identifier) || Tok.is(tok::coloncolon) || 
       Tok.is(tok::kw_typename) || Tok.is(tok::annot_cxxscope))
     TryAnnotateTypeOrScopeToken();
@@ -1879,6 +1882,8 @@ ExprResult Parser::ParseObjCMessageExpression() {
     return ExprError();
   }
   
+  InMessageExpressionRAIIObject InMessage(*this, true);
+  
   if (getLang().CPlusPlus) {
     // We completely separate the C and C++ cases because C++ requires
     // more complicated (read: slower) parsing. 
@@ -1992,6 +1997,8 @@ Parser::ParseObjCMessageExpressionBody(SourceLocation LBracLoc,
                                        SourceLocation SuperLoc,
                                        ParsedType ReceiverType,
                                        ExprArg ReceiverExpr) {
+  InMessageExpressionRAIIObject InMessage(*this, true);
+
   if (Tok.is(tok::code_completion)) {
     if (SuperLoc.isValid())
       Actions.CodeCompleteObjCSuperMessage(getCurScope(), SuperLoc, 0, 0);
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 3ff940faf9..5ebee67b7a 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -460,7 +460,8 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
   PrettyStackTraceLoc CrashInfo(PP.getSourceManager(),
                                 Tok.getLocation(),
                                 "in compound statement ('{}')");
-
+  InMessageExpressionRAIIObject InMessage(*this, false);
+  
   SourceLocation LBraceLoc = ConsumeBrace();  // eat the '{'.
 
   // TODO: "__label__ X, Y, Z;" is the GNU "Local Label" extension.  These are
diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
index 02802773a0..cd4f342042 100644
--- a/lib/Parse/Parser.cpp
+++ b/lib/Parse/Parser.cpp
@@ -23,8 +23,8 @@ using namespace clang;
 
 Parser::Parser(Preprocessor &pp, Sema &actions)
   : CrashInfo(*this), PP(pp), Actions(actions), Diags(PP.getDiagnostics()),
-    GreaterThanIsOperator(true), ColonIsSacred(false),
-    TemplateParameterDepth(0) {
+    GreaterThanIsOperator(true), ColonIsSacred(false), 
+    InMessageExpression(false), TemplateParameterDepth(0) {
   Tok.setKind(tok::eof);
   Actions.CurScope = 0;
   NumCachedScopes = 0;
diff --git a/lib/Parse/RAIIObjectsForParser.h b/lib/Parse/RAIIObjectsForParser.h
index addc795083..583f18428d 100644
--- a/lib/Parse/RAIIObjectsForParser.h
+++ b/lib/Parse/RAIIObjectsForParser.h
@@ -80,6 +80,22 @@ namespace clang {
     }
   };
   
+  class InMessageExpressionRAIIObject {
+    bool &InMessageExpression;
+    bool OldValue;
+    
+  public:
+    InMessageExpressionRAIIObject(Parser &P, bool Value)
+      : InMessageExpression(P.InMessageExpression), 
+        OldValue(P.InMessageExpression) {
+      InMessageExpression = Value;
+    }
+    
+    ~InMessageExpressionRAIIObject() {
+      InMessageExpression = OldValue;
+    }
+  };
+  
   /// \brief RAII object that makes sure paren/bracket/brace count is correct
   /// after declaration/statement parsing, even when there's a parsing error.
   class ParenBraceBracketBalancer {
diff --git a/lib/Sema/SemaExprObjC.cpp b/lib/Sema/SemaExprObjC.cpp
index b56159c453..ebeed130e3 100644
--- a/lib/Sema/SemaExprObjC.cpp
+++ b/lib/Sema/SemaExprObjC.cpp
@@ -626,12 +626,12 @@ Sema::ObjCMessageKind Sema::getObjCMessageKind(Scope *S,
 }
 
 ExprResult Sema::ActOnSuperMessage(Scope *S, 
-                                               SourceLocation SuperLoc,
-                                               Selector Sel,
-                                               SourceLocation LBracLoc,
-                                               SourceLocation SelectorLoc,
-                                               SourceLocation RBracLoc,
-                                               MultiExprArg Args) {
+                                   SourceLocation SuperLoc,
+                                   Selector Sel,
+                                   SourceLocation LBracLoc,
+                                   SourceLocation SelectorLoc,
+                                   SourceLocation RBracLoc,
+                                   MultiExprArg Args) {
   // Determine whether we are inside a method or not.
   ObjCMethodDecl *Method = getCurMethodDecl();
   if (!Method) {
@@ -702,13 +702,21 @@ ExprResult Sema::ActOnSuperMessage(Scope *S,
 ///
 /// \param Args The message arguments.
 ExprResult Sema::BuildClassMessage(TypeSourceInfo *ReceiverTypeInfo,
-                                               QualType ReceiverType,
-                                               SourceLocation SuperLoc,
-                                               Selector Sel,
-                                               ObjCMethodDecl *Method,
-                                               SourceLocation LBracLoc, 
-                                               SourceLocation RBracLoc,
-                                               MultiExprArg ArgsIn) {
+                                   QualType ReceiverType,
+                                   SourceLocation SuperLoc,
+                                   Selector Sel,
+                                   ObjCMethodDecl *Method,
+                                   SourceLocation LBracLoc, 
+                                   SourceLocation RBracLoc,
+                                   MultiExprArg ArgsIn) {
+  SourceLocation Loc = SuperLoc.isValid()? SuperLoc
+    : ReceiverTypeInfo->getTypeLoc().getLocalSourceRange().getBegin();
+  if (LBracLoc.isInvalid()) {
+    Diag(Loc, diag::err_missing_open_square_message_send)
+      << FixItHint::CreateInsertion(Loc, "[");
+    LBracLoc = Loc;
+  }
+  
   if (ReceiverType->isDependentType()) {
     // If the receiver type is dependent, we can't type-check anything
     // at this point. Build a dependent expression.
@@ -720,9 +728,6 @@ ExprResult Sema::BuildClassMessage(TypeSourceInfo *ReceiverTypeInfo,
                                          Args, NumArgs, RBracLoc));
   }
   
-  SourceLocation Loc = SuperLoc.isValid()? SuperLoc
-             : ReceiverTypeInfo->getTypeLoc().getLocalSourceRange().getBegin();
-
   // Find the class to which we are sending this message.
   ObjCInterfaceDecl *Class = 0;
   const ObjCObjectType *ClassType = ReceiverType->getAs<ObjCObjectType>();
@@ -837,6 +842,15 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver,
                                                   SourceLocation LBracLoc, 
                                                   SourceLocation RBracLoc,
                                                   MultiExprArg ArgsIn) {
+  // The location of the receiver.
+  SourceLocation Loc = SuperLoc.isValid()? SuperLoc : Receiver->getLocStart();
+  
+  if (LBracLoc.isInvalid()) {
+    Diag(Loc, diag::err_missing_open_square_message_send)
+      << FixItHint::CreateInsertion(Loc, "[");
+    LBracLoc = Loc;
+  }
+
   // If we have a receiver expression, perform appropriate promotions
   // and determine receiver type.
   if (Receiver) {
@@ -858,9 +872,6 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver,
     ReceiverType = Receiver->getType();
   }
 
-  // The location of the receiver.
-  SourceLocation Loc = SuperLoc.isValid()? SuperLoc : Receiver->getLocStart();
-
   if (!Method) {
     // Handle messages to id.
     bool receiverIsId = ReceiverType->isObjCIdType();
diff --git a/test/FixIt/fixit-objc-message.m b/test/FixIt/fixit-objc-message.m
new file mode 100644
index 0000000000..0eaa7f0547
--- /dev/null
+++ b/test/FixIt/fixit-objc-message.m
@@ -0,0 +1,19 @@
+// Objective-C recovery
+// RUN: cp %s %t
+// RUN: %clang_cc1 -pedantic -Wall -fixit -x objective-c %t || true
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -x objective-c %t
+
+// Objective-C++ recovery
+// RUN: cp %s %t
+// RUN: %clang_cc1 -pedantic -Wall -fixit -x objective-c++ %t || true
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -x objective-c++ %t
+
+@interface A
+- (int)method1:(int)x second:(float)y;
++ (int)method2:(int)x second:(double)y;
+@end
+
+void f(A *a, int i, int j) {
+  a method1:5+2 second:+(3.14159)];
+  a method1:[a method1:3 second:j] second:i++]
+}