Skip to content

Commit a6c40c7

Browse files
committed
Fix two bugs with the isolation of defer bodies.
The first bug is that we weren't computing isolation correctly for nested defers. This is an unlikely pattern of code, but it's good to fix. The second bug is that getActorIsolationOfContext was looking through defers, but getActorIsolation itself was not. This was causing defer bodies to be emitted in SILGen without an isolation parameter, which meant that #isolation could not possibly provide the right value. Fixing this involves teaching SILGen that non-async functions can have nonisolated(nonsending) isolation, but that's relatively straightforward. This commit doesn't fix #isolation or adequately test SILGen, but that'll be handled in a follow-up.
1 parent 2bd4c8c commit a6c40c7

File tree

4 files changed

+36
-5
lines changed

4 files changed

+36
-5
lines changed

include/swift/AST/Decl.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3241,6 +3241,9 @@ class ValueDecl : public Decl {
32413241
/// `AbstractStorageDecl`, returns `false`.
32423242
bool isAsync() const;
32433243

3244+
/// Returns whether this function represents a defer body.
3245+
bool isDeferBody() const;
3246+
32443247
private:
32453248
bool isObjCDynamic() const {
32463249
return isObjC() && isDynamic();

lib/AST/Decl.cpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11811,6 +11811,12 @@ PrecedenceGroupDecl *InfixOperatorDecl::getPrecedenceGroup() const {
1181111811
nullptr);
1181211812
}
1181311813

11814+
bool ValueDecl::isDeferBody() const {
11815+
if (auto fn = dyn_cast<FuncDecl>(this))
11816+
return fn->isDeferBody();
11817+
return false;
11818+
}
11819+
1181411820
bool FuncDecl::isDeferBody() const {
1181511821
return getBaseIdentifier() == getASTContext().getIdentifier("$defer");
1181611822
}
@@ -12004,12 +12010,16 @@ ActorIsolation swift::getActorIsolationOfContext(
1200412010
getClosureActorIsolation) {
1200512011
auto &ctx = dc->getASTContext();
1200612012
auto dcToUse = dc;
12007-
// Defer bodies share actor isolation of their enclosing context.
12008-
if (auto FD = dyn_cast<FuncDecl>(dcToUse)) {
12009-
if (FD->isDeferBody()) {
12010-
dcToUse = FD->getDeclContext();
12011-
}
12013+
12014+
// Defer bodies share the actor isolation of their enclosing context.
12015+
// We don't actually have to do this check here because
12016+
// getActorIsolation does consider it already, but it's nice to
12017+
// avoid some extra request evaluation in a trivial case.
12018+
while (auto FD = dyn_cast<FuncDecl>(dcToUse)) {
12019+
if (!FD->isDeferBody()) break;
12020+
dcToUse = FD->getDeclContext();
1201212021
}
12022+
1201312023
if (auto *vd = dyn_cast_or_null<ValueDecl>(dcToUse->getAsDecl()))
1201412024
return getActorIsolation(vd);
1201512025

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6374,6 +6374,14 @@ static bool shouldSelfIsolationOverrideDefault(
63746374

63756375
static InferredActorIsolation computeActorIsolation(Evaluator &evaluator,
63766376
ValueDecl *value) {
6377+
// Defer bodies share the actor isolation of their enclosing context.
6378+
if (value->isDeferBody()) {
6379+
return {
6380+
getActorIsolationOfContext(value->getDeclContext()),
6381+
IsolationSource()
6382+
};
6383+
}
6384+
63776385
// If this declaration has actor-isolated "self", it's isolated to that
63786386
// actor.
63796387
if (evaluateOrDefault(evaluator, HasIsolatedSelfRequest{value}, false)) {

test/Concurrency/actor_defer.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ func testNonDefer_negative() {
3838
// CHECK-NEXT: function_ref
3939
// CHECK-NEXT: apply
4040

41+
@MainActor func testGlobalActor_nested_positive() {
42+
defer {
43+
defer {
44+
requiresMainActor()
45+
}
46+
doSomething()
47+
}
48+
doSomething()
49+
}
50+
4151
#if NEGATIVES
4252
// expected-note @+1 {{add '@MainActor' to make global function 'testGlobalActor_negative()' part of global actor 'MainActor'}}
4353
func testGlobalActor_negative() {

0 commit comments

Comments
 (0)