Fix some issues in handling of withoutActuallyEscaping.

We need to strip inout/lvalue before casting the second parameter's type
to FunctionType.

There were also some verification issues and the fact that we weren't
allowing already-escaping closures to be passed to it (which is not
useful, but shouldn't result in an error and really awful
diagnostic). We can potentially look at diagnosing this with a warning
at some point in the future.

Fixes rdar://problem/32239354.

Author: rudkx

lib/AST/ASTVerifier.cpp

@@ -1839,35 +1839,51 @@ class Verifier : public ASTWalker {
         Out << "MakeTemporarilyEscapableExpr type does not match subexpression";
         abort();
       }
-      
-      // Closure and opaque value should both be functions, with the closure
-      // noescape and the opaque value escapable but otherwise matching.
-      auto closureFnTy = E->getNonescapingClosureValue()->getType()
-        ->getAs<FunctionType>();
-      if (!closureFnTy) {
-        Out << "MakeTemporarilyEscapableExpr closure type is not a closure";
+
+      auto call = dyn_cast<CallExpr>(E->getSubExpr());
+      if (!call) {
+        Out << "MakeTemporarilyEscapableExpr subexpression is not a call\n";
         abort();
       }
-      auto opaqueValueFnTy = E->getOpaqueValue()->getType()
-        ->getAs<FunctionType>();
-      if (!opaqueValueFnTy) {
-        Out<<"MakeTemporarilyEscapableExpr opaque value type is not a closure";
+
+      auto callFnTy = call->getFn()->getType()->getAs<FunctionType>();
+      if (!callFnTy) {
+        Out << "MakeTemporarilyEscapableExpr call does not call function\n";
         abort();
       }
-      if (!closureFnTy->isNoEscape()) {
-        Out << "MakeTemporarilyEscapableExpr closure type should be noescape";
+      if (!callFnTy->getExtInfo().isNoEscape()) {
+        Out << "MakeTemporarilyEscapableExpr called function is not noescape\n";
         abort();
       }
-      if (opaqueValueFnTy->isNoEscape()) {
-        Out << "MakeTemporarilyEscapableExpr opaque value type should be "
-               "escaping";
+
+      auto callArgTy = call->getArg()->getType()->getAs<FunctionType>();
+      if (!callArgTy) {
+        Out << "MakeTemporarilyEscapableExpr call argument is not a function\n";
+        abort();
+      }
+
+      // Closure and opaque value should both be functions, with the closure
+      // noescape and the opaque value escapable but otherwise matching.
+      auto closureFnTy =
+          E->getNonescapingClosureValue()->getType()->getAs<FunctionType>();
+      if (!closureFnTy) {
+        Out << "MakeTemporarilyEscapableExpr closure type is not a closure\n";
+        abort();
+      }
+      auto opaqueValueFnTy =
+          E->getOpaqueValue()->getType()->getAs<FunctionType>();
+      if (!opaqueValueFnTy) {
+        Out << "MakeTemporarilyEscapableExpr opaque value type is not a "
+               "closure\n";
         abort();
       }
-      if (!closureFnTy->isEqual(
-            opaqueValueFnTy->withExtInfo(opaqueValueFnTy->getExtInfo()
-                                                        .withNoEscape()))) {
+      auto closureFnNoEscape =
+          closureFnTy->withExtInfo(closureFnTy->getExtInfo().withNoEscape());
+      auto opaqueValueNoEscape = opaqueValueFnTy->withExtInfo(
+          opaqueValueFnTy->getExtInfo().withNoEscape());
+      if (!closureFnNoEscape->isEqual(opaqueValueNoEscape)) {
         Out << "MakeTemporarilyEscapableExpr closure and opaque value type "
-               "don't match";
+               "don't match\n";
         abort();
       }
     }

lib/Sema/CSApply.cpp

@@ -6745,7 +6745,8 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
         assert(arg->getNumElements() == 2 && "should have two arguments");
         auto nonescaping = arg->getElements()[0];
         auto body = arg->getElements()[1];
-        auto bodyFnTy = cs.getType(body)->castTo<FunctionType>();
+        auto bodyTy = cs.getType(body)->getLValueOrInOutObjectType();
+        auto bodyFnTy = bodyTy->castTo<FunctionType>();
         auto escapableType = bodyFnTy->getInput();
         auto resultType = bodyFnTy->getResult();
 

lib/Sema/CSSimplify.cpp

@@ -3682,9 +3682,6 @@ ConstraintSystem::simplifyEscapableFunctionOfConstraint(
 
   type2 = getFixedTypeRecursive(type2, flags, /*wantRValue=*/true);
   if (auto fn2 = type2->getAs<FunctionType>()) {
-    // We should have the noescape end of the relation.
-    if (!fn2->getExtInfo().isNoEscape())
-      return SolutionKind::Error;
     // Solve forward by binding the other type variable to the escapable
     // variation of this type.
     auto fn1 = fn2->withExtInfo(fn2->getExtInfo().withNoEscape(false));

test/Constraints/without_actually_escaping_no_errors.swift

@@ -0,0 +1,34 @@
+// RUN: %target-swift-frontend -module-name main -typecheck -swift-version 3 %s
+// RUN: %target-swift-frontend -module-name main -typecheck -swift-version 4 %s
+
+// These tests are split out to ensure that we run the AST verifier's
+// special post-type-checked verifications, which don't currently
+// happen if any errors occur anywhere during compilation.
+
+func rdar32239354_1(_ fn: () -> Void) {
+  var other: (() -> Void) -> Void = { _ in  }
+
+  withoutActuallyEscaping(fn, do: other)
+  // Reassign to avoid warning about changing this to a let, since we
+  // need this to be a var to trigger the original issue.
+  other = { _ in }
+}
+
+func rdar32239354_2(_ fn: () -> Void, other: inout (() -> Void) -> Void) {
+  withoutActuallyEscaping(fn, do: other)
+}
+
+func testVariations(
+  _ no_escape: () -> (),
+  _ escape: @escaping () -> (),
+  _ takesFn: (()->()) -> ()->()
+) -> () -> () {
+  withoutActuallyEscaping(no_escape) { _ in }
+  withoutActuallyEscaping({}) { _ in }
+  withoutActuallyEscaping(escape) { _ in }
+  _ = withoutActuallyEscaping(no_escape, do: takesFn)
+  _ = withoutActuallyEscaping(escape, do: takesFn)
+  _ = withoutActuallyEscaping(no_escape) {
+    return takesFn($0)
+  }
+}