Skip to content

Commit 12b6975

Browse files
gottesmmgottesmm
committed
[region-isolation] Treat async let as a isolation inference boundary closure and fix diagnostics due to change.
Otherwise, we will assume that an async let autoclosure infers isolation from its DeclContext... which we do not want. An async let autoclosure should always be nonisolated + sending. The diagnostic change that I mentioned in the header is that we were emitting unfortunate "sending task or actor isolated could result in races" error. I eliminated this by adding a new diagnostic for transfer non transferrable errors happening in autoclosures. So now we emit this: ```swift func asyncLetInferAsNonIsolated<T : Actor>( isolation actor: isolated T ) async throws { async let subTask: Void = { await useValueAsyncNoReturnWithInstance(self, actor) // expected-warning @-1:47 {{sending 'self' risks causing data races}} // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}} }() await subTask ``` I also noticed that we did not have enough test cases for autoclosures in general so I also added a bunch of tests just so we can see what the current behavior is. I think there are a few issues therein (I believe some may have been reported due to '??'). rdar://130151318 (cherry picked from commit 03b26fd)
1 parent b7a2ff5 commit 12b6975

File tree

7 files changed

+233
-26
lines changed

7 files changed

+233
-26
lines changed

‎include/swift/AST/DiagnosticsSIL.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,10 @@ NOTE(regionbasedisolation_named_isolated_closure_yields_race, none,
10221022
"%0%1 is captured by a %2 closure. %2 uses in closure may race against later %3 uses",
10231023
(StringRef, Identifier, ActorIsolation, ActorIsolation))
10241024

1025+
NOTE(regionbasedisolation_named_transfer_nt_asynclet_capture, none,
1026+
"sending %1 %0 into async let risks causing data races between nonisolated and %1 uses",
1027+
(Identifier, StringRef))
1028+
10251029
// Misc Error.
10261030
ERROR(regionbasedisolation_task_or_actor_isolated_transferred, none,
10271031
"task or actor isolated value cannot be sent", ())

‎lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,10 @@ class TransferNonTransferrableDiagnosticEmitter {
12161216
return getTransferringApplyCalleeInfo(info.transferredOperand->getUser());
12171217
}
12181218

1219+
SILLocation getLoc() const {
1220+
return info.transferredOperand->getUser()->getLoc();
1221+
}
1222+
12191223
/// Return the isolation region info for \p getNonTransferrableValue().
12201224
SILDynamicMergedIsolationInfo getIsolationRegionInfo() const {
12211225
return info.isolationRegionInfo;
@@ -1291,6 +1295,24 @@ class TransferNonTransferrableDiagnosticEmitter {
12911295
.limitBehaviorIf(getBehaviorLimit());
12921296
}
12931297

1298+
void emitNamedAsyncLetCapture(SILLocation loc, Identifier name,
1299+
SILIsolationInfo transferredValueIsolation) {
1300+
assert(!getIsolationRegionInfo().isDisconnected() &&
1301+
"Should never be disconnected?!");
1302+
emitNamedOnlyError(loc, name);
1303+
1304+
SmallString<64> descriptiveKindStr;
1305+
{
1306+
llvm::raw_svector_ostream os(descriptiveKindStr);
1307+
getIsolationRegionInfo().printForDiagnostics(os);
1308+
}
1309+
1310+
diagnoseNote(loc,
1311+
diag::regionbasedisolation_named_transfer_nt_asynclet_capture,
1312+
name, descriptiveKindStr)
1313+
.limitBehaviorIf(getBehaviorLimit());
1314+
}
1315+
12941316
void emitNamedIsolation(SILLocation loc, Identifier name,
12951317
ApplyIsolationCrossing isolationCrossing) {
12961318
emitNamedOnlyError(loc, name);
@@ -1401,6 +1423,8 @@ class TransferNonTransferrableDiagnosticEmitter {
14011423
};
14021424

14031425
class TransferNonTransferrableDiagnosticInferrer {
1426+
struct AutoClosureWalker;
1427+
14041428
TransferNonTransferrableDiagnosticEmitter diagnosticEmitter;
14051429

14061430
public:
@@ -1453,6 +1477,74 @@ bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
14531477
return false;
14541478
}
14551479

1480+
/// This walker visits an AutoClosureExpr and looks for uses of a specific
1481+
/// captured value. We want to error on the uses in the autoclosure.
1482+
struct TransferNonTransferrableDiagnosticInferrer::AutoClosureWalker
1483+
: ASTWalker {
1484+
TransferNonTransferrableDiagnosticEmitter &foundTypeInfo;
1485+
ValueDecl *targetDecl;
1486+
SILIsolationInfo targetDeclIsolationInfo;
1487+
SmallPtrSet<Expr *, 8> visitedCallExprDeclRefExprs;
1488+
SILLocation captureLoc;
1489+
bool isAsyncLet;
1490+
1491+
AutoClosureWalker(TransferNonTransferrableDiagnosticEmitter &foundTypeInfo,
1492+
ValueDecl *targetDecl,
1493+
SILIsolationInfo targetDeclIsolationInfo,
1494+
SILLocation captureLoc, bool isAsyncLet)
1495+
: foundTypeInfo(foundTypeInfo), targetDecl(targetDecl),
1496+
targetDeclIsolationInfo(targetDeclIsolationInfo),
1497+
captureLoc(captureLoc), isAsyncLet(isAsyncLet) {}
1498+
1499+
Expr *lookThroughArgExpr(Expr *expr) {
1500+
while (true) {
1501+
if (auto *memberRefExpr = dyn_cast<MemberRefExpr>(expr)) {
1502+
expr = memberRefExpr->getBase();
1503+
continue;
1504+
}
1505+
1506+
if (auto *cvt = dyn_cast<ImplicitConversionExpr>(expr)) {
1507+
expr = cvt->getSubExpr();
1508+
continue;
1509+
}
1510+
1511+
if (auto *e = dyn_cast<ForceValueExpr>(expr)) {
1512+
expr = e->getSubExpr();
1513+
continue;
1514+
}
1515+
1516+
if (auto *t = dyn_cast<TupleElementExpr>(expr)) {
1517+
expr = t->getBase();
1518+
continue;
1519+
}
1520+
1521+
return expr;
1522+
}
1523+
}
1524+
1525+
PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
1526+
if (auto *declRef = dyn_cast<DeclRefExpr>(expr)) {
1527+
// If this decl ref expr was not visited as part of a callExpr and is our
1528+
// target decl... emit a simple async let error.
1529+
//
1530+
// This occurs if we do:
1531+
//
1532+
// ```
1533+
// let x = ...
1534+
// async let y = x
1535+
// ```
1536+
if (declRef->getDecl() == targetDecl) {
1537+
foundTypeInfo.emitNamedAsyncLetCapture(captureLoc,
1538+
targetDecl->getBaseIdentifier(),
1539+
targetDeclIsolationInfo);
1540+
return Action::Continue(expr);
1541+
}
1542+
}
1543+
1544+
return Action::Continue(expr);
1545+
}
1546+
};
1547+
14561548
bool TransferNonTransferrableDiagnosticInferrer::run() {
14571549
// We need to find the isolation info.
14581550
auto *op = diagnosticEmitter.getOperand();
@@ -1569,6 +1661,26 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
15691661
}
15701662
}
15711663

1664+
// If we are failing due to an autoclosure... see if we can find the captured
1665+
// value that is causing the issue.
1666+
if (auto *autoClosureExpr = loc.getAsASTNode<AutoClosureExpr>()) {
1667+
// To split up this work, we only do this for async let for now.
1668+
if (autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet) {
1669+
auto *i = op->getUser();
1670+
auto pai = ApplySite::isa(i);
1671+
unsigned captureIndex = pai.getAppliedArgIndex(*op);
1672+
auto captureInfo =
1673+
autoClosureExpr->getCaptureInfo().getCaptures()[captureIndex];
1674+
auto loc = RegularLocation(captureInfo.getLoc(), false /*implicit*/);
1675+
AutoClosureWalker walker(
1676+
diagnosticEmitter, captureInfo.getDecl(),
1677+
diagnosticEmitter.getIsolationRegionInfo().getIsolationInfo(), loc,
1678+
autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet);
1679+
autoClosureExpr->walk(walker);
1680+
return true;
1681+
}
1682+
}
1683+
15721684
diagnosticEmitter.emitUnknownUse(loc);
15731685
return true;
15741686
}

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,13 @@ isIsolationInferenceBoundaryClosure(const AbstractClosureExpr *closure,
741741
}
742742
}
743743

744+
// An autoclosure for an async let acts as a boundary. It is non-Sendable
745+
// regardless of its context.
746+
if (auto *autoclosure = dyn_cast<AutoClosureExpr>(closure)) {
747+
if (autoclosure->getThunkKind() == AutoClosureExpr::Kind::AsyncLet)
748+
return true;
749+
}
750+
744751
return isSendableClosure(closure, forActorIsolation);
745752
}
746753

‎test/Concurrency/global_actor_function_types.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,12 @@ func stripActor(_ expr: @Sendable @autoclosure () -> (() -> ())) async {
327327
return await stripActor(mainActorFn) // expected-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
328328
}
329329

330-
// NOTE: this warning is correct, but is only being caught by TypeCheckConcurrency's extra check.
330+
// We used to not emit an error here with strict-concurrency enabled since we
331+
// were inferring the async let to main actor isolated (which was incorrect). We
332+
// now always treat async let as non-isolated, so we get the same error in all
333+
// contexts.
331334
@MainActor func exampleWhereConstraintSolverHasWrongDeclContext_v2() async -> Int {
332-
async let a: () = noActor(mainActorFn) // expected-without-transferring-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
335+
async let a: () = noActor(mainActorFn) // expected-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
333336
await a
334337
}
335338

‎test/Concurrency/issue-57376.swift

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,47 +26,53 @@ func testAsyncSequence1Sendable<Seq: AsyncSequence>(_ seq: Seq) async throws whe
2626

2727
func testAsyncSequenceTypedPattern<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}}
2828
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}}
29-
async let result: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
30-
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
31-
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
29+
async let result: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
30+
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
31+
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
32+
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
3233
let _ = try! await result
3334
}
3435

3536
func testAsyncSequenceTypedPattern1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}}
3637
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}}
37-
async let _: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
38-
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
39-
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
38+
async let _: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
39+
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
40+
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
41+
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
4042
}
4143

4244
func testAsyncSequence<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}}
4345
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}}
44-
async let result = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
45-
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
46-
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
46+
async let result = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
47+
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
48+
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
49+
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
4750
let _ = try! await result
4851
}
4952

5053
func testAsyncSequence1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}}
5154
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}}
52-
async let _ = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
53-
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
54-
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
55+
async let _ = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
56+
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
57+
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
58+
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
5559
}
5660

5761
func testAsyncSequence3<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
5862
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
59-
async let result = seq // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
60-
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
61-
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
63+
async let result = seq // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
64+
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
65+
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
66+
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
6267
let _ = await result
6368
}
6469

6570
func testAsyncSequence4<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
6671
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
67-
async let _ = seq // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
68-
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
69-
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
72+
async let _ = seq // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
73+
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
74+
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
75+
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
7076
}
7177

7278
func search(query: String, entities: [String]) async throws -> [String] {

‎test/Concurrency/transfernonsendable_asynclet.swift

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ class NonSendableKlass {
1818

1919
class SendableKlass : @unchecked Sendable {}
2020

21-
actor Actor {
21+
struct NonSendableStruct { // expected-note {{}}
22+
var ns = NonSendableKlass()
23+
}
24+
25+
actor MyActor {
2226
var klass = NonSendableKlass()
2327
final var finalKlass = NonSendableKlass()
2428

@@ -28,7 +32,7 @@ actor Actor {
2832
func useNonSendableFunction(_: () -> Void) {}
2933
}
3034

31-
final actor FinalActor {
35+
final actor FinalMyActor {
3236
var klass = NonSendableKlass()
3337
func useKlass(_ x: NonSendableKlass) {}
3438
}
@@ -38,6 +42,13 @@ func useInOut<T>(_ x: inout T) {}
3842
func useValue<T>(_ x: T) -> T { x }
3943
func useValueWrapInOptional<T>(_ x: T) -> T? { x }
4044

45+
func useValueNoReturnWithInstance<T, V : Actor>(_ x: T, _ y: V) -> () { fatalError() }
46+
func useValueAsyncNoReturnWithInstance<T, V : Actor>(_ x: T, _ y: V) async -> () { fatalError() }
47+
@MainActor
48+
func useMainActorValueAsyncNoReturn<T>(_ x: T) async -> () { fatalError() }
49+
@MainActor
50+
func useMainActorValueNoReturn<T>(_ x: T) -> () { fatalError() }
51+
4152
@MainActor func returnValueFromMain<T>() async -> T { fatalError() }
4253
@MainActor func transferToMain<T>(_ t: T) async {}
4354
@MainActor func transferToMainInt<T>(_ t: T) async -> Int { 5 }
@@ -722,11 +733,50 @@ func asyncLetWithoutCapture() async {
722733
}
723734

724735
func asyncLet_Let_ActorIsolated_Method() async {
725-
let a = Actor()
736+
let a = MyActor()
726737
let x = NonSendableKlass()
727738
async let y = a.useKlass(x) // expected-warning {{sending 'x' risks causing data races}}
728739
// expected-note @-1 {{sending 'x' to actor-isolated instance method 'useKlass' risks causing data races between actor-isolated and local nonisolated uses}}
729740

730741
useValue(x) // expected-note {{access can happen concurrently}}
731742
let _ = await y
732743
}
744+
745+
extension NonSendableStruct {
746+
func asyncLetInferAsNonIsolated<T : Actor>(
747+
isolation actor: isolated T
748+
) async throws {
749+
async let subTask: Void = {
750+
await useValueAsyncNoReturnWithInstance(self, actor)
751+
// expected-warning @-1:47 {{sending 'self' risks causing data races}}
752+
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
753+
}()
754+
await subTask
755+
756+
async let subTask2: () = await useValueAsyncNoReturnWithInstance(self, actor)
757+
// expected-warning @-1 {{sending 'self' risks causing data races}}
758+
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
759+
await subTask2
760+
761+
async let subTask3: () = useValueNoReturnWithInstance(self, actor)
762+
// expected-warning @-1 {{sending 'self' risks causing data races}}
763+
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
764+
await subTask3
765+
766+
async let subTask4: () = await useMainActorValueAsyncNoReturn(self)
767+
// expected-warning @-1 {{sending 'self' risks causing data races}}
768+
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
769+
await subTask4
770+
771+
async let subTask5: () = useMainActorValueNoReturn(self)
772+
// expected-warning @-1 {{sending 'self' risks causing data races}}
773+
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
774+
await subTask5
775+
776+
async let subTask6: NonSendableStruct = self
777+
// expected-warning @-1 {{sending 'self' risks causing data races}}
778+
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
779+
// expected-warning @-3 {{non-sendable type 'NonSendableStruct' returned by implicitly asynchronous call to nonisolated function cannot cross actor boundary}}
780+
_ = await subTask6
781+
}
782+
}

0 commit comments

Comments
 (0)