Skip to content

Commit f312369

Browse files
committed
Change send-never-sendable of isolated partial applies to use SIL level info instead of AST info.
The reason I am doing this is that we have gotten reports about certain test cases where we are emitting errors about self being captured in isolated closures where the sourceloc is invalid. The reason why this happened is that the decl returned by getIsolationCrossing did not have a SourceLoc since self was being used implicitly. In this commit I fix that issue by using SIL level information instead of AST level information. This guarantees that we get an appropriate SourceLoc. As an additional benefit, this fixed some extant errors where due to some sort of bug in the AST, we were saying that a value was nonisolated when it was actor isolated in some of the error msgs. rdar://151955519
1 parent 4b2af07 commit f312369

File tree

5 files changed

+39
-31
lines changed

5 files changed

+39
-31
lines changed

lib/SILOptimizer/Mandatory/SendNonSendable.cpp

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1857,30 +1857,23 @@ bool SentNeverSendableDiagnosticInferrer::initForSendingPartialApply(
18571857
bool SentNeverSendableDiagnosticInferrer::initForIsolatedPartialApply(
18581858
Operand *op, AbstractClosureExpr *ace,
18591859
std::optional<ActorIsolation> actualCallerIsolation) {
1860-
SmallVector<std::tuple<CapturedValue, unsigned, ApplyIsolationCrossing>, 8>
1861-
foundCapturedIsolationCrossing;
1862-
ace->getIsolationCrossing(foundCapturedIsolationCrossing);
1863-
if (foundCapturedIsolationCrossing.empty())
1860+
auto diagnosticPair = findClosureUse(op);
1861+
if (!diagnosticPair)
18641862
return false;
18651863

1866-
// We use getASTAppliedArgIndex instead of getAppliedArgIndex to ensure that
1867-
// we ignore for our indexing purposes any implicit initial parameters like
1868-
// isolated(any).
1869-
unsigned opIndex = ApplySite(op->getUser()).getASTAppliedArgIndex(*op);
1870-
for (auto &p : foundCapturedIsolationCrossing) {
1871-
if (std::get<1>(p) == opIndex) {
1872-
auto loc = RegularLocation(std::get<0>(p).getLoc());
1873-
auto crossing = std::get<2>(p);
1874-
auto declIsolation = crossing.getCallerIsolation();
1875-
auto closureIsolation = crossing.getCalleeIsolation();
1876-
if (!bool(declIsolation) && actualCallerIsolation) {
1877-
declIsolation = *actualCallerIsolation;
1878-
}
1879-
diagnosticEmitter.emitNamedFunctionArgumentClosure(
1880-
loc, std::get<0>(p).getDecl()->getBaseIdentifier(),
1881-
ApplyIsolationCrossing(declIsolation, closureIsolation));
1882-
return true;
1883-
}
1864+
auto *diagnosticOp = diagnosticPair->first;
1865+
1866+
ApplyIsolationCrossing crossing(
1867+
*op->getFunction()->getActorIsolation(),
1868+
*diagnosticOp->getFunction()->getActorIsolation());
1869+
1870+
// We do not need to worry about failing to infer a name here since we are
1871+
// going to be returning some form of a SILFunctionArgument which is always
1872+
// easy to find a name for.
1873+
if (auto rootValueAndName = inferNameAndRootHelper(op->get())) {
1874+
diagnosticEmitter.emitNamedFunctionArgumentClosure(
1875+
diagnosticOp->getUser()->getLoc(), rootValueAndName->first, crossing);
1876+
return true;
18841877
}
18851878

18861879
return false;

test/Concurrency/transfernonsendable.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2072,3 +2072,23 @@ func indirectEnumTestCase<T>(_ e: MyEnum<T>) async -> Bool {
20722072
return false
20732073
}
20742074
}
2075+
2076+
/// Make sure that we properly infer the location of the capture of self in func
2077+
/// d().
2078+
func inferLocationOfCapturedTaskIsolatedSelfCorrectly() {
2079+
class A {
2080+
var block: @MainActor () -> Void = {}
2081+
}
2082+
class B {
2083+
let a = A()
2084+
2085+
func d() {
2086+
a.block = c // expected-warning {{converting non-Sendable function value to '@MainActor @Sendable () -> Void' may introduce data races}}
2087+
// expected-tns-warning @-1 {{sending 'self' risks causing data races}}
2088+
// expected-tns-note @-2 {{task-isolated 'self' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
2089+
}
2090+
2091+
@MainActor
2092+
func c() {}
2093+
}
2094+
}

test/Concurrency/transfernonsendable_global_actor.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,8 @@ struct Clock {
205205
let ns = customActorIsolatedGlobal
206206

207207
let _ = { @MainActor in
208-
// TODO: The type checker seems to think that the isolation here is
209-
// nonisolated instead of custom actor isolated.
210208
print(ns) // expected-tns-warning {{sending 'ns' risks causing data races}}
211-
// expected-tns-note @-1 {{global actor 'CustomActor'-isolated 'ns' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
209+
// expected-tns-note @-1 {{global actor 'CustomActor'-isolated 'ns' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later global actor 'CustomActor'-isolated uses}}
212210
// expected-complete-warning @-2 {{capture of 'ns' with non-Sendable type 'NonSendableKlass' in a '@Sendable' closure}}
213211
}
214212

test/Concurrency/transfernonsendable_initializers.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,9 @@ func initActorWithSyncNonIsolatedInit2(_ k: NonSendableKlass) {
8181

8282
actor ActorWithAsyncIsolatedInit {
8383
init(_ newK: NonSendableKlass) async {
84-
// TODO: This should say actor isolated.
8584
let _ = { @MainActor in
8685
print(newK) // expected-error {{sending 'newK' risks causing data races}}
87-
// expected-note @-1 {{'self'-isolated 'newK' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
86+
// expected-note @-1 {{'self'-isolated 'newK' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later actor-isolated uses}}
8887
}
8988
}
9089
}
@@ -150,7 +149,7 @@ class ClassWithAsyncIsolatedInit {
150149
init(_ newK: NonSendableKlass) async {
151150
let _ = { @MainActor in
152151
print(newK) // expected-error {{sending 'newK' risks causing data races}}
153-
// expected-note @-1 {{global actor 'CustomActor'-isolated 'newK' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
152+
// expected-note @-1 {{global actor 'CustomActor'-isolated 'newK' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later global actor 'CustomActor'-isolated uses}}
154153
}
155154
}
156155
}

test/Distributed/distributed_actor_transfernonsendable.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,8 @@ distributed actor MyDistributedActor {
5959

6060
distributed func transferActorIsolatedArgIntoClosure(_ x: NonSendableKlass) async {
6161
_ = { @MainActor in
62-
// TODO: In 2nd part of message should say actor-isolated instead of later
63-
// nonisolated uses in the case of a closure.
6462
print(x) // expected-error {{sending 'x' risks causing data races}}
65-
// expected-note @-1 {{'self'-isolated 'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
63+
// expected-note @-1 {{'self'-isolated 'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later actor-isolated uses}}
6664
}
6765
}
6866

0 commit comments

Comments
 (0)