Merge pull request #25863 from apple/revert-25165-improve_trailing_closure_in_condition_diag

Author: swift-ci

lib/Parse/ParseExpr.cpp

@@ -984,40 +984,23 @@ static bool isValidTrailingClosure(bool isExprBasic, Parser &P){
   // the token after the { is on the same line as the {.
   if (P.peekToken().isAtStartOfLine())
     return false;
-
+  
+  
   // Determine if the {} goes with the expression by eating it, and looking
-  // to see if it is immediately followed by a token which indicates we should
-  // consider it part of the preceding expression
+  // to see if it is immediately followed by '{', 'where', or comma.  If so,
+  // we consider it to be part of the proceeding expression.
   Parser::BacktrackingScope backtrack(P);
   P.consumeToken(tok::l_brace);
   P.skipUntil(tok::r_brace);
   SourceLoc endLoc;
-  if (!P.consumeIf(tok::r_brace, endLoc))
-    return false;
-
-  switch (P.Tok.getKind()) {
-  case tok::l_brace:
-  case tok::kw_where:
-  case tok::comma:
-    return true;
-  case tok::l_square:
-  case tok::l_paren:
-  case tok::period:
-  case tok::period_prefix:
-  case tok::kw_is:
-  case tok::kw_as:
-  case tok::question_postfix:
-  case tok::question_infix:
-  case tok::exclaim_postfix:
-  case tok::colon:
-  case tok::equal:
-  case tok::oper_postfix:
-  case tok::oper_binary_spaced:
-  case tok::oper_binary_unspaced:
-    return !P.Tok.isAtStartOfLine();
-  default:
+  if (!P.consumeIf(tok::r_brace, endLoc) ||
+      P.Tok.isNot(tok::l_brace, tok::kw_where, tok::comma)) {
     return false;
   }
+
+  // Recoverable case. Just return true here and Sema will emit a diagnostic
+  // later. see: Sema/MiscDiagnostics.cpp#checkStmtConditionTrailingClosure
+  return true;
 }
 
 

lib/Sema/MiscDiagnostics.cpp

@@ -2963,7 +2963,7 @@ void swift::fixItEncloseTrailingClosure(TypeChecker &TC,
 static void checkStmtConditionTrailingClosure(TypeChecker &TC, const Expr *E) {
   if (E == nullptr || isa<ErrorExpr>(E)) return;
 
-  // Walk into expressions which might have invalid trailing closures
+  // Shallow walker. just dig into implicit expression.
   class DiagnoseWalker : public ASTWalker {
     TypeChecker &TC;
 
@@ -2998,22 +2998,13 @@ static void checkStmtConditionTrailingClosure(TypeChecker &TC, const Expr *E) {
     bool shouldWalkIntoNonSingleExpressionClosure() override { return false; }
 
     std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
-      switch (E->getKind()) {
-      case ExprKind::Paren:
-      case ExprKind::Tuple:
-      case ExprKind::Array:
-      case ExprKind::Dictionary:
-      case ExprKind::InterpolatedStringLiteral:
-        // If a trailing closure appears as a child of one of these types of
-        // expression, don't diagnose it as there is no ambiguity.
-        return {E->isImplicit(), E};
-      case ExprKind::Call:
-        diagnoseIt(cast<CallExpr>(E));
-        break;
-      default:
-        break;
-      }
-      return {true, E};
+      // Dig into implicit expression.
+      if (E->isImplicit()) return { true, E };
+      // Diagnose call expression.
+      if (auto CE = dyn_cast<CallExpr>(E))
+        diagnoseIt(CE);
+      // Don't dig any further.
+      return { false, E };
     }
   };
 
@@ -3033,12 +3024,9 @@ static void checkStmtConditionTrailingClosure(TypeChecker &TC, const Expr *E) {
 static void checkStmtConditionTrailingClosure(TypeChecker &TC, const Stmt *S) {
   if (auto LCS = dyn_cast<LabeledConditionalStmt>(S)) {
     for (auto elt : LCS->getCond()) {
-      if (elt.getKind() == StmtConditionElement::CK_PatternBinding) {
+      if (elt.getKind() == StmtConditionElement::CK_PatternBinding)
         checkStmtConditionTrailingClosure(TC, elt.getInitializer());
-        if (auto *exprPattern = dyn_cast<ExprPattern>(elt.getPattern())) {
-          checkStmtConditionTrailingClosure(TC, exprPattern->getMatchExpr());
-        }
-      } else if (elt.getKind() == StmtConditionElement::CK_Boolean)
+      else if (elt.getKind() == StmtConditionElement::CK_Boolean)
         checkStmtConditionTrailingClosure(TC, elt.getBoolean());
       // No trailing closure for CK_Availability: e.g. `if #available() {}`.
     }

test/expr/closure/trailing.swift

@@ -138,7 +138,6 @@ limitXY(someInt, toGamut: intArray) {}  // expected-error{{cannot invoke 'limitX
 // <rdar://problem/23036383> QoI: Invalid trailing closures in stmt-conditions produce lowsy diagnostics
 func retBool(x: () -> Int) -> Bool {}
 func maybeInt(_: () -> Int) -> Int? {}
-func twoClosureArgs(_:()->Void, _:()->Void) -> Bool {}
 class Foo23036383 {
   init() {}
   func map(_: (Int) -> Int) -> Int? {}
@@ -203,82 +202,7 @@ func r23036383(foo: Foo23036383?, obj: Foo23036383) {
 
   if let _ = maybeInt { 1 }, retBool { 1 } { }
   // expected-error@-1 {{trailing closure requires parentheses for disambiguation in this context}} {{22-23=(}} {{28-28=)}} 
-  // expected-error@-2 {{trailing closure requires parentheses for disambiguation in this context}} {{37-38=(x: }} {{43-43=)}}
-
-  if let _ = foo?.map {$0+1}.map {$0+1} {} // expected-error {{trailing closure requires parentheses for disambiguation in this context}}  {{33-34=(}} {{40-40=)}}
-  // expected-error@-1 {{trailing closure requires parentheses for disambiguation in this context}} {{22-23=(}} {{29-29=)}}
-
-  if let _ = foo?.map {$0+1}.map({$0+1}) {} // expected-error {{trailing closure requires parentheses for disambiguation in this context}} {{22-23=(}} {{29-29=)}}
-
-  if let _ = foo?.map {$0+1}.map({$0+1}).map{$0+1} {} // expected-error {{trailing closure requires parentheses for disambiguation in this context}} {{45-45=(}} {{51-51=)}}
-  // expected-error@-1 {{trailing closure requires parentheses for disambiguation in this context}} {{22-23=(}} {{29-29=)}}
-
-  if twoClosureArgs({}) {} {} // expected-error {{trailing closure requires parentheses for disambiguation in this context}} {{23-25=, }} {{27-27=)}}
-
-  if let _ = (foo?.map {$0+1}.map({$0+1}).map{$0+1}) {} // OK
-
-  if let _ = (foo?.map {$0+1}.map({$0+1})).map({$0+1}) {} // OK
-}
-
-func id<T>(fn: () -> T) -> T { return fn() }
-func any<T>(fn: () -> T) -> Any { return fn() }
-
-func testSR8736() {
-  if !id { true } { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if id { true } == true { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if true == id { true } { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if id { true } ? true : false { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if true ? id { true } : false { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if true ? true : id { false } { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if id { [false,true] }[0] { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if id { { true } } () { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if any { true } as! Bool { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if let _ = any { "test" } as? Int { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if any { "test" } is Int { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if let _ = id { [] as [Int]? }?.first { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if id { true as Bool? }! { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if case id { 1 } = 1 { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if case 1 = id { 1 } { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if case 1 = id { 1 } /*comment*/ { return } // expected-error {{trailing closure requires parentheses for disambiguation in this context}}
-  
-  if case (id { 1 }) = 1 { return } // OK
-  
-  if case 1 = (id { 1 }) { return } // OK
-  
-  if [id { true }].count == 0 { return } // OK
-  
-  if [id { true } : "test"].keys.count == 0 { return } // OK
-  
-  if "\(id { true })" == "foo" { return } // OK
-  
-  if (id { true }) { return } // OK
-  
-  if (id { true }) { }
-  [1, 2, 3].count // expected-warning {{expression of type 'Int' is unused}}
-  
-  if true { }
-  () // OK
-  
-  if true
-  {
-    
-  }
-  () // OK
+  // expected-error@-2 {{trailing closure requires parentheses for disambiguation in this context}} {{37-38=(x: }} {{43-43=)}} 
 }
 
 func overloadOnLabel(a: () -> Void) {}