Skip to content

Commit 6ff708e

Browse files
committed
[Concurrency] Don't strip @Sendable when determining the actor isolation
of a closure in a `@preconcurrency` context. Instead, downgrade any diagnostics about non-`Sendable` captures using the `.limitBehaviorUntilSwiftVersion` mechanism like other preconcurrency errors that are suppressed in minimal checking. Stripping `@Sendable` from closure types actually changes the isolation of the closure, because non-`Sendable` closures are isolated to the context they're formed in, which leads to bogus dynamic assertion failures under `-enable-actor-data-race-checks`.
1 parent 7df8812 commit 6ff708e

File tree

2 files changed

+42
-8
lines changed

2 files changed

+42
-8
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -675,10 +675,6 @@ static bool isSendableClosure(
675675
if (forActorIsolation && explicitClosure->inheritsActorContext()) {
676676
return false;
677677
}
678-
679-
if (explicitClosure->isIsolatedByPreconcurrency() &&
680-
!shouldDiagnoseExistingDataRaces(closure->getParent()))
681-
return false;
682678
}
683679

684680
if (auto type = closure->getType()) {
@@ -3692,13 +3688,16 @@ namespace {
36923688
bool checkLocalCapture(
36933689
ConcreteDeclRef valueRef, SourceLoc loc, DeclRefExpr *declRefExpr) {
36943690
auto value = valueRef.getDecl();
3691+
auto *dc = getDeclContext();
36953692

36963693
// Check whether we are in a context that will not execute concurrently
36973694
// with the context of 'self'. If not, it's safe.
3698-
if (!mayExecuteConcurrentlyWith(
3699-
getDeclContext(), findCapturedDeclContext(value)))
3695+
if (!mayExecuteConcurrentlyWith(dc, findCapturedDeclContext(value)))
37003696
return false;
37013697

3698+
SendableCheckContext sendableBehavior(dc);
3699+
auto limit = sendableBehavior.defaultDiagnosticBehavior();
3700+
37023701
// Check whether this is a local variable, in which case we can
37033702
// determine whether it was safe to access concurrently.
37043703
if (auto var = dyn_cast<VarDecl>(value)) {
@@ -3738,7 +3737,7 @@ namespace {
37383737
loc, diag::concurrent_access_of_local_capture,
37393738
parent.dyn_cast<LoadExpr *>(),
37403739
var)
3741-
.warnUntilSwiftVersion(6);
3740+
.limitBehaviorUntilSwiftVersion(limit, 6);
37423741
return true;
37433742
}
37443743

@@ -3748,7 +3747,7 @@ namespace {
37483747

37493748
func->diagnose(diag::local_function_executed_concurrently, func)
37503749
.fixItInsert(func->getAttributeInsertionLoc(false), "@Sendable ")
3751-
.warnUntilSwiftVersion(6);
3750+
.limitBehaviorUntilSwiftVersion(limit, 6);
37523751

37533752
// Add the @Sendable attribute implicitly, so we don't diagnose
37543753
// again.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %target-run-simple-swift(-Xfrontend -disable-availability-checking -enable-actor-data-race-checks -swift-version 5 -strict-concurrency=minimal) | %FileCheck %s
2+
3+
// REQUIRES: executable_test
4+
// REQUIRES: concurrency
5+
// REQUIRES: concurrency_runtime
6+
7+
@preconcurrency @MainActor
8+
protocol P {
9+
func requirement()
10+
}
11+
12+
class C: P {
13+
func requirement() {
14+
call {
15+
print("don't crash!")
16+
}
17+
}
18+
19+
var task: Task<Void, Never>?
20+
21+
@preconcurrency func call(closure: @escaping @Sendable () -> Void) {
22+
task = Task.detached {
23+
closure()
24+
}
25+
}
26+
27+
func wait() async {
28+
await task?.value
29+
}
30+
}
31+
32+
// CHECK: don't crash!
33+
let c = C()
34+
c.requirement()
35+
await c.wait()

0 commit comments

Comments
 (0)