[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
This commit is contained in:
Michael Gottesman
2025-12-11 18:33:02 -08:00
parent 20bee09dc9
commit d64fda488e
2 changed files with 60 additions and 0 deletions

View File

@@ -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;