Skip to content

[region-isolation] Do not squelch use-after-transfer error even if the value is isolated to the same actor. #75368

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 2 commits into from
Jul 19, 2024
Merged

[region-isolation] Do not squelch use-after-transfer error even if the value is isolated to the same actor. #75368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3aee7da
[region-isolation] Remove expected-complete-warning from a test. It i…
gottesmm Jul 19, 2024
ebedf63
[region-isolation] Do not squelch use-after-transfer error even if th…
gottesmm Jul 19, 2024
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
7 changes: 0 additions & 7 deletions include/swift/SILOptimizer/Utils/PartitionUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1375,13 +1375,6 @@ struct PartitionOpEvaluator {
void handleLocalUseAfterTransferHelper(const PartitionOp &op, Element elt,
Operand *transferringOp) const {
if (shouldTryToSquelchErrors()) {
if (auto isolationInfo = getIsolationInfo(op)) {
if (isolationInfo.isActorIsolated() &&
isolationInfo.hasSameIsolation(
SILIsolationInfo::get(transferringOp->getUser())))
return;
}

if (SILValue equivalenceClassRep =
getRepresentative(transferringOp->get())) {

Expand Down
29 changes: 15 additions & 14 deletions test/Concurrency/concurrent_value_checking.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
// REQUIRES: asserts

class NotConcurrent { } // expected-note 13{{class 'NotConcurrent' does not conform to the 'Sendable' protocol}}
// expected-complete-note @-1 13{{class 'NotConcurrent' does not conform to the 'Sendable' protocol}}
// expected-tns-allow-typechecker-note @-2 {{class 'NotConcurrent' does not conform to the 'Sendable' protocol}}
// expected-tns-allow-typechecker-note @-1 {{class 'NotConcurrent' does not conform to the 'Sendable' protocol}}

// ----------------------------------------------------------------------
// Sendable restriction on actor operations
Expand Down Expand Up @@ -67,23 +66,23 @@ actor A2 {
}

func testActorCreation(value: NotConcurrent) async {
_ = A2(value: value) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent' into actor-isolated context may introduce data races}}
_ = A2(value: value)
// expected-tns-warning @-1 {{sending 'value' risks causing data races}}
// expected-tns-note @-2 {{sending task-isolated 'value' to actor-isolated initializer 'init(value:)' risks causing data races between actor-isolated and task-isolated uses}}

_ = await A2(valueAsync: value) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent' into actor-isolated context may introduce data races}}
_ = await A2(valueAsync: value)
// expected-tns-warning @-1 {{sending 'value' risks causing data races}}
// expected-tns-note @-2 {{sending task-isolated 'value' to actor-isolated initializer 'init(valueAsync:)' risks causing data races between actor-isolated and task-isolated uses}}

_ = A2(delegatingSync: value) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent' into actor-isolated context may introduce data races}}
_ = A2(delegatingSync: value)
// expected-tns-warning @-1 {{sending 'value' risks causing data races}}
// expected-tns-note @-2 {{sending task-isolated 'value' to actor-isolated initializer 'init(delegatingSync:)' risks causing data races between actor-isolated and task-isolated uses}}

_ = await A2(delegatingAsync: value, 9) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent' into actor-isolated context may introduce data races}}
_ = await A2(delegatingAsync: value, 9)
// expected-tns-warning @-1 {{sending 'value' risks causing data races}}
// expected-tns-note @-2 {{sending task-isolated 'value' to actor-isolated initializer 'init(delegatingAsync:_:)' risks causing data races between actor-isolated and task-isolated uses}}

_ = await A2(nonisoAsync: value, 3) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent' into actor-isolated context may introduce data races}}
_ = await A2(nonisoAsync: value, 3)
// expected-tns-warning @-1 {{sending 'value' risks causing data races}}
// expected-tns-note @-2 {{sending task-isolated 'value' to actor-isolated initializer 'init(nonisoAsync:_:)' risks causing data races between actor-isolated and task-isolated uses}}
}
Expand All @@ -104,7 +103,7 @@ extension A1 {
// expected-warning@-1 {{expression is 'async' but is not marked with 'await'}}
// expected-note@-2 {{property access is 'async'}}
_ = await other.synchronous() // expected-warning{{non-sendable type 'NotConcurrent?' returned by call to actor-isolated function cannot cross actor boundary}}
_ = await other.asynchronous(nil) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent?' into actor-isolated context may introduce data races}}
_ = await other.asynchronous(nil)
}
}

Expand Down Expand Up @@ -142,8 +141,9 @@ 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?' in implicitly asynchronous access to global actor 'SomeGlobalActor'-isolated let 'globalValue' cannot cross actor boundary}}
await globalAsync(a) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}
await globalSync(a) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}
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}}

// expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
// expected-note@+1 {{property access is 'async'}}
Expand All @@ -154,7 +154,7 @@ func globalTest() async {
// expected-typechecker-note@+2 {{call is 'async'}}
// expected-typechecker-note@+1 {{property access is 'async'}}
globalAsync(E.notSafe)
// expected-complete-warning@-1 {{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}

// expected-typechecker-warning@-2 {{non-sendable type 'NotConcurrent?' in implicitly asynchronous access to global actor 'SomeGlobalActor'-isolated static property 'notSafe' cannot cross actor boundary}}
#endif
}
Expand All @@ -178,9 +178,10 @@ 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?' in implicitly asynchronous access to global actor 'SomeGlobalActor'-isolated let 'globalValue' cannot cross actor boundary}}
await globalAsync(a) // expected-complete-warning {{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}
await globalSync(a) // expected-complete-warning {{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}
_ = await ClassWithGlobalActorInits(nc) // expected-complete-warning {{passing argument of non-sendable type 'NotConcurrent' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}
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 ClassWithGlobalActorInits(nc)
// expected-warning @-1 {{non-sendable type 'ClassWithGlobalActorInits' returned by call to global actor 'SomeGlobalActor'-isolated function cannot cross actor boundary}}
// expected-tns-warning @-2 {{sending 'nc' risks causing data races}}
// expected-tns-note @-3 {{sending main actor-isolated 'nc' to global actor 'SomeGlobalActor'-isolated initializer 'init(_:)' risks causing data races between global actor 'SomeGlobalActor'-isolated and main actor-isolated uses}}
Expand Down
3 changes: 3 additions & 0 deletions test/Concurrency/sendable_checking.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,12 @@ func testNonSendableBaseArg() async {
let t = NonSendable()
await t.update()
// expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}
// expected-tns-warning @-2 {{sending 't' risks causing data races}}
// expected-tns-note @-3 {{sending 't' to main actor-isolated instance method 'update()' risks causing data races between main actor-isolated and local nonisolated uses}}

_ = await t.x
// expected-warning @-1 {{non-sendable type 'NonSendable' passed in implicitly asynchronous call to main actor-isolated property 'x' cannot cross actor boundary}}
// expected-tns-note @-2 {{access can happen concurrently}}
}

// We get the region isolation error here since t.y is custom actor isolated.
Expand Down
18 changes: 8 additions & 10 deletions test/Concurrency/transfernonsendable.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,7 @@ func closureInOut(_ a: MyActor) async {
// expected-tns-note @-3 {{sending 'ns0' to actor-isolated instance method 'useKlass' risks causing data races between actor-isolated and local nonisolated uses}}

if await booleanFlag {
// This is not an actual use since we are passing values to the same
// isolation domain.
await a.useKlass(ns1)
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass'}}
await a.useKlass(ns1) // expected-tns-note {{access can happen concurrently}}
} else {
closure() // expected-tns-note {{access can happen concurrently}}
}
Expand Down Expand Up @@ -1230,11 +1227,11 @@ func varNonSendableNonTrivialLetStructFieldClosureFlowSensitive4() async {
// good... that is QoI though.
await transferToMain(test) // expected-tns-warning {{sending 'test' risks causing data races}}
// expected-tns-note @-1 {{sending 'test' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
// expected-complete-warning @-2 {{passing argument of non-sendable type 'StructFieldTests' into main actor-isolated context may introduce data races}}
// expected-tns-note @-2 {{access can happen concurrently}}

// This is treated as a use since test is in box form and is mutable. So we
// treat assignment as a merge.
test = StructFieldTests() // expected-tns-note {{access can happen concurrently}}
test = StructFieldTests()
cls = {
useInOut(&test.varSendableNonTrivial)
}
Expand Down Expand Up @@ -1265,7 +1262,7 @@ func varNonSendableNonTrivialLetStructFieldClosureFlowSensitive5() async {
}

test.varSendableNonTrivial = SendableKlass()
useValue(test) // expected-tns-note {{access can happen concurrently}}
useValue(test)
}

func varNonSendableNonTrivialLetStructFieldClosureFlowSensitive6() async {
Expand Down Expand Up @@ -1434,7 +1431,7 @@ func controlFlowTest2() async {
x = NonSendableKlass()
}

useValue(x) // expected-tns-note {{access can happen concurrently}}
useValue(x)
}

////////////////////////
Expand Down Expand Up @@ -1750,8 +1747,9 @@ func sendableGlobalActorIsolated() {
// value.
func testIndirectParameterSameIsolationNoError() async {
let x = NonSendableKlass()
await transferToMain(x) // expected-complete-warning {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
await transferToMain(x) // expected-complete-warning {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
await transferToMain(x) // expected-tns-warning {{sending 'x' risks causing data races}}
// expected-tns-note @-1 {{sending 'x' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
await transferToMain(x) // expected-tns-note {{access can happen concurrently}}
}

extension MyActor {
Expand Down
6 changes: 3 additions & 3 deletions test/Concurrency/transfernonsendable_asynclet.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,13 +293,13 @@ func asyncLet_Let_ActorIsolated_CallBuriedInOtherExpr3() async {
let _ = await y
}

// Make sure that we do not emit an error for transferToMainInt in the async val
// function itself since we are sending the value to the same main actor
// isolated use and transferring it into one async let variable.
func asyncLet_Let_ActorIsolated_CallBuriedInOtherExpr4() async {
let x = NonSendableKlass()

async let y = useValue(transferToMainInt(x) + transferToMainInt(x))
// expected-warning @-1:26 {{sending 'x' risks causing data races}}
// expected-note @-2:26 {{sending 'x' to main actor-isolated global function 'transferToMainInt' risks causing data races between main actor-isolated and local nonisolated uses}}
// expected-note @-3:49 {{access can happen concurrently}}

let _ = await y
}
Expand Down
8 changes: 3 additions & 5 deletions test/Concurrency/transfernonsendable_instruction_matching.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,14 +645,12 @@ bb0:

fix_lifetime %value : $SendableKlass

// This is not considered a bad use of the raw pointer since this is
// sending to the same isolation domain.
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transferRawPointer(%rawPointer) : $@convention(thin) @async (Builtin.RawPointer) -> ()
// expected-warning @-1 {{}}
// expected-note @-2 {{access can happen concurrently}}

fix_lifetime %rawPointer : $Builtin.RawPointer
// expected-note @-1 {{access can happen concurrently}}
// expected-note @-2 {{access can happen concurrently}}

destroy_value %value : $SendableKlass
%9999 = tuple ()
Expand Down Expand Up @@ -725,10 +723,10 @@ bb0:

apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transferRawPointer(%rawPointer) : $@convention(thin) @async (Builtin.RawPointer) -> ()
// expected-warning @-1 {{}}
// expected-note @-2 {{access can happen concurrently}}

fix_lifetime %rawPointer : $Builtin.RawPointer
// expected-note @-1 {{access can happen concurrently}}
// expected-note @-2 {{access can happen concurrently}}

destroy_value %value : $SendableKlass
%9999 = tuple ()
Expand Down Expand Up @@ -804,10 +802,10 @@ bb0:

apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transferRawPointer(%rawPointer) : $@convention(thin) @async (Builtin.RawPointer) -> ()
// expected-warning @-1 {{}}
// expected-note @-2 {{access can happen concurrently}}

fix_lifetime %rawPointer : $Builtin.RawPointer
// expected-note @-1 {{access can happen concurrently}}
// expected-note @-2 {{access can happen concurrently}}

end_borrow %valueB : $SendableKlass
destroy_value %value : $SendableKlass
Expand Down