Skip to content

[6.2] Revert nonisolated(unsafe) closure inference #83102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 17, 2025
Merged

[6.2] Revert nonisolated(unsafe) closure inference #83102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e06769
Revert "Fix an error message due to a difference in Task.detached fro…
gottesmm Jul 16, 2025
e81838f
Revert "[region-isolation] Add a test showing that we do not infer no…
gottesmm Jul 16, 2025
348720b
Revert "[rbi] Treat a partial_apply as nonisolated(unsafe) if all of …
gottesmm Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/SILOptimizer/Analysis/RegionAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2490,8 +2490,7 @@ class PartitionOpTranslator {
}
}

if (auto isolationRegionInfo = SILIsolationInfo::get(pai);
isolationRegionInfo && !isolationRegionInfo.isDisconnected()) {
if (auto isolationRegionInfo = SILIsolationInfo::get(pai)) {
return translateIsolatedPartialApply(pai, isolationRegionInfo);
}

Expand Down
89 changes: 4 additions & 85 deletions lib/SILOptimizer/Utils/SILIsolationInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,69 +363,6 @@ inferIsolationInfoForTempAllocStack(AllocStackInst *asi) {
return SILIsolationInfo::get(targetOperand->getUser());
}

static SILValue lookThroughNonVarDeclOwnershipInsts(SILValue v) {
while (true) {
if (auto *svi = dyn_cast<SingleValueInstruction>(v)) {
if (isa<CopyValueInst>(svi)) {
v = svi->getOperand(0);
continue;
}

if (auto *bbi = dyn_cast<BeginBorrowInst>(v)) {
if (!bbi->isFromVarDecl()) {
v = bbi->getOperand();
continue;
}
return v;
}

if (auto *mvi = dyn_cast<MoveValueInst>(v)) {
if (!mvi->isFromVarDecl()) {
v = mvi->getOperand();
continue;
}

return v;
}
}

return v;
}
}

/// See if \p pai has at least one nonisolated(unsafe) capture and that all
/// captures are either nonisolated(unsafe) or sendable.
static bool isPartialApplyNonisolatedUnsafe(PartialApplyInst *pai) {
bool foundOneNonIsolatedUnsafe = false;
for (auto &op : pai->getArgumentOperands()) {
if (SILIsolationInfo::isSendableType(op.get()))
continue;

// Normally we would not look through copy_value, begin_borrow, or
// move_value since this is meant to find the inherent isolation of a
// specific element. But since we are checking for captures rather
// than the actual value itself (just for unsafe nonisolated
// purposes), it is ok.
//
// E.x.: As an example of something we want to prevent, consider an
// invocation of a nonisolated function that is a parameter to an
// @MainActor function. That means from a region isolation
// perspective, the function parameter is in the MainActor region, but
// the actual function itself is not MainActor isolated, since the
// function will not hop onto the main actor.
//
// TODO: We should use some of the shared infrastructure to find the
// underlying value of op.get(). This is conservatively correct for
// now.
auto value = lookThroughNonVarDeclOwnershipInsts(op.get());
if (!SILIsolationInfo::get(value).isUnsafeNonIsolated())
return false;
foundOneNonIsolatedUnsafe = true;
}

return foundOneNonIsolatedUnsafe;
}

SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
if (auto fas = FullApplySite::isa(inst)) {
// Before we do anything, see if we have a sending result. In such a case,
Expand Down Expand Up @@ -520,23 +457,12 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
}

if (auto *pai = dyn_cast<PartialApplyInst>(inst)) {
// Check if we have any captures and if the isolation info for all captures
// are nonisolated(unsafe) or Sendable. In such a case, we consider the
// partial_apply to be nonisolated(unsafe). We purposely do not do this if
// the partial_apply does not have any parameters just out of paranoia... we
// shouldn't have such a partial_apply emitted by SILGen (it should use a
// thin to thick function or something like that)... but in that case since
// we do not have any nonisolated(unsafe), it doesn't make sense to
// propagate nonisolated(unsafe).
bool partialApplyIsNonIsolatedUnsafe = isPartialApplyNonisolatedUnsafe(pai);

if (auto *ace = pai->getLoc().getAsASTNode<AbstractClosureExpr>()) {
auto actorIsolation = ace->getActorIsolation();

if (actorIsolation.isGlobalActor()) {
return SILIsolationInfo::getGlobalActorIsolated(
pai, actorIsolation.getGlobalActor())
.withUnsafeNonIsolated(partialApplyIsNonIsolatedUnsafe);
pai, actorIsolation.getGlobalActor());
}

if (actorIsolation.isActorInstanceIsolated()) {
Expand All @@ -559,16 +485,13 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
if (auto *fArg = dyn_cast<SILFunctionArgument>(
actualIsolatedValue.getValue())) {
if (auto info =
SILIsolationInfo::getActorInstanceIsolated(pai, fArg)
.withUnsafeNonIsolated(
partialApplyIsNonIsolatedUnsafe))
SILIsolationInfo::getActorInstanceIsolated(pai, fArg))
return info;
}
}

return SILIsolationInfo::getActorInstanceIsolated(
pai, actorInstance, actorIsolation.getActor())
.withUnsafeNonIsolated(partialApplyIsNonIsolatedUnsafe);
pai, actorInstance, actorIsolation.getActor());
}

// For now, if we do not have an actor instance, just create an actor
Expand All @@ -582,16 +505,12 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
//
// TODO: How do we want to resolve this.
return SILIsolationInfo::getPartialApplyActorInstanceIsolated(
pai, actorIsolation.getActor())
.withUnsafeNonIsolated(partialApplyIsNonIsolatedUnsafe);
pai, actorIsolation.getActor());
}

assert(actorIsolation.getKind() != ActorIsolation::Erased &&
"Implement this!");
}

if (partialApplyIsNonIsolatedUnsafe)
return SILIsolationInfo::getDisconnected(partialApplyIsNonIsolatedUnsafe);
}

// See if the memory base is a ref_element_addr from an address. If so, add
Expand Down
222 changes: 1 addition & 221 deletions test/Concurrency/transfernonsendable_nonisolatedunsafe.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// MARK: Declarations //
////////////////////////

class NonSendableKlass {
class NonSendableKlass { // expected-complete-note 98{{}}
var field: NonSendableKlass? = nil
}

Expand Down Expand Up @@ -823,223 +823,3 @@ actor ActorContainingSendableStruct {
}


////////////////////
// MARK: Closures //
////////////////////

func closureTests() async {
func sendingClosure(_ x: sending () -> ()) {
}

func testLetOneNSVariableError() async {
let x = NonSendableKlass()
sendingClosure { _ = x } // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
sendingClosure { _ = x } // expected-note {{access can happen concurrently}}
}

func testLetNonIsolatedUnsafeNSVariableNoError() async {
nonisolated(unsafe) let x = NonSendableKlass()
sendingClosure { _ = x }
sendingClosure { _ = x }
}

func testLetOneNSVariableSVariableError() async {
let x = NonSendableKlass()
let y = CustomActorInstance()
sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
_ = x
_ = y
}
sendingClosure { // expected-note {{access can happen concurrently}}
_ = x
_ = y
}
}

func testLetNonIsolatedUnsafeNSSVariableNoError() async {
nonisolated(unsafe) let x = NonSendableKlass()
let y = CustomActorInstance()
sendingClosure {
_ = x
_ = y
}
sendingClosure {
_ = x
_ = y
}
}

func testLetTwoNSVariableError() async {
let x = NonSendableKlass()
let y = NonSendableKlass()
sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
_ = x
_ = y
}
sendingClosure { // expected-note {{access can happen concurrently}}
_ = x
_ = y
}
}

func testLetTwoNSVariableError2() async {
nonisolated(unsafe) let x = NonSendableKlass()
let y = NonSendableKlass()
sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
_ = x
_ = y
}
sendingClosure { // expected-note {{access can happen concurrently}}
_ = x
_ = y
}
}

func testLetTwoNSVariableError3() async {
nonisolated(unsafe) let x = NonSendableKlass()
nonisolated(unsafe) let y = NonSendableKlass()
sendingClosure {
_ = x
_ = y
}
sendingClosure {
_ = x
_ = y
}
}

func testVarOneNSVariableError() async {
var x = NonSendableKlass()
x = NonSendableKlass()

sendingClosure { _ = x } // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
sendingClosure { _ = x } // expected-note {{access can happen concurrently}}
}

func testVarNonIsolatedUnsafeNSVariableNoError() async {
nonisolated(unsafe) var x = NonSendableKlass()
x = NonSendableKlass()

sendingClosure { _ = x }
sendingClosure { _ = x }
}

func testVarOneNSVariableSVariableError() async {
var x = NonSendableKlass()
x = NonSendableKlass()
var y = CustomActorInstance()
y = CustomActorInstance()
sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
_ = x
_ = y
}
sendingClosure { // expected-note {{access can happen concurrently}}
_ = x
_ = y
}
}

func testVarNonIsolatedUnsafeNSSVariableNoError() async {
nonisolated(unsafe) var x = NonSendableKlass()
x = NonSendableKlass()
var y = CustomActorInstance()
y = CustomActorInstance()
sendingClosure {
_ = x
_ = y
}
sendingClosure {
_ = x
_ = y
}
}

func testVarTwoNSVariableError() async {
var x = NonSendableKlass()
x = NonSendableKlass()
var y = NonSendableKlass()
y = NonSendableKlass()
sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
_ = x
_ = y
}
sendingClosure { // expected-note {{access can happen concurrently}}
_ = x
_ = y
}
}

func testVarTwoNSVariableError2() async {
nonisolated(unsafe) var x = NonSendableKlass()
x = NonSendableKlass()
var y = NonSendableKlass()
y = NonSendableKlass()
sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
_ = x
_ = y
}
sendingClosure { // expected-note {{access can happen concurrently}}
_ = x
_ = y
}
}

func testVarTwoNSVariableError3() async {
nonisolated(unsafe) var x = NonSendableKlass()
x = NonSendableKlass()
nonisolated(unsafe) var y = NonSendableKlass()
y = NonSendableKlass()
sendingClosure {
_ = x
_ = y
}
sendingClosure {
_ = x
_ = y
}
}

func testWithTaskDetached() async {
let x1 = NonSendableKlass()
Task.detached { _ = x1 } // expected-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to static method 'detached(name:priority:operation:)' risks causing races in between local and caller code}}
Task.detached { _ = x1 } // expected-note {{access can happen concurrently}}

nonisolated(unsafe) let x2 = NonSendableKlass()
Task.detached { _ = x2 }
Task.detached { _ = x2 }

nonisolated(unsafe) let x3a = NonSendableKlass()
nonisolated(unsafe) let x3b = NonSendableKlass()
Task.detached { _ = x3a; _ = x3b }
Task.detached { _ = x3a; _ = x3b }

nonisolated(unsafe) let x4a = NonSendableKlass()
let x4b = NonSendableKlass()
Task.detached { _ = x4a; _ = x4b } // expected-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to static method 'detached(name:priority:operation:)' risks causing races in between local and caller code}}
Task.detached { _ = x4a; _ = x4b } // expected-note {{access can happen concurrently}}
}

// The reason why this works is that we do not infer nonisolated(unsafe)
// passed the begin_borrow [var_decl] of y. So we think the closure is
// nonisolated(unsafe), but its uses via the begin_borrow [var_decl] is
// not.
func testNamedClosure() async {
nonisolated(unsafe) let x = NonSendableKlass()
let y = {
_ = x
}
sendingClosure(y) // expected-warning {{sending 'y' risks causing data races}}
// expected-note @-1 {{'y' used after being passed as a 'sending' parameter}}
sendingClosure(y) // expected-note {{access can happen concurrently}}
}
}