Skip to content

[rbi] Simplify some logic that got confused so that passing an actor isolated value to a callee that is isolated ot the same actor is not considered a send. #80911

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 11 additions & 8 deletions include/swift/SILOptimizer/Utils/PartitionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -1438,15 +1438,18 @@ struct PartitionOpEvaluator {
}
std::tie(sentRegionIsolation, regionHasClosureCapturedElt) = *pairOpt;

// If we merged anything, we need to handle an attempt to send a
// never-sent value unless our value has the same isolation info as our
// callee.
auto calleeIsolationInfo = getIsolationInfo(op);
if (!(calleeIsolationInfo &&
sentRegionIsolation.hasSameIsolation(calleeIsolationInfo)) &&
!sentRegionIsolation.isDisconnected()) {
return handleSendNeverSentHelper(op, op.getOpArg1(),
sentRegionIsolation);

// If our callee and region are both actor isolated and part of the same
// isolation domain, do not treat this as a send.
if (calleeIsolationInfo.isActorIsolated() &&
sentRegionIsolation.hasSameIsolation(calleeIsolationInfo))
return;

// At this point, check if our sent value is not disconnected. If so, emit
// a sent never sendable helper.
if (sentRegionIsolation && !sentRegionIsolation.isDisconnected()) {
return handleSendNeverSentHelper(op, op.getOpArg1(), sentRegionIsolation);
}

// Next see if we are disconnected and have the same isolation. In such a
Expand Down
10 changes: 4 additions & 6 deletions test/Concurrency/concurrent_value_checking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,8 @@ func globalTest() async {
// expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
// expected-note@+1 {{property access is 'async'}}
let a = globalValue // expected-warning{{non-sendable type 'NotConcurrent?' of let 'globalValue' cannot exit global actor 'SomeGlobalActor'-isolated context}}
await globalAsync(a) // expected-tns-warning {{sending 'a' risks causing data races}}
// expected-tns-note @-1 {{sending global actor 'SomeGlobalActor'-isolated 'a' to global actor 'SomeGlobalActor'-isolated global function 'globalAsync' risks causing data races between global actor 'SomeGlobalActor'-isolated and local nonisolated uses}}
await globalSync(a) // expected-tns-note {{access can happen concurrently}}
await globalAsync(a)
await globalSync(a)

// expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
// expected-note@+1 {{property access is 'async'}}
Expand Down Expand Up @@ -178,9 +177,8 @@ func globalTestMain(nc: NotConcurrent) async {
// expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
// expected-note@+1 {{property access is 'async'}}
let a = globalValue // expected-warning {{non-sendable type 'NotConcurrent?' of let 'globalValue' cannot exit global actor 'SomeGlobalActor'-isolated context}}
await globalAsync(a) // expected-tns-warning {{sending 'a' risks causing data races}}
// expected-tns-note @-1 {{sending global actor 'SomeGlobalActor'-isolated 'a' to global actor 'SomeGlobalActor'-isolated global function 'globalAsync' risks causing data races between global actor 'SomeGlobalActor'-isolated and local main actor-isolated uses}}
await globalSync(a) // expected-tns-note {{access can happen concurrently}}
await globalAsync(a)
await globalSync(a)
_ = await ClassWithGlobalActorInits(nc)
// expected-tns-warning @-1 {{non-Sendable 'ClassWithGlobalActorInits'-typed result can not be returned from global actor 'SomeGlobalActor'-isolated initializer 'init(_:)' to main actor-isolated context}}
// expected-tns-warning @-2 {{sending 'nc' risks causing data races}}
Expand Down
7 changes: 4 additions & 3 deletions test/Concurrency/transfernonsendable.sil
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,14 @@ bb0:
%f = function_ref @transferIndirectWithOutResult : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f<NonSendableKlass>(%1, %0) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0 // expected-warning {{}}

// We do not error here on use after send since %1 is already isolated to the
// same global actor as transferIndirect which is not considered to be a
// send. We leave the actual error to be the error about returning an isolated
// value into a non-isolated context.
%f2 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f2<NonSendableKlass>(%1) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
// expected-warning @-1 {{}}
// expected-note @-2 {{}}
%useIndirect = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
apply %useIndirect<NonSendableKlass>(%1) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
// expected-note @-1 {{access can happen concurrently}}

destroy_addr %1 : $*NonSendableKlass
dealloc_stack %1 : $*NonSendableKlass
Expand Down
27 changes: 27 additions & 0 deletions test/Concurrency/transfernonsendable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2027,3 +2027,30 @@ func testIsolatedParamInference() {
}
}
}

// We shouldn't error here since test2 is isolated to B since we are capturing
// self and funcParam is also exposed to B's isolated since it is a parameter to
// one of B's methods.
func sendIsolatedValueToItsOwnIsolationDomain() {
class A {
func useValue() {}
}

func helper(_ x: @escaping () -> A) {}

actor B {
let field = A()

private func test(funcParam: A?) async {
helper {
if let funcParam {
return funcParam
} else {
return self.field
}
}

funcParam?.useValue()
}
}
}