Skip to content

Commit 54dc437

Browse files
committed
[Concurrency] Ensure we get the right context for captured variables.
The declaration context of an explicitly-captured value is the context of the original, captured declaration itself... not the closure in which the value is captured. Account for this in data race checking, by tracking the effective capture context for such variables. This eliminates some erroneous complains about accesses to explicitly-captured variables in concurrent code. Part of rdar://74281361.
1 parent 5f9b898 commit 54dc437

File tree

2 files changed

+56
-2
lines changed

2 files changed

+56
-2
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,11 @@ namespace {
886886
SmallVector<const DeclContext *, 4> contextStack;
887887
SmallVector<ApplyExpr*, 4> applyStack;
888888

889+
/// Keeps track of the capture context of variables that have been
890+
/// explicitly captured in closures.
891+
llvm::SmallDenseMap<VarDecl *, TinyPtrVector<const DeclContext *>>
892+
captureContexts;
893+
889894
using MutableVarSource = llvm::PointerUnion<DeclRefExpr *, InOutExpr *>;
890895
using MutableVarParent = llvm::PointerUnion<InOutExpr *, LoadExpr *>;
891896

@@ -1134,6 +1139,13 @@ namespace {
11341139
if (isa<ObjCSelectorExpr>(expr))
11351140
return { false, expr };
11361141

1142+
// Track the capture contexts for variables.
1143+
if (auto captureList = dyn_cast<CaptureListExpr>(expr)) {
1144+
for (const auto &entry : captureList->getCaptureList()) {
1145+
captureContexts[entry.Var].push_back(captureList->getClosureBody());
1146+
}
1147+
}
1148+
11371149
return { true, expr };
11381150
}
11391151

@@ -1156,6 +1168,17 @@ namespace {
11561168
mutableLocalVarParent.erase(inoutExpr);
11571169
}
11581170

1171+
// Remove the tracked capture contexts.
1172+
if (auto captureList = dyn_cast<CaptureListExpr>(expr)) {
1173+
for (const auto &entry : captureList->getCaptureList()) {
1174+
auto &contexts = captureContexts[entry.Var];
1175+
assert(contexts.back() == captureList->getClosureBody());
1176+
contexts.pop_back();
1177+
if (contexts.empty())
1178+
captureContexts.erase(entry.Var);
1179+
}
1180+
}
1181+
11591182
return expr;
11601183
}
11611184

@@ -1526,6 +1549,21 @@ namespace {
15261549
llvm_unreachable("unhandled actor isolation kind!");
15271550
}
15281551

1552+
/// Find the innermost context in which this declaration was explicitly
1553+
/// captured.
1554+
const DeclContext *findCapturedDeclContext(ValueDecl *value) {
1555+
assert(value->isLocalCapture());
1556+
auto var = dyn_cast<VarDecl>(value);
1557+
if (!var)
1558+
return value->getDeclContext();
1559+
1560+
auto knownContexts = captureContexts.find(var);
1561+
if (knownContexts == captureContexts.end())
1562+
return value->getDeclContext();
1563+
1564+
return knownContexts->second.back();
1565+
}
1566+
15291567
/// Check a reference to a local or global.
15301568
bool checkNonMemberReference(
15311569
ConcreteDeclRef valueRef, SourceLoc loc, DeclRefExpr *declRefExpr) {
@@ -1552,7 +1590,7 @@ namespace {
15521590
// Check whether we are in a context that will not execute concurrently
15531591
// with the context of 'self'. If not, it's safe.
15541592
if (!mayExecuteConcurrentlyWith(
1555-
getDeclContext(), isolation.getLocalContext()))
1593+
getDeclContext(), findCapturedDeclContext(value)))
15561594
return false;
15571595

15581596
// Check whether this is a local variable, in which case we can

test/Concurrency/actor_isolation.swift

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,28 @@ extension MyActor {
139139
}
140140

141141
// Concurrent closures might run... concurrently.
142-
acceptConcurrentClosure {
142+
var otherLocalVar = 12
143+
acceptConcurrentClosure { [otherLocalVar] in
144+
defer {
145+
_ = otherLocalVar
146+
}
147+
143148
_ = self.text[0] // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent closure}}
144149
_ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent closure}}
145150
_ = localVar // expected-error{{reference to captured var 'localVar' in concurrently-executing code}}
146151
localVar = 25 // expected-error{{mutation of captured var 'localVar' in concurrently-executing code}}
147152
_ = localConstant
153+
154+
_ = otherLocalVar
155+
}
156+
otherLocalVar = 17
157+
158+
acceptConcurrentClosure { [weak self, otherLocalVar] in
159+
defer {
160+
_ = self?.actorIndependentVar
161+
}
162+
163+
_ = otherLocalVar
148164
}
149165

150166
// Escaping closures might run concurrently.

0 commit comments

Comments
 (0)