Sema: Implicit conversion for single-expression closures of Never type

This fixes a usability regression with the removal of @NoReturn
in Swift 3. Previously, it was legal to write this:

let callback: () -> Int = { fatalError() }

Now that the special @NoReturn attribute has been replaced with
a Never type, the above fails to typecheck, because the expression
now has type 'Never', and we expect a value of type 'Int'.

Getting around this behavior requires ugly workarounds to force the
parser to treat the body as a statement rather than an expression;
for example,

let callback: () -> Int = { _ = (); fatalError() }

This patch generalized single-expression closures to allow
the 'Never to T' conversion. Note that this is rather narrow
in scope -- it only applies to closure *literals*, single-expression
ones at that, not arbitrary function *values*.

In fact, it is not really a conversion at all, but more of a
desugaring rule for single-expression closures. They can now be
summarized as follows:

- If the closure literal has contextual return type T and
  the expression has Never type, the closure desugars as
  { _ = <expr> }, with no ReturnStmt.

- If the closure literal has contextual return type T for some
  non-void type T, the closure desugars as { return <expr> };
  the expression type must be convertible to T.

- If the closure literal has contextual return type Void, and
  the expression has some non-Void type T, the closure
  desugars as { _ = <expr>; return () }.

Fixes <rdar://problem/28269358> and <https://bugs.swift.org/browse/SR-2661>.

Author: slavapestov

include/swift/AST/Expr.h

@@ -324,12 +324,8 @@ class alignas(8) Expr {
     /// True if closure parameters were synthesized from anonymous closure
     /// variables.
     unsigned HasAnonymousClosureVars : 1;
-    
-    /// True if this is a closure created as a result of a void contextual
-    /// conversion.
-    unsigned IsVoidConversionClosure : 1;
   };
-  enum { NumClosureExprBits = NumAbstractClosureExprBits + 2 };
+  enum { NumClosureExprBits = NumAbstractClosureExprBits + 1 };
   static_assert(NumClosureExprBits <= 32, "fits in an unsigned");
 
   class BindOptionalExprBitfields {
@@ -3375,7 +3371,6 @@ class ClosureExpr : public AbstractClosureExpr {
       Body(nullptr) {
     setParameterList(params);
     ClosureExprBits.HasAnonymousClosureVars = false;
-    ClosureExprBits.IsVoidConversionClosure = false;
   }
 
   SourceRange getSourceRange() const;
@@ -3401,18 +3396,6 @@ class ClosureExpr : public AbstractClosureExpr {
     ClosureExprBits.HasAnonymousClosureVars = true;
   }
 
-  /// \brief Determine if this closure was created to satisfy a contextual
-  /// conversion to a void function type.
-  bool isVoidConversionClosure() const {
-    return ClosureExprBits.IsVoidConversionClosure;
-  }
-  
-  /// \brief Indicate that this closure was created to satisfy a contextual
-  /// conversion to a void function type.
-  void setIsVoidConversionClosure() {
-    ClosureExprBits.IsVoidConversionClosure = true;
-  }
-
   /// \brief Determine whether this closure expression has an
   /// explicitly-specified result type.
   bool hasExplicitResultType() const { return ArrowLoc.isValid(); }

lib/AST/ASTDumper.cpp

@@ -2081,8 +2081,6 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
     printClosure(E, "closure_expr");
     if (E->hasSingleExpressionBody())
       OS << " single-expression";
-    if (E->isVoidConversionClosure())
-      OS << " void-conversion";
 
     if (E->getParameters()) {
       OS << '\n';

lib/AST/Expr.cpp

@@ -1811,22 +1811,20 @@ FORWARD_SOURCE_LOCS_TO(ClosureExpr, Body.getPointer())
 
 Expr *ClosureExpr::getSingleExpressionBody() const {
   assert(hasSingleExpressionBody() && "Not a single-expression body");
-  if (!isVoidConversionClosure()) {
-    return cast<ReturnStmt>(getBody()->getElement(0).get<Stmt *>())
-             ->getResult();
-  } else {
-    return getBody()->getElement(0).get<Expr *>();
-  }
+  auto body = getBody()->getElement(0);
+  if (body.is<Stmt *>())
+    return cast<ReturnStmt>(body.get<Stmt *>())->getResult();
+  return body.get<Expr *>();
 }
 
 void ClosureExpr::setSingleExpressionBody(Expr *NewBody) {
   assert(hasSingleExpressionBody() && "Not a single-expression body");
-  if (!isVoidConversionClosure()) {
-    cast<ReturnStmt>(getBody()->getElement(0).get<Stmt *>())
-      ->setResult(NewBody);
-  } else {
-    return getBody()->setElement(0, NewBody);
+  auto body = getBody()->getElement(0);
+  if (body.is<Stmt *>()) {
+    cast<ReturnStmt>(body.get<Stmt *>())->setResult(NewBody);
+    return;
   }
+  getBody()->setElement(0, NewBody);
 }
 
 FORWARD_SOURCE_LOCS_TO(AutoClosureExpr, Body)

lib/Sema/CSApply.cpp

@@ -1144,10 +1144,10 @@ namespace {
     }
 
   public:
-    
-    
-    /// \brief Coerce a closure expression with a non-void return type to a
-    /// contextual function type with a void return type.
+
+
+    /// \brief Coerce a closure expression with a non-Void return type to a
+    /// contextual function type with a Void return type.
     ///
     /// This operation cannot fail.
     ///
@@ -1156,6 +1156,17 @@ namespace {
     /// \returns The coerced closure expression.
     ///
     ClosureExpr *coerceClosureExprToVoid(ClosureExpr *expr);
+
+    /// \brief Coerce a closure expression with a Never return type to a
+    /// contextual function type with some other return type.
+    ///
+    /// This operation cannot fail.
+    ///
+    /// \param expr The closure expression to coerce.
+    ///
+    /// \returns The coerced closure expression.
+    ///
+    ClosureExpr *coerceClosureExprFromNever(ClosureExpr *expr);
 
     /// \brief Coerce the given expression to the given type.
     ///
@@ -4954,14 +4965,12 @@ ClosureExpr *ExprRewriter::coerceClosureExprToVoid(ClosureExpr *closureExpr) {
 
   // Re-write the single-expression closure to return '()'
   assert(closureExpr->hasSingleExpressionBody());
-  
-  // Transform the ClosureExpr representation into the "expr + return ()" rep
-  // if it isn't already.
-  if (!closureExpr->isVoidConversionClosure()) {
-    
-    auto member = closureExpr->getBody()->getElement(0);
-    
-    // A single-expression body contains a single return statement.
+
+  // A single-expression body contains a single return statement
+  // prior to this transformation.
+  auto member = closureExpr->getBody()->getElement(0);
+ 
+  if (member.is<Stmt *>()) {
     auto returnStmt = cast<ReturnStmt>(member.get<Stmt *>());
     auto singleExpr = returnStmt->getResult();
     auto voidExpr = TupleExpr::createEmpty(tc.Context,
@@ -4988,7 +4997,6 @@ ClosureExpr *ExprRewriter::coerceClosureExprToVoid(ClosureExpr *closureExpr) {
                                        /*implicit*/true);
 
     closureExpr->setImplicit();
-    closureExpr->setIsVoidConversionClosure();
     closureExpr->setBody(braceStmt, /*isSingleExpression*/true);
   }
 
@@ -5002,6 +5010,38 @@ ClosureExpr *ExprRewriter::coerceClosureExprToVoid(ClosureExpr *closureExpr) {
   return closureExpr;
 }
 
+ClosureExpr *ExprRewriter::coerceClosureExprFromNever(ClosureExpr *closureExpr) {
+  auto &tc = cs.getTypeChecker();
+
+  // Re-write the single-expression closure to drop the 'return'.
+  assert(closureExpr->hasSingleExpressionBody());
+
+  // A single-expression body contains a single return statement
+  // prior to this transformation.
+  auto member = closureExpr->getBody()->getElement(0);
+
+  if (member.is<Stmt *>()) {
+    auto returnStmt = cast<ReturnStmt>(member.get<Stmt *>());
+    auto singleExpr = returnStmt->getResult();
+
+    tc.checkIgnoredExpr(singleExpr);
+
+    SmallVector<ASTNode, 1> elements;
+    elements.push_back(singleExpr);
+
+    auto braceStmt = BraceStmt::create(tc.Context,
+                                       closureExpr->getStartLoc(),
+                                       elements,
+                                       closureExpr->getEndLoc(),
+                                       /*implicit*/true);
+
+    closureExpr->setImplicit();
+    closureExpr->setBody(braceStmt, /*isSingleExpression*/true);
+  }
+
+  return closureExpr;
+}
+
 static void
 maybeDiagnoseUnsupportedFunctionConversion(TypeChecker &tc, Expr *expr,
                                            AnyFunctionType *toType) {
@@ -6397,9 +6437,15 @@ namespace {
             closure->setSingleExpressionBody(body);
 
           if (body) {
-            
+            // A single-expression closure with a non-Void expression type
+            // coerces to a Void-returning function type.
             if (fnType->getResult()->isVoid() && !body->getType()->isVoid()) {
               closure = Rewriter.coerceClosureExprToVoid(closure);
+            // A single-expression closure with a Never expression type
+            // coerces to any other function type.
+            } else if (!fnType->getResult()->isUninhabited() &&
+                       body->getType()->isUninhabited()) {
+              closure = Rewriter.coerceClosureExprFromNever(closure);
             } else {
 
               body = Rewriter.coerceToType(body,

lib/Sema/CSSimplify.cpp

@@ -1956,10 +1956,11 @@ ConstraintSystem::matchTypes(Type type1, Type type2, TypeMatchKind kind,
     }
   }
 
-  // If the types disagree, but we're comparing a non-void, single-expression
-  // closure result type to a void function result type, allow the conversion.
+  // Allow '() -> T' to '() -> ()' and '() -> Never' to '() -> T' for closure
+  // literals.
   {
-    if (concrete && kind >= TypeMatchKind::Subtype && type2->isVoid()) {
+    if (concrete && kind >= TypeMatchKind::Subtype &&
+        (type1->isUninhabited() || type2->isVoid())) {
       SmallVector<LocatorPathElt, 2> parts;
       locator.getLocatorParts(parts);
 

test/Misc/single_expr_closure_conversion.swift

@@ -42,3 +42,60 @@ func expect<T>(_ expression: @autoclosure () -> T) -> Expectation<T> {
 }
 func describe(_ closure: () -> ()) {}
 func f() { describe { _ = expect("what") } }
+
+struct Blob {}
+
+func withBlob(block: (Blob) -> ()) {}
+
+protocol Binding {}
+extension Int: Binding {}
+extension Double: Binding {}
+extension String: Binding {}
+extension Blob: Binding {}
+
+struct Stmt {
+  @discardableResult
+  func bind(_ values: Binding?...) -> Stmt {
+    return self
+  }
+
+  @discardableResult
+  func bind(_ values: [Binding?]) -> Stmt {
+    return self
+  }
+
+  @discardableResult
+  func bind(_ values: [String: Binding?]) -> Stmt {
+    return self
+  }
+}
+
+let stmt = Stmt()
+withBlob { stmt.bind(1, 2.0, "3", $0) }
+withBlob { stmt.bind([1, 2.0, "3", $0]) }
+withBlob { stmt.bind(["1": 1, "2": 2.0, "3": "3", "4": $0]) }
+
+// <rdar://problem/19840785>
+// We shouldn't crash on the call to 'a.dispatch' below.
+class A {
+	func dispatch(_ f : () -> Void) {
+		f()
+	}
+}
+
+class C {
+	var prop = 0
+	var a = A()
+
+	func act() {
+		a.dispatch({() -> Void in
+                  self.prop // expected-warning {{expression of type 'Int' is unused}}
+                })
+	}
+}
+
+// Never-returning expressions
+func haltAndCatchFire() -> Never { while true { } }
+let backupPlan: () -> Int = { haltAndCatchFire() }
+func missionCritical(storage: () -> String) {}
+missionCritical(storage: { haltAndCatchFire() })