[region-isolation] Treat sending indirect_results as disconnected even if it is a return value of an actor isolated function.

rdar://134623227

Author: gottesmm

include/swift/SIL/ApplySite.h

@@ -534,6 +534,10 @@ class ApplySite {
   /// result argument to the apply site.
   bool isIndirectResultOperand(const Operand &op) const;
 
+  /// Returns true if \p op is an operand that is passed as an indirect error
+  /// result.
+  bool isIndirectErrorResultOperand(const Operand &op) const;
+
   ApplyOptions getApplyOptions() const {
     switch (ApplySiteKind(getInstruction()->getKind())) {
     case ApplySiteKind::ApplyInst:
@@ -597,6 +601,14 @@ class ApplySite {
     return getSubstCalleeConv().getParamInfoForSILArg(calleeArgIndex);
   }
 
+  bool isSending(const Operand &oper) const {
+    if (isIndirectErrorResultOperand(oper))
+      return false;
+    if (isIndirectResultOperand(oper))
+      return getSubstCalleeType()->hasSendingResult();
+    return getArgumentParameterInfo(oper).hasOption(SILParameterInfo::Sending);
+  }
+
   static ApplySite getFromOpaqueValue(void *p) { return ApplySite(p); }
 
   friend bool operator==(ApplySite lhs, ApplySite rhs) {
@@ -955,6 +967,13 @@ inline bool ApplySite::isIndirectResultOperand(const Operand &op) const {
   return fas.isIndirectResultOperand(op);
 }
 
+inline bool ApplySite::isIndirectErrorResultOperand(const Operand &op) const {
+  auto fas = asFullApplySite();
+  if (!fas)
+    return false;
+  return fas.isIndirectErrorResultOperand(op);
+}
+
 } // namespace swift
 
 #endif

include/swift/SIL/SILArgument.h

@@ -424,9 +424,7 @@ class SILFunctionArgument : public SILArgument {
     sharedUInt32().SILFunctionArgument.lifetimeAnnotation = newValue;
   }
 
-  bool isSending() const {
-    return getKnownParameterInfo().hasOption(SILParameterInfo::Sending);
-  }
+  bool isSending() const;
 
   Lifetime getLifetime() const {
     return getType()

lib/SIL/IR/SILArgument.cpp

@@ -433,3 +433,11 @@ bool SILFunctionArgument::isSelf() const {
   return getFunction()->hasSelfParam() &&
          getParent()->getArguments().back() == this;
 }
+
+bool SILFunctionArgument::isSending() const {
+  if (isIndirectErrorResult())
+    return false;
+  if (isIndirectResult())
+    return getFunction()->getLoweredFunctionType()->hasSendingResult();
+  return getKnownParameterInfo().hasOption(SILParameterInfo::Sending);
+}

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -1976,6 +1976,7 @@ class PartitionOpTranslator {
     // For non-self parameters, gather all of the transferring parameters and
     // gather our non-transferring parameters.
     SmallVector<Operand *, 8> nonTransferringParameters;
+    SmallVector<Operand *, 8> sendingIndirectResults;
     if (fas.getNumArguments()) {
       // NOTE: We want to process indirect parameters as if they are
       // parameters... so we process them in nonTransferringParameters.
@@ -1984,12 +1985,16 @@ class PartitionOpTranslator {
         if (fas.isCalleeOperand(op))
           continue;
 
-        if (!fas.getArgumentConvention(op).isIndirectOutParameter() &&
-            fas.getArgumentParameterInfo(op).hasOption(
-                SILParameterInfo::Sending)) {
+        if (fas.isSending(op)) {
+          if (fas.isIndirectResultOperand(op)) {
+            sendingIndirectResults.push_back(&op);
+            continue;
+          }
+
           if (auto value = tryToTrackValue(op.get())) {
             builder.addRequire(value->getRepresentative().getValue());
             builder.addTransfer(value->getRepresentative().getValue(), &op);
+            continue;
           }
         } else {
           nonTransferringParameters.push_back(&op);
@@ -2041,11 +2046,20 @@ class PartitionOpTranslator {
     // override isolation, then perform assign fresh.
     ArrayRef<SILValue> empty;
     translateSILMultiAssign(empty, nonTransferringParameters, {});
+
+    // Sending direct results.
     for (SILValue result : applyResults) {
       if (auto value = tryToTrackValue(result)) {
         builder.addAssignFresh(value->getRepresentative().getValue());
       }
     }
+
+    // Sending indirect results.
+    for (Operand *op : sendingIndirectResults) {
+      if (auto value = tryToTrackValue(op->get())) {
+        builder.addAssignFresh(value->getRepresentative().getValue());
+      }
+    }
   }
 
   void translateSILApply(SILInstruction *inst) {
@@ -2073,8 +2087,9 @@ class PartitionOpTranslator {
       return translateIsolationCrossingSILApply(cast);
     if (auto cast = dyn_cast<BeginApplyInst>(inst))
       return translateIsolationCrossingSILApply(cast);
-    if (auto cast = dyn_cast<TryApplyInst>(inst))
+    if (auto cast = dyn_cast<TryApplyInst>(inst)) {
       return translateIsolationCrossingSILApply(cast);
+    }
 
     llvm_unreachable("Only ApplyInst, BeginApplyInst, and TryApplyInst should "
                      "cross isolation domains");

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -832,11 +832,19 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
 
   auto *fArg = cast<SILFunctionArgument>(arg);
 
-  // Transferring is always disconnected.
+  // Sending is always disconnected.
+  if (fArg->isSending())
+    return SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
+
+  // If we have a closure capture that is not an indirect result or indirect
+  // result error, we want to treat it as sending so that we properly handle
+  // async lets.
+  //
+  // This pattern should only come up with async lets. See comment in
+  // isTransferrableFunctionArgument.
   if (!fArg->isIndirectResult() && !fArg->isIndirectErrorResult() &&
-      ((fArg->isClosureCapture() &&
-        fArg->getFunction()->getLoweredFunctionType()->isSendable()) ||
-       fArg->isSending()))
+      fArg->isClosureCapture() &&
+      fArg->getFunction()->getLoweredFunctionType()->isSendable())
     return SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
 
   // Before we do anything further, see if we have an isolated parameter. This

test/Concurrency/sending_continuation.swift

@@ -60,9 +60,10 @@ func withCheckedContinuation_3() async {
     // expected-note @-2 {{'x' used after being passed as a 'sending' parameter}}
     useValue(x) // expected-note {{access can happen concurrently}}
   }
+
+  // Since x is returned as sending from withCheckedContinuation, we can send it
+  // further.
   await useValueAsync(x)
-  // expected-error @-1 {{sending 'x' risks causing data races}}
-  // expected-note @-2 {{sending main actor-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
 }
 
 func withCheckedContinuation_3a() async {
@@ -88,9 +89,10 @@ func withCheckedContinuation_4() async {
     // expected-note @-2 {{main actor-isolated 'y' is passed as a 'sending' parameter}}
     useValue(y)
   }
+
+  // Since withCheckedContinuation returns value as sending, we can call
+  // useValueAsync since it is disconnected.
   await useValueAsync(x)
-  // expected-error @-1 {{sending 'x' risks causing data races}}
-  // expected-note @-2 {{sending main actor-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
 }
 
 func withCheckedContinuation_4a() async {
@@ -189,9 +191,10 @@ func withUnsafeContinuation_3() async {
     // expected-note @-2 {{'x' used after being passed as a 'sending' parameter}}
     useValue(x) // expected-note {{access can happen concurrently}}
   }
+
+  // withUnsafeContinuation returns x as sending, so we can pass it to a
+  // nonisolated function.
   await useValueAsync(x)
-  // expected-error @-1 {{sending 'x' risks causing data races}}
-  // expected-note @-2 {{sending main actor-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
 }
 
 func withUnsafeContinuation_3a() async {
@@ -215,9 +218,10 @@ func withUnsafeContinuation_4() async {
     // expected-note @-2 {{main actor-isolated 'y' is passed as a 'sending' parameter}}
     useValue(y)
   }
+
+  // Since withUnsafeContinuation returns x as sending, we can use it in a
+  // nonisolated function.
   await useValueAsync(x)
-  // expected-error @-1 {{sending 'x' risks causing data races}}
-  // expected-note @-2 {{sending main actor-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
 }
 
 func withUnsafeContinuation_4a() async {

test/Concurrency/silisolationinfo_inference.sil

@@ -819,3 +819,60 @@ bb0(%0 : @guaranteed $MyActor):
   %9999 = tuple ()
   return %9999 : $()
 }
+
+// CHECK-LABEL: begin running test 1 of 1 on sending_indirect_result_isnt_actor_isolated: sil_isolation_info_inference with: @trace[0]
+// CHECK-NEXT: Input Value: %0 = argument of bb0 : $*Optional<τ_0_0>          // user: %2
+// CHECK-NEXT: Isolation: disconnected
+// CHECK-NEXT: end running test 1 of 1 on sending_indirect_result_isnt_actor_isolated: sil_isolation_info_inference with: @trace[0]
+sil [ossa] @sending_indirect_result_isnt_actor_isolated : $@convention(thin) @async <τ_0_0> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out Optional<τ_0_0>) {
+bb0(%0 : $*Optional<τ_0_0>, %1 : @guaranteed $MyActor):
+  specify_test "sil_isolation_info_inference @trace[0]"
+  debug_value [trace] %0 : $*Optional<τ_0_0>
+  inject_enum_addr %0 : $*Optional<τ_0_0>, #Optional.none!enumelt
+  %9999 = tuple ()
+  return %9999 : $()
+}
+
+sil @myactor_sending_result_sync : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> @sil_sending @out τ_0_0
+
+// CHECK: begin running test 1 of 1 on sending_indirect_result_isnt_actor_isolated_apply: sil_isolation_info_inference with: @trace[0]
+// CHECK-NEXT: Input Value:   %3 = alloc_stack $T                             // users: %6, %5, %4
+// CHECK-NEXT: Isolation: disconnected
+// CHECK-NEXT: end running test 1 of 1 on sending_indirect_result_isnt_actor_isolated_apply: sil_isolation_info_inference with: @trace[0]
+sil [ossa] @sending_indirect_result_isnt_actor_isolated_apply : $@convention(method) @async <T> (@sil_isolated @guaranteed MyActor) -> @sil_sending @out T {
+bb0(%0 : $*T, %1 : @guaranteed $MyActor):
+  specify_test "sil_isolation_info_inference @trace[0]"
+  %4 = function_ref @myactor_sending_result_sync : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> @sil_sending @out τ_0_0
+  %2 = alloc_stack $T
+  debug_value [trace] %2 : $*T
+  %5 = apply %4<T>(%2, %1) : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> @sil_sending @out τ_0_0
+  copy_addr [take] %2 to [init] %0 : $*T
+  dealloc_stack %2 : $*T
+  %6 = tuple ()
+  return %6 : $()
+}
+
+sil @myactor_sending_result_sync_throws : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out τ_0_0, @error any Error)
+
+// CHECK-LABEL: begin running test 1 of 1 on sending_indirect_result_isnt_actor_isolated_try_apply: sil_isolation_info_inference with: @trace[0]
+// CHECK-NEXT: Input Value:   %3 = alloc_stack $T
+// CHECK-NEXT: Isolation: disconnected
+// CHECK-NEXT: end running test 1 of 1 on sending_indirect_result_isnt_actor_isolated_try_apply: sil_isolation_info_inference with: @trace[0]
+sil [ossa] @sending_indirect_result_isnt_actor_isolated_try_apply : $@convention(method) @async <T> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out T, @error any Error) {
+bb0(%0 : $*T, %1 : @guaranteed $MyActor):
+  specify_test "sil_isolation_info_inference @trace[0]"
+  %5 = function_ref @myactor_sending_result_sync_throws : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out τ_0_0, @error any Error)
+  %2 = alloc_stack $*T
+  try_apply %5<T>(%2, %1) : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out τ_0_0, @error any Error), normal bb1, error bb2
+
+bb1(%7 : $()):
+  copy_addr [take] %2 to [init] %0 : $*T
+  debug_value [trace] %2 : $*T
+  dealloc_stack %2 : $*T
+  %8 = tuple ()
+  return %8 : $()
+
+bb2(%10 : @owned $any Error):
+  dealloc_stack %2 : $*T
+  throw %10 : $any Error
+}

test/Concurrency/transfernonsendable_sending_results.swift

@@ -90,6 +90,10 @@ class GenericNonSendableKlass<T> {
 
 func sendParameter<T>(_ t: sending T) {}
 
+actor MyActor {
+  private var ns = NonSendableKlass()
+}
+
 /////////////////
 // MARK: Tests //
 /////////////////
@@ -295,3 +299,19 @@ func indirectSendingOptionalClassField<T>(_ t: GenericNonSendableKlass<T>) -> se
   // expected-note @-1 {{returning a task-isolated 'Optional<T>' value risks causing races since the caller assumes the value can be safely sent to other isolation domains}}
   // expected-note @-2 {{'Optional<T>' is a non-Sendable type}}
 }
+
+func useBlock<T>(block: () throws -> T) throws -> sending T {
+  fatalError()
+}
+
+extension MyActor {
+  // This shouldn't emit any errors. We used to error on returning result.
+  public func withContext<T>(_ block: sending () throws -> T) async throws -> sending T {
+    let value: T = try useBlock {
+      _ = ns
+      return try block()
+    }
+
+    return value
+  }
+}