Rearrange and clean up how we disambiguate lambda-introducers from ObjC

message sends, designators, and attributes.

Instead of having the tentative parsing phase sometimes return an
indicator to say what diagnostic to produce if parsing fails and
sometimes ask the caller to run it again, consistently ask the caller to
try parsing again if tentative parsing would fail or is otherwise unable
to completely parse the lambda-introducer without producing an
irreversible semantic effect.

Mostly NFC, but we should recover marginally better in some error cases
(avoiding duplicate diagnostics).

llvm-svn: 361182

clang/include/clang/Parse/Parser.h

@@ -1752,16 +1752,29 @@ private:
                                       bool OnlyNamespace = false);
 
   //===--------------------------------------------------------------------===//
-  // C++0x 5.1.2: Lambda expressions
+  // C++11 5.1.2: Lambda expressions
+
+  /// Result of tentatively parsing a lambda-introducer.
+  enum class LambdaIntroducerTentativeParse {
+    /// This appears to be a lambda-introducer, which has been fully parsed.
+    Success,
+    /// This is a lambda-introducer, but has not been fully parsed, and this
+    /// function needs to be called again to parse it.
+    Incomplete,
+    /// This is definitely an Objective-C message send expression, rather than
+    /// a lambda-introducer, attribute-specifier, or array designator.
+    MessageSend,
+    /// This is not a lambda-introducer.
+    Invalid,
+  };
 
   // [...] () -> type {...}
   ExprResult ParseLambdaExpression();
   ExprResult TryParseLambdaExpression();
-  Optional<unsigned> ParseLambdaIntroducer(LambdaIntroducer &Intro,
+  bool
-                                           bool *SkippedInits = nullptr);
+  ParseLambdaIntroducer(LambdaIntroducer &Intro,
-  bool TryParseLambdaIntroducer(LambdaIntroducer &Intro);
+                        LambdaIntroducerTentativeParse *Tentative = nullptr);
-  ExprResult ParseLambdaExpressionAfterIntroducer(
+  ExprResult ParseLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro);
-               LambdaIntroducer &Intro);
 
   //===--------------------------------------------------------------------===//
   // C++ 5.2p1: C++ Casts

clang/lib/Parse/ParseExprCXX.cpp

@@ -686,9 +686,7 @@ ExprResult Parser::ParseCXXIdExpression(bool isAddressOfOperand) {
 ExprResult Parser::ParseLambdaExpression() {
   // Parse lambda-introducer.
   LambdaIntroducer Intro;
-  Optional<unsigned> DiagID = ParseLambdaIntroducer(Intro);
+  if (ParseLambdaIntroducer(Intro)) {
-  if (DiagID) {
-    Diag(Tok, DiagID.getValue());
     SkipUntil(tok::r_square, StopAtSemi);
     SkipUntil(tok::l_brace, StopAtSemi);
     SkipUntil(tok::r_brace, StopAtSemi);
@@ -698,9 +696,8 @@ ExprResult Parser::ParseLambdaExpression() {
   return ParseLambdaExpressionAfterIntroducer(Intro);
 }
 
-/// TryParseLambdaExpression - Use lookahead and potentially tentative
+/// Use lookahead and potentially tentative parsing to determine if we are
-/// parsing to determine if we are looking at a C++0x lambda expression, and parse
+/// looking at a C++11 lambda expression, and parse it if we are.
-/// it if we are.
 ///
 /// If we are not looking at a lambda expression, returns ExprError().
 ExprResult Parser::TryParseLambdaExpression() {
@@ -726,19 +723,44 @@ ExprResult Parser::TryParseLambdaExpression() {
 
   // If lookahead indicates an ObjC message send...
   // [identifier identifier
-  if (Next.is(tok::identifier) && After.is(tok::identifier)) {
+  if (Next.is(tok::identifier) && After.is(tok::identifier))
     return ExprEmpty();
-  }
 
   // Here, we're stuck: lambda introducers and Objective-C message sends are
   // unambiguous, but it requires arbitrary lookhead.  [a,b,c,d,e,f,g] is a
   // lambda, and [a,b,c,d,e,f,g h] is a Objective-C message send.  Instead of
   // writing two routines to parse a lambda introducer, just try to parse
   // a lambda introducer first, and fall back if that fails.
-  // (TryParseLambdaIntroducer never produces any diagnostic output.)
   LambdaIntroducer Intro;
-  if (TryParseLambdaIntroducer(Intro))
+  {
+    TentativeParsingAction TPA(*this);
+    LambdaIntroducerTentativeParse Tentative;
+    if (ParseLambdaIntroducer(Intro, &Tentative)) {
+      TPA.Commit();
+      return ExprError();
+    }
+
+    switch (Tentative) {
+    case LambdaIntroducerTentativeParse::Success:
+      TPA.Commit();
+      break;
+
+    case LambdaIntroducerTentativeParse::Incomplete:
+      // Didn't fully parse the lambda-introducer, try again with a
+      // non-tentative parse.
+      TPA.Revert();
+      Intro = LambdaIntroducer();
+      if (ParseLambdaIntroducer(Intro))
+        return ExprError();
+      break;
+
+    case LambdaIntroducerTentativeParse::MessageSend:
+    case LambdaIntroducerTentativeParse::Invalid:
+      // Not a lambda-introducer, might be a message send.
+      TPA.Revert();
       return ExprEmpty();
+    }
+  }
 
   return ParseLambdaExpressionAfterIntroducer(Intro);
 }
@@ -746,15 +768,16 @@ ExprResult Parser::TryParseLambdaExpression() {
 /// Parse a lambda introducer.
 /// \param Intro A LambdaIntroducer filled in with information about the
 ///        contents of the lambda-introducer.
-/// \param SkippedInits If non-null, we are disambiguating between an Obj-C
+/// \param Tentative If non-null, we are disambiguating between a
-///        message send and a lambda expression. In this mode, we will
+///        lambda-introducer and some other construct. In this mode, we do not
-///        sometimes skip the initializers for init-captures and not fully
+///        produce any diagnostics or take any other irreversible action unless
-///        populate \p Intro. This flag will be set to \c true if we do so.
+///        we're sure that this is a lambda-expression.
-/// \return A DiagnosticID if it hit something unexpected. The location for
+/// \return \c true if parsing (or disambiguation) failed with a diagnostic and
-///         the diagnostic is that of the current token.
+///         the caller should bail out / recover.
-Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
+bool Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
-                                                 bool *SkippedInits) {
+                                   LambdaIntroducerTentativeParse *Tentative) {
-  typedef Optional<unsigned> DiagResult;
+  if (Tentative)
+    *Tentative = LambdaIntroducerTentativeParse::Success;
 
   assert(Tok.is(tok::l_square) && "Lambda expressions begin with '['.");
   BalancedDelimiterTracker T(*this, tok::l_square);
@@ -762,37 +785,55 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
 
   Intro.Range.setBegin(T.getOpenLocation());
 
-  bool first = true;
+  bool First = true;
+
+  // Produce a diagnostic if we're not tentatively parsing; otherwise track
+  // that our parse has failed.
+  auto Invalid = [&](llvm::function_ref<void()> Action) {
+    if (Tentative) {
+      *Tentative = LambdaIntroducerTentativeParse::Invalid;
+      return false;
+    }
+    Action();
+    return true;
+  };
 
   // Parse capture-default.
   if (Tok.is(tok::amp) &&
       (NextToken().is(tok::comma) || NextToken().is(tok::r_square))) {
     Intro.Default = LCD_ByRef;
     Intro.DefaultLoc = ConsumeToken();
-    first = false;
+    First = false;
+    if (!Tok.getIdentifierInfo()) {
+      // This can only be a lambda; no need for tentative parsing any more.
+      // '[[and]]' can still be an attribute, though.
+      Tentative = nullptr;
+    }
   } else if (Tok.is(tok::equal)) {
     Intro.Default = LCD_ByCopy;
     Intro.DefaultLoc = ConsumeToken();
-    first = false;
+    First = false;
+    Tentative = nullptr;
   }
 
   while (Tok.isNot(tok::r_square)) {
-    if (!first) {
+    if (!First) {
       if (Tok.isNot(tok::comma)) {
         // Provide a completion for a lambda introducer here. Except
         // in Objective-C, where this is Almost Surely meant to be a message
         // send. In that case, fail here and let the ObjC message
         // expression parser perform the completion.
         if (Tok.is(tok::code_completion) &&
-            !(getLangOpts().ObjC && Intro.Default == LCD_None &&
+            !(getLangOpts().ObjC && Tentative)) {
-              !Intro.Captures.empty())) {
           Actions.CodeCompleteLambdaIntroducer(getCurScope(), Intro,
                                                /*AfterAmpersand=*/false);
           cutOffParsing();
           break;
         }
 
-        return DiagResult(diag::err_expected_comma_or_rsquare);
+        return Invalid([&] {
+          Diag(Tok.getLocation(), diag::err_expected_comma_or_rsquare);
+        });
       }
       ConsumeToken();
     }
@@ -800,7 +841,7 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
     if (Tok.is(tok::code_completion)) {
       // If we're in Objective-C++ and we have a bare '[', then this is more
       // likely to be a message receiver.
-      if (getLangOpts().ObjC && first)
+      if (getLangOpts().ObjC && Tentative && First)
         Actions.CodeCompleteObjCMessageReceiver(getCurScope());
       else
         Actions.CodeCompleteLambdaIntroducer(getCurScope(), Intro,
@@ -809,7 +850,7 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
       break;
     }
 
-    first = false;
+    First = false;
 
     // Parse capture.
     LambdaCaptureKind Kind = LCK_ByCopy;
@@ -826,7 +867,9 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
         ConsumeToken();
         Kind = LCK_StarThis;
       } else {
-        return DiagResult(diag::err_expected_star_this_capture);
+        return Invalid([&] {
+          Diag(Tok.getLocation(), diag::err_expected_star_this_capture);
+        });
       }
     } else if (Tok.is(tok::kw_this)) {
       Kind = LCK_This;
@@ -848,12 +891,14 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
         Id = Tok.getIdentifierInfo();
         Loc = ConsumeToken();
       } else if (Tok.is(tok::kw_this)) {
-        // FIXME: If we want to suggest a fixit here, will need to return more
+        return Invalid([&] {
-        // than just DiagnosticID. Perhaps full DiagnosticBuilder that can be
+          // FIXME: Suggest a fixit here.
-        // Clear()ed to prevent emission in case of tentative parsing?
+          Diag(Tok.getLocation(), diag::err_this_captured_by_reference);
-        return DiagResult(diag::err_this_captured_by_reference);
+        });
       } else {
-        return DiagResult(diag::err_expected_capture);
+        return Invalid([&] {
+          Diag(Tok.getLocation(), diag::err_expected_capture);
+        });
       }
 
       if (Tok.is(tok::l_paren)) {
@@ -864,9 +909,9 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
 
         ExprVector Exprs;
         CommaLocsTy Commas;
-        if (SkippedInits) {
+        if (Tentative) {
           Parens.skipToEnd();
-          *SkippedInits = true;
+          *Tentative = LambdaIntroducerTentativeParse::Incomplete;
         } else if (ParseExpressionList(Exprs, Commas)) {
           Parens.skipToEnd();
           Init = ExprError();
@@ -888,13 +933,13 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
         else
           InitKind = LambdaCaptureInitKind::ListInit;
 
-        if (!SkippedInits) {
+        if (!Tentative) {
           Init = ParseInitializer();
         } else if (Tok.is(tok::l_brace)) {
           BalancedDelimiterTracker Braces(*this, tok::l_brace);
           Braces.consumeOpen();
           Braces.skipToEnd();
-          *SkippedInits = true;
+          *Tentative = LambdaIntroducerTentativeParse::Incomplete;
         } else {
           // We're disambiguating this:
           //
@@ -937,9 +982,19 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
             ConsumeAnnotationToken();
           }
         }
-      } else
+      } else {
         TryConsumeToken(tok::ellipsis, EllipsisLoc);
       }
+    }
+
+    // Check if this is a message send before we act on a possible init-capture.
+    if (Tentative && Tok.is(tok::identifier) &&
+        NextToken().isOneOf(tok::colon, tok::r_square)) {
+      // This can only be a message send. We're done with disambiguation.
+      *Tentative = LambdaIntroducerTentativeParse::MessageSend;
+      return false;
+    }
+
     // If this is an init capture, process the initialization expression
     // right away.  For lambda init-captures such as the following:
     // const int x = 10;
@@ -980,7 +1035,9 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
     // that would be an error.
 
     ParsedType InitCaptureType;
-    if (!Init.isInvalid())
+    if (Tentative && Init.isUsable())
+      *Tentative = LambdaIntroducerTentativeParse::Incomplete;
+    else if (Init.isUsable()) {
       Init = Actions.CorrectDelayedTyposInExpr(Init.get());
       if (Init.isUsable()) {
         // Get the pointer and store it in an lvalue, so we can use it as an
@@ -992,6 +1049,7 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
             Loc, Kind == LCK_ByRef, Id, InitKind, InitExpr);
         Init = InitExpr;
       }
+    }
 
     SourceLocation LocEnd = PrevTokLocation;
 
@@ -1001,41 +1059,7 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
 
   T.consumeClose();
   Intro.Range.setEnd(T.getCloseLocation());
-  return DiagResult();
-}
-
-/// TryParseLambdaIntroducer - Tentatively parse a lambda introducer.
-///
-/// Returns true if it hit something unexpected.
-bool Parser::TryParseLambdaIntroducer(LambdaIntroducer &Intro) {
-  {
-    bool SkippedInits = false;
-    TentativeParsingAction PA1(*this);
-
-    if (ParseLambdaIntroducer(Intro, &SkippedInits)) {
-      PA1.Revert();
-      return true;
-    }
-
-    if (!SkippedInits) {
-      PA1.Commit();
   return false;
-    }
-
-    PA1.Revert();
-  }
-
-  // Try to parse it again, but this time parse the init-captures too.
-  Intro = LambdaIntroducer();
-  TentativeParsingAction PA2(*this);
-
-  if (!ParseLambdaIntroducer(Intro)) {
-    PA2.Commit();
-    return false;
-  }
-
-  PA2.Revert();
-  return true;
 }
 
 static void

clang/lib/Parse/ParseInit.cpp

@@ -65,15 +65,28 @@ bool Parser::MayBeDesignationStart() {
 
   // Parse up to (at most) the token after the closing ']' to determine
   // whether this is a C99 designator or a lambda.
-  TentativeParsingAction Tentative(*this);
+  RevertingTentativeParsingAction Tentative(*this);
 
   LambdaIntroducer Intro;
-  bool SkippedInits = false;
+  LambdaIntroducerTentativeParse ParseResult;
-  Optional<unsigned> DiagID(ParseLambdaIntroducer(Intro, &SkippedInits));
+  if (ParseLambdaIntroducer(Intro, &ParseResult)) {
+    // Hit and diagnosed an error in a lambda.
+    // FIXME: Tell the caller this happened so they can recover.
+    return true;
+  }
 
-  if (DiagID) {
+  switch (ParseResult) {
-    // If this can't be a lambda capture list, it's a designator.
+  case LambdaIntroducerTentativeParse::Success:
-    Tentative.Revert();
+  case LambdaIntroducerTentativeParse::Incomplete:
+    // Might be a lambda-expression. Keep looking.
+    // FIXME: If our tentative parse was not incomplete, parse the lambda from
+    // here rather than throwing away then reparsing the LambdaIntroducer.
+    break;
+
+  case LambdaIntroducerTentativeParse::MessageSend:
+  case LambdaIntroducerTentativeParse::Invalid:
+    // Can't be a lambda-expression. Treat it as a designator.
+    // FIXME: Should we disambiguate against a message-send?
     return true;
   }
 
@@ -82,11 +95,7 @@ bool Parser::MayBeDesignationStart() {
   // lambda expression. This decision favors lambdas over the older
   // GNU designator syntax, which allows one to omit the '=', but is
   // consistent with GCC.
-  tok::TokenKind Kind = Tok.getKind();
+  return Tok.is(tok::equal);
-  // FIXME: If we didn't skip any inits, parse the lambda from here
-  // rather than throwing away then reparsing the LambdaIntroducer.
-  Tentative.Revert();
-  return Kind == tok::equal;
 }
 
 static void CheckArrayDesignatorSyntax(Parser &P, SourceLocation Loc,

clang/lib/Parse/ParseTentative.cpp

@@ -653,12 +653,15 @@ Parser::isCXX11AttributeSpecifier(bool Disambiguate,
   if (!Disambiguate && !getLangOpts().ObjC)
     return CAK_AttributeSpecifier;
 
+  // '[[using ns: ...]]' is an attribute.
+  if (GetLookAheadToken(2).is(tok::kw_using))
+    return CAK_AttributeSpecifier;
+
   RevertingTentativeParsingAction PA(*this);
 
   // Opening brackets were checked for above.
   ConsumeBracket();
 
-  // Outside Obj-C++11, treat anything with a matching ']]' as an attribute.
   if (!getLangOpts().ObjC) {
     ConsumeBracket();
 
@@ -677,15 +680,30 @@ Parser::isCXX11AttributeSpecifier(bool Disambiguate,
   //   4) [[obj]{ return self; }() doStuff]; Lambda in message send.
   // (1) is an attribute, (2) is ill-formed, and (3) and (4) are accepted.
 
-  // If we have a lambda-introducer, then this is definitely not a message send.
+  // Check to see if this is a lambda-expression.
   // FIXME: If this disambiguation is too slow, fold the tentative lambda parse
   // into the tentative attribute parse below.
+  {
+    RevertingTentativeParsingAction LambdaTPA(*this);
     LambdaIntroducer Intro;
-  if (!TryParseLambdaIntroducer(Intro)) {
+    LambdaIntroducerTentativeParse Tentative;
-    // A lambda cannot end with ']]', and an attribute must.
+    if (ParseLambdaIntroducer(Intro, &Tentative)) {
-    bool IsAttribute = Tok.is(tok::r_square);
+      // We hit a hard error after deciding this was not an attribute.
+      // FIXME: Don't parse and annotate expressions when disambiguating
+      // against an attribute.
+      return CAK_NotAttributeSpecifier;
+    }
 
-    if (IsAttribute)
+    switch (Tentative) {
+    case LambdaIntroducerTentativeParse::MessageSend:
+      // Case 3: The inner construct is definitely a message send, so the
+      // outer construct is definitely not an attribute.
+      return CAK_NotAttributeSpecifier;
+
+    case LambdaIntroducerTentativeParse::Success:
+    case LambdaIntroducerTentativeParse::Incomplete:
+      // This is a lambda-introducer or attribute-specifier.
+      if (Tok.is(tok::r_square))
         // Case 1: C++11 attribute.
         return CAK_AttributeSpecifier;
 
@@ -695,6 +713,12 @@ Parser::isCXX11AttributeSpecifier(bool Disambiguate,
 
       // Case 2: Lambda in array size / index.
       return CAK_InvalidAttributeSpecifier;
+
+    case LambdaIntroducerTentativeParse::Invalid:
+      // No idea what this is; we couldn't parse it as a lambda-introducer.
+      // Might still be an attribute-specifier or a message send.
+      break;
+    }
   }
 
   ConsumeBracket();

clang/test/Parser/objcxx11-invalid-lambda.cpp

@@ -4,7 +4,7 @@ void foo() {  // expected-note {{to match this '{'}}
   int bar;
   auto baz = [
       bar(  // expected-note {{to match this '('}} expected-note {{to match this '('}}
-        foo_undeclared() // expected-error{{use of undeclared identifier 'foo_undeclared'}} expected-error{{use of undeclared identifier 'foo_undeclared'}}
+        foo_undeclared() // expected-error{{use of undeclared identifier 'foo_undeclared'}}
       /* ) */
     ] () { };   // expected-error{{expected ')'}}
-}               // expected-error{{expected ')'}} expected-error{{expected ';' at end of declaration}} expected-error{{expected '}'}}
+}               // expected-error{{expected ')'}} expected-error {{expected ',' or ']'}} expected-error{{expected ';' at end of declaration}} expected-error{{expected '}'}}