From d64fda488e13b1b63bb072bdc54a730aa14df2db Mon Sep 17 00:00:00 2001
From: Michael Gottesman <mgottesman@apple.com>
Date: Thu, 11 Dec 2025 18:33:02 -0800
Subject: [PATCH] [rbi] When looking for closure uses, handle
 unresolved_non_copyable_value and store_borrow temporaries.

This ensures that in cases like the following:

+func testNoncopyableNonsendableStructWithNonescapingMainActorAsync() {
+  let x = NoncopyableStructNonsendable()    <=========
+  let _ = {
+    nonescapingAsyncClosure { @MainActor in
+      useValueNoncopyable(x) // expected-warning {{sending 'x' risks causing data races}}
+      // expected-note @-1 {{task-isolated 'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
+    }
+  }
+}

We emit the diagnostic on the use instead of the <=====.

rdar://166347485
---
 .../Mandatory/SendNonSendable.cpp             | 43 +++++++++++++++++++
 test/Concurrency/transfernonsendable.swift    | 17 ++++++++
 2 files changed, 60 insertions(+)

diff --git a/lib/SILOptimizer/Mandatory/SendNonSendable.cpp b/lib/SILOptimizer/Mandatory/SendNonSendable.cpp
index 5a30a3a44dd4b..eb6a950f79e7e 100644
--- a/lib/SILOptimizer/Mandatory/SendNonSendable.cpp
+++ b/lib/SILOptimizer/Mandatory/SendNonSendable.cpp
@@ -217,6 +217,32 @@ inferNameAndRootHelper(SILValue value) {
   return VariableNameInferrer::inferNameAndRoot(value);
 }
 
+/// Sometimes we use a store_borrow + temporary to materialize a borrowed value
+/// to be passed to another function. We want to emit the error on the function
+/// itself, not the store_borrow so we get the best location. We only do this if
+/// we can prove that we have a store_borrow to an alloc_stack, the store_borrow
+/// is the only thing stored into the alloc_stack, and except for the
+/// store_borrow, the alloc_stack has a single non_destroy user, a function we
+/// are calling.
+static Operand *
+findClosureUseThroughStoreBorrowTemporary(StoreBorrowInst *sbi) {
+  auto *asi = dyn_cast<AllocStackInst>(sbi->getDest());
+  if (!asi)
+    return {};
+  Operand *resultUse = nullptr;
+  for (auto *use : asi->getUses()) {
+    if (use == &sbi->getAllOperands()[StoreBorrowInst::Dest])
+      continue;
+    if (isa<EndBorrowInst>(use->getUser()))
+      continue;
+    auto fas = FullApplySite::isa(use->getUser());
+    if (!fas)
+      return {};
+    resultUse = use;
+  }
+  return resultUse;
+}
+
 /// Find a use corresponding to the potentially recursive capture of \p
 /// initialOperand that would be appropriate for diagnostics.
 ///
@@ -261,9 +287,15 @@ findClosureUse(Operand *initialOperand) {
     if (isIncidentalUse(user) && !isa<IgnoredUseInst>(user))
       continue;
 
+    // Ignore some instructions we do not care about.
+    if (isa<DestroyValueInst, EndBorrowInst, EndAccessInst, DeallocStackInst>(
+            user))
+      continue;
+
     // Look through some insts we do not care about.
     if (isa<CopyValueInst, BeginBorrowInst, ProjectBoxInst, BeginAccessInst>(
             user) ||
+        isa<MarkUnresolvedNonCopyableValueInst>(user) ||
         isMoveOnlyWrapperUse(user) ||
         // We want to treat move_value [var_decl] as a real use since we are
         // assigning to a var.
@@ -308,6 +340,17 @@ findClosureUse(Operand *initialOperand) {
       }
     }
 
+    // If we have a store_borrow, see if we are using it to just marshal a use
+    // to a full apply site.
+    if (auto *sbi = dyn_cast<StoreBorrowInst>(op->getUser());
+        sbi && op == &sbi->getAllOperands()[StoreBorrowInst::Src]) {
+      if (auto *use = findClosureUseThroughStoreBorrowTemporary(sbi)) {
+        if (visitedOperand.insert(use).second)
+          worklist.emplace_back(use, fArg);
+        continue;
+      }
+    }
+
     // Otherwise, we have a real use. Return it and the function argument that
     // it was derived from.
     return pair;
diff --git a/test/Concurrency/transfernonsendable.swift b/test/Concurrency/transfernonsendable.swift
index 458ec91077f8d..1f262720c044f 100644
--- a/test/Concurrency/transfernonsendable.swift
+++ b/test/Concurrency/transfernonsendable.swift
@@ -65,6 +65,7 @@ final class FinalMainActorIsolatedKlass {
 func useInOut<T>(_ x: inout T) {}
 func useValue<T>(_ x: T) {}
 func useValueAsync<T>(_ x: T) async {}
+func useValueNoncopyable<T : ~Copyable>(_ x: borrowing T) {}
 @concurrent func useValueAsyncConcurrent<T>(_ x: T) async {}
 
 @MainActor func transferToMain<T>(_ t: T) async {}
@@ -90,12 +91,18 @@ struct SendableGenericStruct : Sendable {
   var x = SendableKlass()
 }
 
+struct NoncopyableStructNonsendable : ~Copyable {
+  var x = NonSendableKlass()
+}
+
 enum MyEnum<T> {
     case none
     indirect case some(NonSendableKlass)
     case more(T)
 }
 
+func nonescapingAsyncClosure(_ x: () async -> ()) {}
+
 ////////////////////////////
 // MARK: Actor Self Tests //
 ////////////////////////////
@@ -501,6 +508,16 @@ extension MyActor {
   }
 }
 
+func testNoncopyableNonsendableStructWithNonescapingMainActorAsync() {
+  let x = NoncopyableStructNonsendable()
+  let _ = {
+    nonescapingAsyncClosure { @MainActor in
+      useValueNoncopyable(x) // expected-warning {{sending 'x' risks causing data races}}
+      // expected-note @-1 {{task-isolated 'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
+    }
+  }
+}
+
 ///////////////////////////////////
 // MARK: Sendable Function Tests //
 ///////////////////////////////////