Skip to content

Commit 1160951

Browse files
gottesmmgottesmm
authored
Merge pull request #75069 from gottesmm/pr-6b7256344bb1a1ceb9d13dae6d52db0d356b46a3
[region-isolation] Treat async let as a isolation inference boundary closure and fix diagnostics due to change.
2 parents 155b96c + 03b26fd commit 1160951

File tree

7 files changed

+234
-27
lines changed

7 files changed

+234
-27
lines changed

‎include/swift/AST/DiagnosticsSIL.def

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

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

‎lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ getTransferringApplyCalleeInfo(SILInstruction *inst) {
111111
return {};
112112

113113
auto *decl = declRef->getDecl();
114-
if (!decl->hasName())
114+
if (!decl || !decl->hasName())
115115
return {};
116116

117117
return {{decl->getDescriptiveKind(), decl->getName()}};
@@ -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,26 @@ 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+
// To split up this work, we only do this for async let for now.
1669+
if (autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet) {
1670+
auto *i = op->getUser();
1671+
auto pai = ApplySite::isa(i);
1672+
unsigned captureIndex = pai.getAppliedArgIndex(*op);
1673+
auto captureInfo =
1674+
autoClosureExpr->getCaptureInfo().getCaptures()[captureIndex];
1675+
auto loc = RegularLocation(captureInfo.getLoc(), false /*implicit*/);
1676+
AutoClosureWalker walker(
1677+
diagnosticEmitter, captureInfo.getDecl(),
1678+
diagnosticEmitter.getIsolationRegionInfo().getIsolationInfo(), loc,
1679+
autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet);
1680+
autoClosureExpr->walk(walker);
1681+
return true;
1682+
}
1683+
}
1684+
15731685
diagnosticEmitter.emitUnknownUse(loc);
15741686
return true;
15751687
}

‎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)