Skip to content

Commit 1d506da

Browse files
committed
[Concurrency] Put the flow-sensitive concurrent captures check behind a flag
We're not quite ready to commit to the flow-sensitive check that would allow a concurrent function to read from a mutable local capture so long as the captured variable wasn't changed after the point of capture. Put it behind a flag and implement the more restrictive rule (no access to mutable local captures in concurrent code). We can relax it later.
1 parent 9de5afd commit 1d506da

11 files changed

+40
-19
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4274,9 +4274,10 @@ ERROR(actor_isolated_from_escaping_closure,none,
42744274
ERROR(local_function_executed_concurrently,none,
42754275
"concurrently-executed %0 %1 must be marked as '@concurrent'",
42764276
(DescriptiveDeclKind, DeclName))
4277-
ERROR(concurrent_mutation_of_local_capture,none,
4278-
"mutation of captured %0 %1 in concurrently-executing code",
4279-
(DescriptiveDeclKind, DeclName))
4277+
ERROR(concurrent_access_of_local_capture,none,
4278+
"%select{mutation of|reference to}0 captured %1 %2 in "
4279+
"concurrently-executing code",
4280+
(bool, DescriptiveDeclKind, DeclName))
42804281
NOTE(actor_isolated_sync_func,none,
42814282
"calls to %0 %1 from outside of its actor context are "
42824283
"implicitly asynchronous",

include/swift/Basic/LangOptions.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,9 @@ namespace swift {
247247
/// Enable experimental ConcurrentValue checking.
248248
bool EnableExperimentalConcurrentValueChecking = false;
249249

250+
/// Enable experimental flow-sensitive concurrent captures.
251+
bool EnableExperimentalFlowSensitiveConcurrentCaptures = false;
252+
250253
/// Disable the implicit import of the _Concurrency module.
251254
bool DisableImplicitConcurrencyModuleImport = false;
252255

include/swift/Option/FrontendOptions.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,10 @@ def enable_experimental_concurrent_value_checking :
226226
Flag<["-"], "enable-experimental-concurrent-value-checking">,
227227
HelpText<"Enable ConcurrentValue checking">;
228228

229+
def enable_experimental_flow_sensitive_concurrent_captures :
230+
Flag<["-"], "enable-experimental-flow-sensitive-concurrent-captures">,
231+
HelpText<"Enable flow-sensitive concurrent captures">;
232+
229233
def enable_resilience : Flag<["-"], "enable-resilience">,
230234
HelpText<"Deprecated, use -enable-library-evolution instead">;
231235
}

lib/Frontend/CompilerInvocation.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
387387
Args.hasArg(OPT_enable_experimental_concurrency);
388388
Opts.EnableExperimentalConcurrentValueChecking |=
389389
Args.hasArg(OPT_enable_experimental_concurrent_value_checking);
390+
Opts.EnableExperimentalFlowSensitiveConcurrentCaptures |=
391+
Args.hasArg(OPT_enable_experimental_flow_sensitive_concurrent_captures);
390392

391393
Opts.DisableImplicitConcurrencyModuleImport |=
392394
Args.hasArg(OPT_disable_implicit_concurrency_module_import);

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,21 +1596,28 @@ namespace {
15961596
return false;
15971597

15981598
// Check whether this is a local variable, in which case we can
1599-
// determine whether it was captured by value.
1599+
// determine whether it was safe to access concurrently.
16001600
if (auto var = dyn_cast<VarDecl>(value)) {
16011601
auto parent = mutableLocalVarParent[declRefExpr];
16021602

1603-
// If we have an immediate load of this variable, or it's a let,
1604-
// we will separately ensure that this variable is not modified.
1605-
if (!var->supportsMutation() || parent.dyn_cast<LoadExpr *>()) {
1603+
// If the variable is immutable, it's fine so long as it involves
1604+
// ConcurrentValue types.
1605+
//
1606+
// When flow-sensitive concurrent captures are enabled, we also
1607+
// allow reads, depending on a SIL diagnostic pass to identify the
1608+
// remaining race conditions.
1609+
if (!var->supportsMutation() ||
1610+
(ctx.LangOpts.EnableExperimentalFlowSensitiveConcurrentCaptures &&
1611+
parent.dyn_cast<LoadExpr *>())) {
16061612
return diagnoseNonConcurrentTypesInReference(
16071613
valueRef, getDeclContext(), loc,
16081614
ConcurrentReferenceKind::LocalCapture);
16091615
}
16101616

1611-
// Otherwise, we have concurrent mutation. Complain.
1617+
// Otherwise, we have concurrent access. Complain.
16121618
ctx.Diags.diagnose(
1613-
loc, diag::concurrent_mutation_of_local_capture,
1619+
loc, diag::concurrent_access_of_local_capture,
1620+
parent.dyn_cast<LoadExpr *>(),
16141621
var->getDescriptiveKind(), var->getName());
16151622
return true;
16161623
}

test/Concurrency/Runtime/actor_counters.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func runTest(numCounters: Int, numWorkers: Int, numIterations: Int) async {
6161
var workers: [Task.Handle<Void>] = []
6262
for i in 0..<numWorkers {
6363
workers.append(
64-
Task.runDetached {
64+
Task.runDetached { [counters] in
6565
usleep(UInt32.random(in: 0..<100) * 1000)
6666
await worker(
6767
identity: i, counters: counters, numIterations: numIterations,

test/Concurrency/actor_isolation.swift

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ extension MyActor {
142142
acceptConcurrentClosure {
143143
_ = self.text[0] // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent closure}}
144144
_ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent closure}}
145-
_ = localVar // okay
145+
_ = localVar // expected-error{{reference to captured var 'localVar' in concurrently-executing code}}
146146
localVar = 25 // expected-error{{mutation of captured var 'localVar' in concurrently-executing code}}
147147
_ = localConstant
148148
}
@@ -159,7 +159,7 @@ extension MyActor {
159159
@concurrent func localFn1() {
160160
_ = self.text[0] // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent function}}
161161
_ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent function}}
162-
_ = localVar // okay
162+
_ = localVar // expected-error{{reference to captured var 'localVar' in concurrently-executing code}}
163163
localVar = 25 // expected-error{{mutation of captured var 'localVar' in concurrently-executing code}}
164164
_ = localConstant
165165
}
@@ -168,7 +168,7 @@ extension MyActor {
168168
acceptClosure {
169169
_ = text[0] // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent function}}
170170
_ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent function}}
171-
_ = localVar // okay
171+
_ = localVar // expected-error{{reference to captured var 'localVar' in concurrently-executing code}}
172172
localVar = 25 // expected-error{{mutation of captured var 'localVar' in concurrently-executing code}}
173173
_ = localConstant
174174
}
@@ -324,14 +324,18 @@ func testGlobalRestrictions(actor: MyActor) async {
324324
// Global mutable state cannot be accessed.
325325
_ = mutableGlobal // expected-warning{{reference to var 'mutableGlobal' is not concurrency-safe because it involves shared mutable state}}
326326

327-
// Local mutable variables cannot be modified from concurrently-executing
327+
// Local mutable variables cannot be accessed from concurrently-executing
328328
// code.
329329
var i = 17
330330
acceptConcurrentClosure {
331-
_ = i
331+
_ = i // expected-error{{reference to captured var 'i' in concurrently-executing code}}
332332
i = 42 // expected-error{{mutation of captured var 'i' in concurrently-executing code}}
333333
}
334334
print(i)
335+
336+
acceptConcurrentClosure { [i] in
337+
_ = i
338+
}
335339
}
336340

337341
func f() {

test/Concurrency/concurrent_value_checking.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func testConcurrency() {
118118
}
119119
acceptConcurrent {
120120
print(x) // expected-warning{{cannot use let 'x' with a non-concurrent-value type 'NotConcurrent' from concurrently-executed code}}
121-
print(y) // expected-warning{{cannot use var 'y' with a non-concurrent-value type 'NotConcurrent' from concurrently-executed code}}
121+
print(y) // expected-error{{reference to captured var 'y' in concurrently-executing code}}
122122
}
123123
}
124124

test/Concurrency/concurrentfunction_capturediagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -enable-experimental-concurrency -verify -emit-sil %s -o - >/dev/null
1+
// RUN: %target-swift-frontend -enable-experimental-concurrency -enable-experimental-flow-sensitive-concurrent-captures -verify -emit-sil %s -o - >/dev/null
22

33
// REQUIRES: concurrency
44

test/SILGen/concurrent_functions.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-silgen %s -module-name test -swift-version 5 -enable-experimental-concurrency | %FileCheck %s
1+
// RUN: %target-swift-frontend -emit-silgen %s -module-name test -swift-version 5 -enable-experimental-concurrency -enable-experimental-flow-sensitive-concurrent-captures | %FileCheck %s
22
// REQUIRES: concurrency
33

44
func acceptsConcurrent(_: @escaping @concurrent () -> Int) { }

0 commit comments

Comments
 (0)