[concurrency] Perform some fixes so that transfernonsendable_closureliterals_isolationinference.swift now passes.

The reason why this failed is that concurrently to @xedin landing
79af04c, I enabled
NonisolatedNonsendingByDefault on a bunch of other tests. That change broke the
test and so we needed to fix it.

This commit fixes a few issues that were exposed:

1. We do not propagate nonisolated(nonsending) into a closure if its inferred
context isolation is global actor isolated or if the closure captures an
isolated parameter. We previously just always inferred
nonisolated(nonsending). Unfortunately since we do not yet have capture
information in CSApply, this required us to put the isolation change into
TypeCheckConcurrency.cpp and basically have function conversions of the form:

```
(function_conversion_expr type="nonisolated(nonsending) () async -> Void"
   (closure_expr type="() async -> ()" isolated_to_caller_isolation))
```

Notice how we have a function conversion to nonisolated(nonsending) from a
closure expr that has an isolation that is isolated_to_caller.

2. With this in hand, we found that this pattern caused us to first thunk a
nonisolated(nonsending) function to an @Concurrent function and then thunk that
back to nonisolated(nonsending), causing the final function to always be
concurrent. I put into SILGen a peephole that recognizes this pattern and emits
the correct code.

3. With that in hand, we found that we were emitting nonisolated(nonsending)
parameters for inheritActorContext functions. This was then fixed by @xedin in

With all this in hand, closure literal isolation and all of the other RBI tests
with nonisolated(nonsending) enabled pass.

rdar://154969621

Author: gottesmm

lib/SILGen/SILGenExpr.cpp

@@ -497,6 +497,10 @@ namespace {
     RValue
     emitFunctionCvtFromExecutionCallerToGlobalActor(FunctionConversionExpr *E,
                                                     SGFContext C);
+
+    RValue emitFunctionCvtForNonisolatedNonsendingClosureExpr(
+        FunctionConversionExpr *E, SGFContext C);
+
     RValue visitActorIsolationErasureExpr(ActorIsolationErasureExpr *E,
                                           SGFContext C);
     RValue visitExtractFunctionIsolationExpr(ExtractFunctionIsolationExpr *E,
@@ -2031,6 +2035,44 @@ RValueEmitter::emitFunctionCvtToExecutionCaller(FunctionConversionExpr *e,
   return RValue(SGF, e, destType, result);
 }
 
+RValue RValueEmitter::emitFunctionCvtForNonisolatedNonsendingClosureExpr(
+    FunctionConversionExpr *E, SGFContext C) {
+  // The specific AST pattern for this looks as follows:
+  //
+  //   (function_conversion_expr type="nonisolated(nonsending) () async -> Void"
+  //      (closure_expr type="() async -> ()" isolated_to_caller_isolation))
+  CanAnyFunctionType destType =
+      cast<FunctionType>(E->getType()->getCanonicalType());
+  auto subExpr = E->getSubExpr()->getSemanticsProvidingExpr();
+
+  // If we do not have a closure or if that closure is not caller isolation
+  // inheriting, bail.
+  auto *closureExpr = dyn_cast<ClosureExpr>(subExpr);
+  if (!closureExpr ||
+      !closureExpr->getActorIsolation().isCallerIsolationInheriting())
+    return RValue();
+
+  // Then grab our closure type... make sure it is non isolated and then make
+  // sure it is the same as our destType but with nonisolated.
+  CanAnyFunctionType closureType =
+      cast<FunctionType>(closureExpr->getType()->getCanonicalType());
+  if (!closureType->getIsolation().isNonIsolated() ||
+      closureType !=
+          destType->withIsolation(FunctionTypeIsolation::forNonIsolated())
+              ->getCanonicalType())
+    return RValue();
+
+  // NOTE: This is a partial inline of getClosureTypeInfo. We do this so we have
+  // more control and make this change less viral in the compiler for 6.2.
+  auto newExtInfo = closureType->getExtInfo().withIsolation(
+      FunctionTypeIsolation::forNonIsolatedCaller());
+  closureType = closureType.withExtInfo(newExtInfo);
+  auto info = SGF.getFunctionTypeInfo(closureType);
+
+  auto closure = emitClosureReference(closureExpr, info);
+  return RValue(SGF, closureExpr, destType, closure);
+}
+
 RValue RValueEmitter::emitFunctionCvtFromExecutionCallerToGlobalActor(
     FunctionConversionExpr *e, SGFContext C) {
   // We are pattern matching a conversion sequence like the following:
@@ -2142,6 +2184,28 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
   // TODO: Move this up when we can emit closures directly with C calling
   // convention.
   auto subExpr = e->getSubExpr()->getSemanticsProvidingExpr();
+
+  // Before we go any further into emitting the convert function expr, see if
+  // our SubExpr is a ClosureExpr with the exact same type as our
+  // FunctionConversionExpr except with the FunctionConversionExpr adding
+  // nonisolated(nonsending). Then see if the ClosureExpr itself (even though it
+  // is not nonisolated(nonsending) typed is considered to have
+  // nonisolated(nonsending) isolation. In such a case, emit the closure
+  // directly. We are going to handle it especially in closure emission to work
+  // around the missing information in the type.
+  //
+  // DISCUSSION: We need to do this here since in the Expression TypeChecker we
+  // do not have access to capture information when we would normally want to
+  // mark the closure type as being nonisolated(nonsending). As a result, we
+  // cannot know if the nonisolated(nonsending) should be overridden by for
+  // example an actor that is captured by the closure. So to work around this in
+  // Sema, we still mark the ClosureExpr as having the appropriate isolation
+  // even though its type does not have it... and then we work around this here
+  // and also in getClosureTypeInfo.
+  if (destType->getIsolation().isNonIsolatedCaller())
+    if (auto rv = emitFunctionCvtForNonisolatedNonsendingClosureExpr(e, C))
+      return rv;
+
   // Look through `as` type ascriptions that don't induce bridging too.
   while (auto subCoerce = dyn_cast<CoerceExpr>(subExpr)) {
     // Coercions that introduce bridging aren't simple type ascriptions.

lib/Sema/CSApply.cpp

@@ -7849,23 +7849,6 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
       }
     }
 
-    // If we have a ClosureExpr, then we can safely propagate the
-    // 'nonisolated(nonsending)' isolation if it's not explicitly
-    // marked as `@concurrent`.
-    if (toEI.getIsolation().isNonIsolatedCaller() &&
-        (fromEI.getIsolation().isNonIsolated() &&
-         !isClosureMarkedAsConcurrent(expr))) {
-      auto newFromFuncType = fromFunc->withIsolation(
-          FunctionTypeIsolation::forNonIsolatedCaller());
-      if (applyTypeToClosureExpr(cs, expr, newFromFuncType)) {
-        fromFunc = newFromFuncType->castTo<FunctionType>();
-        // Propagating 'nonisolated(nonsending)' might have satisfied the entire
-        // conversion. If so, we're done, otherwise keep converting.
-        if (fromFunc->isEqual(toType))
-          return expr;
-      }
-    }
-
     if (ctx.LangOpts.isDynamicActorIsolationCheckingEnabled()) {
       // Passing a synchronous global actor-isolated function value and
       // parameter that expects a synchronous non-isolated function type could

lib/Sema/TypeCheckConcurrency.cpp

@@ -4964,8 +4964,33 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation(
         closure->getParent(), getClosureActorIsolation);
     preconcurrency |= parentIsolation.preconcurrency();
 
-    return computeClosureIsolationFromParent(closure, parentIsolation,
-                                             checkIsolatedCapture);
+    auto normalIsolation = computeClosureIsolationFromParent(
+        closure, parentIsolation, checkIsolatedCapture);
+
+    // The solver has to be conservative and produce a conversion to
+    // `nonisolated(nonsending)` because at solution application time
+    // we don't yet know whether there are any captures which would
+    // make closure isolated.
+    //
+    // At this point we know that closure is not explicitly annotated with
+    // global actor, nonisolated/@concurrent attributes and doesn't have
+    // isolated parameters. If our closure is nonisolated and we have a
+    // conversion to nonisolated(nonsending), then we should respect that.
+    if (auto *explicitClosure = dyn_cast<ClosureExpr>(closure);
+        !normalIsolation.isGlobalActor()) {
+      if (auto *fce =
+              dyn_cast_or_null<FunctionConversionExpr>(Parent.getAsExpr())) {
+        auto expectedIsolation =
+            fce->getType()->castTo<FunctionType>()->getIsolation();
+        if (expectedIsolation.isNonIsolatedCaller()) {
+          auto captureInfo = explicitClosure->getCaptureInfo();
+          if (!captureInfo.getIsolatedParamCapture())
+            return ActorIsolation::forCallerIsolationInheriting();
+        }
+      }
+    }
+
+    return normalIsolation;
   }();
 
   // Apply computed preconcurrency.

test/Concurrency/attr_execution/attr_execution.swift

@@ -18,7 +18,7 @@ func callerTest() async {}
 struct Test {
   // CHECK-LABEL: // closure #1 in variable initialization expression of Test.x
   // CHECK: // Isolation: caller_isolation_inheriting
-  // CHECK: sil private [ossa] @$s14attr_execution4TestV1xyyYaYCcvpfiyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
+  // CHECK: sil private [ossa] @$s14attr_execution4TestV1xyyYaYCcvpfiyyYacfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
   var x: () async -> Void = {}
 
   // CHECK-LABEL: // Test.test()
@@ -67,15 +67,15 @@ func takesClosure(fn: () async -> Void) {
 }
 
 // CHECK-LABEL: sil hidden [ossa] @$s14attr_execution11testClosureyyF : $@convention(thin) () -> ()
-// CHECK:  [[CLOSURE:%.*]] = function_ref @$s14attr_execution11testClosureyyFyyYaYCXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
+// CHECK:  [[CLOSURE:%.*]] = function_ref @$s14attr_execution11testClosureyyFyyYaXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
 // CHECK:  [[THUNKED_CLOSURE:%.*]] = thin_to_thick_function %0 to $@noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
 // CHECK:  [[TAKES_CLOSURE:%.*]] = function_ref @$s14attr_execution12takesClosure2fnyyyYaYCXE_tF : $@convention(thin) (@guaranteed @noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()) -> ()
 // CHECK:  apply [[TAKES_CLOSURE]]([[THUNKED_CLOSURE]])
 // CHECK: } // end sil function '$s14attr_execution11testClosureyyF'
 
 // CHECK-LABEL: // closure #1 in testClosure()
 // CHECK: // Isolation: caller_isolation_inheriting
-// CHECK: sil private [ossa] @$s14attr_execution11testClosureyyFyyYaYCXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
+// CHECK: sil private [ossa] @$s14attr_execution11testClosureyyFyyYaXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
 func testClosure() {
   takesClosure {
   }

test/Concurrency/attr_execution/conversions_silgen.swift

@@ -421,7 +421,7 @@ func conversionsFromSyncToAsync(_ x: @escaping @Sendable (NonSendableKlass) -> V
 }
 
 func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) async -> Void) {
-  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYacfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
   // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
   // CHECK: hop_to_executor [[EXECUTOR]]
   let _: nonisolated(nonsending) () async -> Void = {
@@ -430,7 +430,7 @@ func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) asy
 
   func testParam(_: nonisolated(nonsending) () async throws -> Void) {}
 
-  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCXEfU0_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> @error any Error
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU0_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
   // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
   // CHECK: hop_to_executor [[EXECUTOR]]
   testParam { 42 }
@@ -440,7 +440,7 @@ func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) asy
   // CHECK: hop_to_executor [[GENERIC_EXECUTOR]]
   testParam { @concurrent in 42 }
 
-  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYaYCcfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> ()
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYacfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> () {
   // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>, %1 : $Int):
   // CHECK: hop_to_executor [[EXECUTOR]]
   fn = { _ in }

test/Concurrency/transfernonsendable_closureliterals_isolationinference.swift

@@ -1,14 +1,12 @@
-// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 5 -strict-concurrency=complete %s -o - | %FileCheck %s
+// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 5 -strict-concurrency=complete %s -o - 2>/dev/null | %FileCheck %s
 // RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 6 -verify %s -o /dev/null -verify-additional-prefix ni-
-// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 5 -strict-concurrency=complete %s -o - -enable-upcoming-feature NonisolatedNonsendingByDefault | %FileCheck %s
+// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 5 -strict-concurrency=complete %s -o - -enable-upcoming-feature NonisolatedNonsendingByDefault 2>/dev/null | %FileCheck %s
 // RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 6 -verify %s -o /dev/null -enable-upcoming-feature NonisolatedNonsendingByDefault -verify-additional-prefix ni-ns-
 
 // REQUIRES: concurrency
 // REQUIRES: asserts
 // REQUIRES: swift_feature_NonisolatedNonsendingByDefault
 
-// REQUIRES: rdar154969621
-
 // This test validates the behavior of transfernonsendable around
 // closure literals