Merge pull request #71940 from gottesmm/layer-of-onion

[region-isolation] Transferring results shouldn't have the following error emitted: "non-sendable type 'NonSendableKlass' returned by implicitly asynchronous call to main actor-isolated function cannot cross actor boundary"

Author: gottesmm

lib/AST/ASTDumper.cpp

@@ -4309,6 +4309,7 @@ namespace {
         printFlag(T->isSendable(), "Sendable");
         printFlag(T->isAsync(), "async");
         printFlag(T->isThrowing(), "throws");
+        printFlag(T->hasTransferringResult(), "transferring_result");
       }
       if (Type globalActor = T->getGlobalActor()) {
         printFieldQuoted(globalActor.getString(), "global_actor");

lib/AST/ASTPrinter.cpp

@@ -3809,7 +3809,20 @@ static bool usesFeatureTransferringArgsAndResults(Decl *decl) {
     if (pd->isTransferring())
       return true;
 
-  // TODO: Results.
+  if (auto *fDecl = dyn_cast<FuncDecl>(decl)) {
+    auto fnTy = fDecl->getInterfaceType();
+    bool hasTransferring = false;
+    if (auto *ft = llvm::dyn_cast_if_present<FunctionType>(fnTy)) {
+      if (ft->hasExtInfo())
+        hasTransferring = ft->hasTransferringResult();
+    } else if (auto *ft =
+               llvm::dyn_cast_if_present<GenericFunctionType>(fnTy)) {
+      if (ft->hasExtInfo())
+        hasTransferring = ft->hasTransferringResult();
+    }
+    if (hasTransferring)
+      return true;
+  }
 
   return false;
 }
@@ -7432,7 +7445,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
     Printer << " -> ";
 
     if (T->hasExtInfo() && T->hasTransferringResult()) {
-      Printer.printKeyword("transferring", Options);
+      Printer.printKeyword("transferring ", Options);
     }
 
     if (T->hasLifetimeDependenceInfo()) {

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -417,13 +417,19 @@ static bool isProjectedFromAggregate(SILValue value) {
   return visitor.isMerge;
 }
 
-static PartialApplyInst *findAsyncLetPartialApplyFromStart(BuiltinInst *bi) {
+namespace {
+using AsyncLetSourceValue =
+    llvm::PointerUnion<PartialApplyInst *, ThinToThickFunctionInst *>;
+} // namespace
+
+static std::optional<AsyncLetSourceValue>
+findAsyncLetPartialApplyFromStart(BuiltinInst *bi) {
   // If our operand is Sendable then we want to return nullptr. We only want to
   // return a value if we are not
   SILValue value = bi->getOperand(1);
   auto fType = value->getType().castTo<SILFunctionType>();
   if (fType->isSendable())
-    return nullptr;
+    return {};
 
   SILValue temp = value;
   while (true) {
@@ -436,10 +442,15 @@ static PartialApplyInst *findAsyncLetPartialApplyFromStart(BuiltinInst *bi) {
     value = temp;
   }
 
-  return cast<PartialApplyInst>(value);
+  // We can also get a thin_to_thick_function here if we do not capture
+  // anything. In such a case, we just do not process the partial apply get
+  if (auto *pai = dyn_cast<PartialApplyInst>(value))
+    return {{pai}};
+  return {{cast<ThinToThickFunctionInst>(value)}};
 }
 
-static PartialApplyInst *findAsyncLetPartialApplyFromGet(ApplyInst *ai) {
+static std::optional<AsyncLetSourceValue>
+findAsyncLetPartialApplyFromGet(ApplyInst *ai) {
   auto *bi = cast<BuiltinInst>(FullApplySite(ai).getArgument(0));
   assert(*bi->getBuiltinKind() ==
          BuiltinValueKind::StartAsyncLetWithLocalBuffer);
@@ -1552,11 +1563,18 @@ class PartitionOpTranslator {
   }
 
   void translateAsyncLetGet(ApplyInst *ai) {
-    auto *pai = findAsyncLetPartialApplyFromGet(ai);
+    auto source = findAsyncLetPartialApplyFromGet(ai);
+    assert(source.has_value());
+
+    // If we didn't find a partial_apply, then we must have had a
+    // thin_to_thick_function meaning we did not capture anything.
+    if (source->is<ThinToThickFunctionInst *>())
+      return;
 
     // We should always be able to derive a partial_apply since we pattern
     // matched against the actual function call to swift_asyncLet_get in our
     // caller.
+    auto *pai = source->get<PartialApplyInst *>();
     assert(pai && "AsyncLet Get should always have a derivable partial_apply");
 
     ApplySite applySite(pai);
@@ -2965,6 +2983,11 @@ static bool canComputeRegionsForFunction(SILFunction *fn) {
   if (!fn->getASTContext().LangOpts.hasFeature(Feature::RegionBasedIsolation))
     return false;
 
+  assert(fn->getASTContext().LangOpts.StrictConcurrencyLevel ==
+             StrictConcurrency::Complete &&
+         "Need strict concurrency to be enabled for RegionBasedIsolation to be "
+         "enabled as well");
+
   // If this function does not correspond to a syntactic declContext and it
   // doesn't have a parent module, don't check it since we cannot check if a
   // type is sendable.

lib/Sema/TypeCheckConcurrency.cpp

@@ -3522,8 +3522,11 @@ namespace {
         diagnoseApplyArgSendability(apply, getDeclContext());
       }
 
-      // Check for sendability of the result type.
-      if (diagnoseNonSendableTypes(
+      // Check for sendability of the result type if we do not have a
+      // transferring result.
+      if ((!ctx.LangOpts.hasFeature(Feature::TransferringArgsAndResults) ||
+           !fnType->hasTransferringResult()) &&
+          diagnoseNonSendableTypes(
              fnType->getResult(), getDeclContext(),
              /*inDerivedConformance*/Type(),
              apply->getLoc(),

test/Concurrency/transfernonsendable.sil

@@ -74,6 +74,9 @@ case none
 case some(T)
 }
 
+sil @swift_asyncLet_get : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
+sil @swift_asyncLet_finish : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
+
 /////////////////
 // MARK: Tests //
 /////////////////
@@ -144,4 +147,31 @@ bb0:
 
   %9999 = tuple ()
   return %9999 : $()
+}
+
+sil @implicitClosure : $@convention(thin) @Sendable @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>
+
+sil [ossa] @asyncLetWithThinToThickFunction : $@convention(thin) @async () -> () {
+bb0:
+  %0 = enum $Optional<Builtin.Executor>, #Optional.none!enumelt
+  hop_to_executor %0 : $Optional<Builtin.Executor>
+  %2 = alloc_stack $NonSendableKlass
+  %3 = address_to_pointer %2 : $*NonSendableKlass to $Builtin.RawPointer
+  %4 = enum $Optional<Builtin.RawPointer>, #Optional.none!enumelt
+  %5 = function_ref @implicitClosure : $@convention(thin) @Sendable @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>
+  %6 = convert_function %5 : $@convention(thin) @Sendable @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass> to $@convention(thin) @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>
+  %7 = thin_to_thick_function %6 : $@convention(thin) @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass> to $@noescape @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>
+  %8 = builtin "startAsyncLetWithLocalBuffer"<NonSendableKlass>(%4 : $Optional<Builtin.RawPointer>, %7 : $@noescape @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>, %3 : $Builtin.RawPointer) : $Builtin.RawPointer
+  %9 = function_ref @swift_asyncLet_get : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
+  %10 = apply %9(%8, %3) : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
+  hop_to_executor %0 : $Optional<Builtin.Executor>
+  %12 = load [copy] %2 : $*NonSendableKlass
+  destroy_value %12 : $NonSendableKlass
+  %14 = function_ref @swift_asyncLet_finish : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
+  %15 = apply %14(%8, %3) : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
+  hop_to_executor %0 : $Optional<Builtin.Executor>
+  %17 = builtin "endAsyncLetLifetime"(%8 : $Builtin.RawPointer) : $()
+  dealloc_stack %2 : $*NonSendableKlass
+  %19 = tuple ()
+  return %19 : $()
 }

test/Concurrency/transfernonsendable_asynclet.swift

@@ -9,7 +9,8 @@
 ////////////////////////
 
 /// Classes are always non-sendable, so this is non-sendable
-class NonSendableKlass { // expected-complete-note 92{{}}
+class NonSendableKlass { // expected-complete-note 94{{}}
+  // expected-tns-note @-1 {{}}
   var field: NonSendableKlass? = nil
   var field2: NonSendableKlass? = nil
 
@@ -38,6 +39,7 @@ func useInOut<T>(_ x: inout T) {}
 func useValue<T>(_ x: T) -> T { x }
 func useValueWrapInOptional<T>(_ x: T) -> T? { x }
 
+@MainActor func returnValueFromMain<T>() async -> T { fatalError() }
 @MainActor func transferToMain<T>(_ t: T) async {}
 @MainActor func transferToMainInt<T>(_ t: T) async -> Int { 5 }
 @MainActor func transferToMainIntOpt<T>(_ t: T) async -> Int? { 5 }
@@ -735,3 +737,15 @@ func asyncLet_Let_NormalUse_Simple2() async {
   let _ = await y
   useValue(x)
 }
+
+func asyncLetWithoutCapture() async {
+  // Make sure that we setup y correctly as fresh.
+  //
+  // NOTE: Error below will go away in next commit.
+  async let x: NonSendableKlass = await returnValueFromMain()
+  // expected-warning @-1 {{non-sendable type 'NonSendableKlass' returned by implicitly asynchronous call to main actor-isolated function cannot cross actor boundary}}
+  let y = await x
+  await transferToMain(y) // expected-tns-warning {{transferring value of non-Sendable type 'NonSendableKlass' from nonisolated context to main actor-isolated context}}
+  // expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
+  useValue(y) // expected-tns-note {{access here could race}}
+}

test/Concurrency/transfernonsendable_strong_transferring_params.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -emit-sil -disable-availability-checking -enable-experimental-feature TransferringArgsAndResults -verify -enable-experimental-feature RegionBasedIsolation %s -o /dev/null
+// RUN: %target-swift-frontend -emit-sil -parse-as-library -disable-availability-checking -strict-concurrency=complete -enable-experimental-feature TransferringArgsAndResults -verify -enable-experimental-feature RegionBasedIsolation %s -o /dev/null
 
 // REQUIRES: asserts
 
@@ -174,8 +174,7 @@ actor MyActor {
 }
 
 @MainActor func canAssignTransferringIntoGlobalActor3(_ x: transferring Klass) async {
-  await transferToCustom(globalKlass) // expected-warning {{transferring 'globalKlass' may cause a race}}
-  // expected-note @-1 {{transferring main actor-isolated 'globalKlass' to global actor 'CustomActor'-isolated callee could cause races between global actor 'CustomActor'-isolated and main actor-isolated uses}}
+  await transferToCustom(globalKlass) // expected-warning {{task isolated value of type 'Klass' transferred to global actor 'CustomActor'-isolated context}}
 }
 
 func canTransferAssigningIntoLocal(_ x: transferring Klass) async {

test/Concurrency/transfernonsendable_strong_transferring_results.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -emit-sil -disable-experimental-parser-round-trip -disable-availability-checking -enable-experimental-feature TransferringArgsAndResults -verify -enable-experimental-feature RegionBasedIsolation %s -o /dev/null
+// RUN: %target-swift-frontend -emit-sil -parse-as-library -strict-concurrency=complete -disable-experimental-parser-round-trip -disable-availability-checking -enable-experimental-feature TransferringArgsAndResults -verify -enable-experimental-feature RegionBasedIsolation %s -o /dev/null
 
 // REQUIRES: concurrency
 // REQUIRES: asserts
@@ -49,6 +49,8 @@ func transferAsyncResultWithTransferringArg(_ x: transferring NonSendableKlass)
 func transferAsyncResultWithTransferringArg2(_ x: transferring NonSendableKlass, _ y: NonSendableKlass) async -> transferring NonSendableKlass { NonSendableKlass() }
 func transferAsyncResultWithTransferringArg2Throwing(_ x: transferring NonSendableKlass, _ y: NonSendableKlass) async throws -> transferring NonSendableKlass { NonSendableKlass() }
 
+@MainActor func transferAsyncResultMainActor() async -> transferring NonSendableKlass { NonSendableKlass() }
+
 @MainActor var globalNonSendableKlass = NonSendableKlass()
 
 /////////////////
@@ -106,3 +108,11 @@ func transferReturnArg(_ x: NonSendableKlass) -> transferring NonSendableKlass {
 func transferReturnArgTuple(_ x: transferring NonSendableKlass) -> transferring (NonSendableKlass, NonSendableKlass) {
   return (x, x)
 }
+
+func useTransferredResultMainActor() async {
+  let _ = await transferAsyncResultMainActor()
+}
+
+func useTransferredResult() async {
+  let _ = await transferAsyncResult()
+}

test/Parse/transferring.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -disable-experimental-parser-round-trip -disable-availability-checking -enable-experimental-feature TransferringArgsAndResults
+// RUN: %target-typecheck-verify-swift -disable-experimental-parser-round-trip -disable-availability-checking -enable-experimental-feature TransferringArgsAndResults -strict-concurrency=complete -enable-experimental-feature RegionBasedIsolation
 
 // REQUIRES: asserts
 

test/Serialization/transferring.swift

@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t)
-// RUN: %target-swift-frontend -enable-experimental-feature RegionBasedIsolation -enable-experimental-feature TransferringArgsAndResults -disable-experimental-parser-round-trip -module-name transferring_test -emit-module -o %t/transferring_test.swiftmodule %S/Inputs/transferring.swift
-// RUN: %target-swift-frontend -enable-experimental-feature RegionBasedIsolation -enable-experimental-feature TransferringArgsAndResults -disable-experimental-parser-round-trip -emit-sil -I %t %s | %FileCheck %s
-// RUN: %target-sil-opt -enable-experimental-feature RegionBasedIsolation -enable-experimental-feature TransferringArgsAndResults %t/transferring_test.swiftmodule | %FileCheck -check-prefix=AST %s
+// RUN: %target-swift-frontend -strict-concurrency=complete -enable-experimental-feature RegionBasedIsolation -enable-experimental-feature TransferringArgsAndResults -disable-experimental-parser-round-trip -module-name transferring_test -emit-module -o %t/transferring_test.swiftmodule %S/Inputs/transferring.swift
+// RUN: %target-swift-frontend -strict-concurrency=complete -enable-experimental-feature RegionBasedIsolation -enable-experimental-feature TransferringArgsAndResults -disable-experimental-parser-round-trip -emit-sil -I %t %s | %FileCheck %s
+// RUN: %target-sil-opt -strict-concurrency=complete -enable-experimental-feature RegionBasedIsolation -enable-experimental-feature TransferringArgsAndResults %t/transferring_test.swiftmodule | %FileCheck -check-prefix=AST %s
 
 // REQUIRES: concurrency
 // REQUIRES: asserts