Skip to content

Commit 9d31a41

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
1 parent 57392b1 commit 9d31a41

File tree

7 files changed

+230
-26
lines changed

7 files changed

+230
-26
lines changed

‎include/swift/AST/DiagnosticsSIL.def

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

1029+
NOTE(regionbasedisolation_named_transfer_nt_asynclet_capture, none,
1030+
"sending %1 %0 into async let risks causing data races between nonisolated and %1 uses",
1031+
(Identifier, StringRef))
1032+
10291033
// Misc Error.
10301034
ERROR(regionbasedisolation_task_or_actor_isolated_transferred, none,
10311035
"task or actor isolated value cannot be sent", ())

‎lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

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

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

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

14041426
class TransferNonTransferrableDiagnosticInferrer {
1427+
struct AutoClosureWalker;
1428+
14051429
TransferNonTransferrableDiagnosticEmitter diagnosticEmitter;
14061430

14071431
public:
@@ -1454,6 +1478,74 @@ bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
14541478
return false;
14551479
}
14561480

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

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

‎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
}
@@ -45,6 +49,13 @@ func useInOut<T>(_ x: inout T) {}
4549
func useValue<T>(_ x: T) -> T { x }
4650
func useValueWrapInOptional<T>(_ x: T) -> T? { x }
4751

52+
func useValueNoReturnWithInstance<T, V : Actor>(_ x: T, _ y: V) -> () { fatalError() }
53+
func useValueAsyncNoReturnWithInstance<T, V : Actor>(_ x: T, _ y: V) async -> () { fatalError() }
54+
@MainActor
55+
func useMainActorValueAsyncNoReturn<T>(_ x: T) async -> () { fatalError() }
56+
@MainActor
57+
func useMainActorValueNoReturn<T>(_ x: T) -> () { fatalError() }
58+
4859
@MainActor func returnValueFromMain<T>() async -> T { fatalError() }
4960
@MainActor func transferToMain<T>(_ t: T) async {}
5061
@MainActor func transferToMainInt<T>(_ t: T) async -> Int { 5 }
@@ -720,11 +731,50 @@ func asyncLetWithoutCapture() async {
720731
}
721732

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

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

0 commit comments

Comments
 (0)