Skip to content

Commit c4ea7bd

Browse files
committed
[Concurrency] Drop ActorIsolationRestriction::LocalCapture.
We are only interest in local captures in very narrow places, and they aren't actually related to actor isolation at all. Drop this case and check local captures in the one place they matter.
1 parent 54dc437 commit c4ea7bd

File tree

4 files changed

+69
-84
lines changed

4 files changed

+69
-84
lines changed

lib/Sema/TypeCheckConcurrency.cpp

+69-65
Original file line numberDiff line numberDiff line change
@@ -530,9 +530,8 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
530530
case DeclKind::Func:
531531
case DeclKind::Subscript: {
532532
// Local captures are checked separately.
533-
if (cast<ValueDecl>(decl)->isLocalCapture()) {
534-
return forLocalCapture(decl->getDeclContext());
535-
}
533+
if (cast<ValueDecl>(decl)->isLocalCapture())
534+
return forUnrestricted();
536535

537536
// 'let' declarations are immutable, so they can be accessed across
538537
// actors.
@@ -1280,7 +1279,6 @@ namespace {
12801279
auto isolation = ActorIsolationRestriction::forDeclaration(decl);
12811280
switch (isolation) {
12821281
case ActorIsolationRestriction::Unrestricted:
1283-
case ActorIsolationRestriction::LocalCapture:
12841282
case ActorIsolationRestriction::Unsafe:
12851283
break;
12861284
case ActorIsolationRestriction::CrossGlobalActor:
@@ -1564,13 +1562,80 @@ namespace {
15641562
return knownContexts->second.back();
15651563
}
15661564

1565+
/// Check a reference to a local capture.
1566+
bool checkLocalCapture(
1567+
ConcreteDeclRef valueRef, SourceLoc loc, DeclRefExpr *declRefExpr) {
1568+
auto value = valueRef.getDecl();
1569+
1570+
// Check whether we are in a context that will not execute concurrently
1571+
// with the context of 'self'. If not, it's safe.
1572+
if (!mayExecuteConcurrentlyWith(
1573+
getDeclContext(), findCapturedDeclContext(value)))
1574+
return false;
1575+
1576+
// Check whether this is a local variable, in which case we can
1577+
// determine whether it was safe to access concurrently.
1578+
if (auto var = dyn_cast<VarDecl>(value)) {
1579+
auto parent = mutableLocalVarParent[declRefExpr];
1580+
1581+
// If the variable is immutable, it's fine so long as it involves
1582+
// ConcurrentValue types.
1583+
//
1584+
// When flow-sensitive concurrent captures are enabled, we also
1585+
// allow reads, depending on a SIL diagnostic pass to identify the
1586+
// remaining race conditions.
1587+
if (!var->supportsMutation() ||
1588+
(ctx.LangOpts.EnableExperimentalFlowSensitiveConcurrentCaptures &&
1589+
parent.dyn_cast<LoadExpr *>())) {
1590+
return diagnoseNonConcurrentTypesInReference(
1591+
valueRef, getDeclContext(), loc,
1592+
ConcurrentReferenceKind::LocalCapture);
1593+
}
1594+
1595+
// Otherwise, we have concurrent access. Complain.
1596+
ctx.Diags.diagnose(
1597+
loc, diag::concurrent_access_of_local_capture,
1598+
parent.dyn_cast<LoadExpr *>(),
1599+
var->getDescriptiveKind(), var->getName());
1600+
return true;
1601+
}
1602+
1603+
if (auto func = dyn_cast<FuncDecl>(value)) {
1604+
if (func->isConcurrent())
1605+
return false;
1606+
1607+
func->diagnose(
1608+
diag::local_function_executed_concurrently,
1609+
func->getDescriptiveKind(), func->getName())
1610+
.fixItInsert(func->getAttributeInsertionLoc(false), "@concurrent ");
1611+
1612+
// Add the @concurrent attribute implicitly, so we don't diagnose
1613+
// again.
1614+
const_cast<FuncDecl *>(func)->getAttrs().add(
1615+
new (ctx) ConcurrentAttr(true));
1616+
return true;
1617+
}
1618+
1619+
// Concurrent access to some other local.
1620+
ctx.Diags.diagnose(
1621+
loc, diag::concurrent_access_local,
1622+
value->getDescriptiveKind(), value->getName());
1623+
value->diagnose(
1624+
diag::kind_declared_here, value->getDescriptiveKind());
1625+
return true;
1626+
}
1627+
15671628
/// Check a reference to a local or global.
15681629
bool checkNonMemberReference(
15691630
ConcreteDeclRef valueRef, SourceLoc loc, DeclRefExpr *declRefExpr) {
15701631
if (!valueRef)
15711632
return false;
15721633

15731634
auto value = valueRef.getDecl();
1635+
1636+
if (value->isLocalCapture())
1637+
return checkLocalCapture(valueRef, loc, declRefExpr);
1638+
15741639
switch (auto isolation =
15751640
ActorIsolationRestriction::forDeclaration(valueRef)) {
15761641
case ActorIsolationRestriction::Unrestricted:
@@ -1586,64 +1651,6 @@ namespace {
15861651
valueRef, loc, isolation.getGlobalActor(),
15871652
isolation == ActorIsolationRestriction::CrossGlobalActor);
15881653

1589-
case ActorIsolationRestriction::LocalCapture:
1590-
// Check whether we are in a context that will not execute concurrently
1591-
// with the context of 'self'. If not, it's safe.
1592-
if (!mayExecuteConcurrentlyWith(
1593-
getDeclContext(), findCapturedDeclContext(value)))
1594-
return false;
1595-
1596-
// Check whether this is a local variable, in which case we can
1597-
// determine whether it was safe to access concurrently.
1598-
if (auto var = dyn_cast<VarDecl>(value)) {
1599-
auto parent = mutableLocalVarParent[declRefExpr];
1600-
1601-
// If the variable is immutable, it's fine so long as it involves
1602-
// ConcurrentValue types.
1603-
//
1604-
// When flow-sensitive concurrent captures are enabled, we also
1605-
// allow reads, depending on a SIL diagnostic pass to identify the
1606-
// remaining race conditions.
1607-
if (!var->supportsMutation() ||
1608-
(ctx.LangOpts.EnableExperimentalFlowSensitiveConcurrentCaptures &&
1609-
parent.dyn_cast<LoadExpr *>())) {
1610-
return diagnoseNonConcurrentTypesInReference(
1611-
valueRef, getDeclContext(), loc,
1612-
ConcurrentReferenceKind::LocalCapture);
1613-
}
1614-
1615-
// Otherwise, we have concurrent access. Complain.
1616-
ctx.Diags.diagnose(
1617-
loc, diag::concurrent_access_of_local_capture,
1618-
parent.dyn_cast<LoadExpr *>(),
1619-
var->getDescriptiveKind(), var->getName());
1620-
return true;
1621-
}
1622-
1623-
if (auto func = dyn_cast<FuncDecl>(value)) {
1624-
if (func->isConcurrent())
1625-
return false;
1626-
1627-
func->diagnose(
1628-
diag::local_function_executed_concurrently,
1629-
func->getDescriptiveKind(), func->getName())
1630-
.fixItInsert(func->getAttributeInsertionLoc(false), "@concurrent ");
1631-
1632-
// Add the @concurrent attribute implicitly, so we don't diagnose
1633-
// again.
1634-
const_cast<FuncDecl *>(func)->getAttrs().add(
1635-
new (ctx) ConcurrentAttr(true));
1636-
return true;
1637-
}
1638-
1639-
// Concurrent access to some other local.
1640-
ctx.Diags.diagnose(
1641-
loc, diag::concurrent_access_local,
1642-
value->getDescriptiveKind(), value->getName());
1643-
value->diagnose(
1644-
diag::kind_declared_here, value->getDescriptiveKind());
1645-
return true;
1646-
16471654
case ActorIsolationRestriction::Unsafe:
16481655
return diagnoseReferenceToUnsafeGlobal(value, loc);
16491656
}
@@ -1794,9 +1801,6 @@ namespace {
17941801
memberRef, memberLoc, isolation.getGlobalActor(),
17951802
isolation == ActorIsolationRestriction::CrossGlobalActor);
17961803

1797-
case ActorIsolationRestriction::LocalCapture:
1798-
llvm_unreachable("Locals cannot be referenced with member syntax");
1799-
18001804
case ActorIsolationRestriction::Unsafe:
18011805
// This case is hit when passing actor state inout to functions in some
18021806
// cases. The error is emitted by diagnoseInOutArg.

lib/Sema/TypeCheckConcurrency.h

-17
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,6 @@ class ActorIsolationRestriction {
7777
/// Access to the declaration is unsafe in a concurrent context.
7878
Unsafe,
7979

80-
/// The declaration is a local entity whose capture could introduce
81-
/// data races. The context in which the local was defined is provided.
82-
LocalCapture,
83-
8480
/// References to this entity are allowed from anywhere, but doing so
8581
/// may cross an actor boundary if it is not on \c self.
8682
CrossActorSelf,
@@ -117,12 +113,6 @@ class ActorIsolationRestriction {
117113
public:
118114
Kind getKind() const { return kind; }
119115

120-
/// Retrieve the declaration context in which a local was defined.
121-
DeclContext *getLocalContext() const {
122-
assert(kind == LocalCapture);
123-
return data.localContext;
124-
}
125-
126116
/// Retrieve the actor class that the declaration is within.
127117
ClassDecl *getActorClass() const {
128118
assert(kind == ActorSelf || kind == CrossActorSelf);
@@ -154,13 +144,6 @@ class ActorIsolationRestriction {
154144
return result;
155145
}
156146

157-
/// Access is restricted to code running within the given local context.
158-
static ActorIsolationRestriction forLocalCapture(DeclContext *dc) {
159-
ActorIsolationRestriction result(LocalCapture);
160-
result.data.localContext = dc;
161-
return result;
162-
}
163-
164147
/// Accesses to the given declaration can only be made via this particular
165148
/// global actor or is a cross-actor access.
166149
static ActorIsolationRestriction forGlobalActor(

lib/Sema/TypeCheckDeclObjC.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,6 @@ static bool checkObjCActorIsolation(const ValueDecl *VD,
413413
// FIXME: Consider whether to limit @objc on global-actor-qualified
414414
// declarations.
415415
case ActorIsolationRestriction::Unrestricted:
416-
case ActorIsolationRestriction::LocalCapture:
417416
case ActorIsolationRestriction::Unsafe:
418417
return false;
419418
}

lib/Sema/TypeCheckProtocol.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -2755,7 +2755,6 @@ bool ConformanceChecker::checkActorIsolation(
27552755
}
27562756

27572757
case ActorIsolationRestriction::Unsafe:
2758-
case ActorIsolationRestriction::LocalCapture:
27592758
break;
27602759

27612760
case ActorIsolationRestriction::Unrestricted:

0 commit comments

Comments
 (0)