diff --git a/lib/SILOptimizer/Analysis/RegionAnalysis.cpp b/lib/SILOptimizer/Analysis/RegionAnalysis.cpp index 1d19efe5c1144..82a892a393347 100644 --- a/lib/SILOptimizer/Analysis/RegionAnalysis.cpp +++ b/lib/SILOptimizer/Analysis/RegionAnalysis.cpp @@ -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); } diff --git a/lib/SILOptimizer/Utils/SILIsolationInfo.cpp b/lib/SILOptimizer/Utils/SILIsolationInfo.cpp index b4af1a75e28fc..0b98dbb1f7d58 100644 --- a/lib/SILOptimizer/Utils/SILIsolationInfo.cpp +++ b/lib/SILOptimizer/Utils/SILIsolationInfo.cpp @@ -363,69 +363,6 @@ inferIsolationInfoForTempAllocStack(AllocStackInst *asi) { return SILIsolationInfo::get(targetOperand->getUser()); } -static SILValue lookThroughNonVarDeclOwnershipInsts(SILValue v) { - while (true) { - if (auto *svi = dyn_cast(v)) { - if (isa(svi)) { - v = svi->getOperand(0); - continue; - } - - if (auto *bbi = dyn_cast(v)) { - if (!bbi->isFromVarDecl()) { - v = bbi->getOperand(); - continue; - } - return v; - } - - if (auto *mvi = dyn_cast(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, @@ -520,23 +457,12 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) { } if (auto *pai = dyn_cast(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()) { auto actorIsolation = ace->getActorIsolation(); if (actorIsolation.isGlobalActor()) { return SILIsolationInfo::getGlobalActorIsolated( - pai, actorIsolation.getGlobalActor()) - .withUnsafeNonIsolated(partialApplyIsNonIsolatedUnsafe); + pai, actorIsolation.getGlobalActor()); } if (actorIsolation.isActorInstanceIsolated()) { @@ -559,16 +485,13 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) { if (auto *fArg = dyn_cast( 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 @@ -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 diff --git a/test/Concurrency/transfernonsendable_nonisolatedunsafe.swift b/test/Concurrency/transfernonsendable_nonisolatedunsafe.swift index 13f1c2e414442..e59a787cc0f34 100644 --- a/test/Concurrency/transfernonsendable_nonisolatedunsafe.swift +++ b/test/Concurrency/transfernonsendable_nonisolatedunsafe.swift @@ -12,7 +12,7 @@ // MARK: Declarations // //////////////////////// -class NonSendableKlass { +class NonSendableKlass { // expected-complete-note 98{{}} var field: NonSendableKlass? = nil } @@ -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}} - } -}