Skip to content

Commit d6891a7

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 (cherry picked from commit f312369)
1 parent 17095df commit d6891a7

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
@@ -1866,30 +1866,23 @@ bool SentNeverSendableDiagnosticInferrer::initForSendingPartialApply(
18661866
bool SentNeverSendableDiagnosticInferrer::initForIsolatedPartialApply(
18671867
Operand *op, AbstractClosureExpr *ace,
18681868
std::optional<ActorIsolation> actualCallerIsolation) {
1869-
SmallVector<std::tuple<CapturedValue, unsigned, ApplyIsolationCrossing>, 8>
1870-
foundCapturedIsolationCrossing;
1871-
ace->getIsolationCrossing(foundCapturedIsolationCrossing);
1872-
if (foundCapturedIsolationCrossing.empty())
1869+
auto diagnosticPair = findClosureUse(op);
1870+
if (!diagnosticPair)
18731871
return false;
18741872

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

18951888
return false;

test/Concurrency/transfernonsendable.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,3 +2045,23 @@ func indirectEnumTestCase<T>(_ e: MyEnum<T>) async -> Bool {
20452045
return false
20462046
}
20472047
}
2048+
2049+
/// Make sure that we properly infer the location of the capture of self in func
2050+
/// d().
2051+
func inferLocationOfCapturedTaskIsolatedSelfCorrectly() {
2052+
class A {
2053+
var block: @MainActor () -> Void = {}
2054+
}
2055+
class B {
2056+
let a = A()
2057+
2058+
func d() {
2059+
a.block = c // expected-warning {{converting non-Sendable function value to '@MainActor @Sendable () -> Void' may introduce data races}}
2060+
// expected-tns-warning @-1 {{sending 'self' risks causing data races}}
2061+
// 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}}
2062+
}
2063+
2064+
@MainActor
2065+
func c() {}
2066+
}
2067+
}

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)