Skip to content

Commit a70ed0f

Browse files
committed
permit isolated-member references in actor deinits with warning
This capability was available in Swift 5.5, but with flow-isolation it's not safe to do so in general. When I initially implemented flow-isolation, I intended for it to not break too much existing code, and emit warnings about things changing in Swift 6. But I missed this case, where we have just a simple method call in a deinit, which is likely to be common: ```swift actor A { func cleanup() { ... } deinit { cleanup() } } ``` Instead of rejecting that call to `cleanup`, we now warn that it's not going to be allowed in Swift 6, because `cleanup` is isolated and the deinit is not.
1 parent 5cc85b4 commit a70ed0f

File tree

4 files changed

+51
-5
lines changed

4 files changed

+51
-5
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4479,7 +4479,9 @@ NOTE(note_distributed_actor_system_conformance_missing_adhoc_requirement,none,
44794479
ERROR(override_implicit_unowned_executor,none,
44804480
"cannot override an actor's 'unownedExecutor' property that wasn't "
44814481
"explicitly defined", ())
4482-
4482+
ERROR(actor_isolated_from_deinit,none,
4483+
"actor-isolated %0 %1 can not be referenced from a non-isolated deinit",
4484+
(DescriptiveDeclKind, DeclName))
44834485
ERROR(actor_isolated_non_self_reference,none,
44844486
"actor-isolated %0 %1 can not be "
44854487
"%select{referenced|mutated|used 'inout'}2 "

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2676,12 +2676,14 @@ namespace {
26762676
/// \param refCxt the context in which the member reference happens.
26772677
/// \param baseActor the actor referenced in the base of the member access.
26782678
/// \param member the declaration corresponding to the accessed member.
2679+
/// \param memberLoc the source location of the reference to the member.
26792680
///
26802681
/// \returns true iff the member access is permitted in Sema because it will
26812682
/// be verified later by flow-isolation.
26822683
bool checkedByFlowIsolation(DeclContext const *refCxt,
26832684
ReferencedActor &baseActor,
2684-
ValueDecl const *member) {
2685+
ValueDecl const *member,
2686+
SourceLoc memberLoc) {
26852687

26862688
// base of member reference must be `self`
26872689
if (!baseActor.isActorSelf())
@@ -2717,6 +2719,18 @@ namespace {
27172719
if (var->hasStorage() && var->isInstanceMember())
27182720
return true;
27192721

2722+
// In Swift 5, we were allowing all members to be referenced from a
2723+
// deinit, but that will not be valid in Swift 6+, so warn about it.
2724+
if (!refCxt->getASTContext().isSwiftVersionAtLeast(6)) {
2725+
if (isa<DestructorDecl>(fnDecl) && member->isInstanceMember()) {
2726+
auto &diags = refCxt->getASTContext().Diags;
2727+
diags.diagnose(memberLoc, diag::actor_isolated_from_deinit,
2728+
member->getDescriptiveKind(),
2729+
member->getName()).warnUntilSwiftVersion(6);
2730+
return true;
2731+
}
2732+
}
2733+
27202734
return false;
27212735
}
27222736

@@ -2820,7 +2834,7 @@ namespace {
28202834
// access an isolated member on `self`. If that case applies, then we
28212835
// can skip checking.
28222836
if (checkedByFlowIsolation(getDeclContext(), isolatedActor,
2823-
member))
2837+
member, memberLoc))
28242838
return false;
28252839

28262840
// An escaping partial application of something that is part of

test/Concurrency/actor_isolation.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -844,7 +844,7 @@ actor SomeActorWithInits {
844844
let _ = self.nonSendable // OK only through typechecking, not SIL.
845845

846846
defer {
847-
isolated() // expected-error{{actor-isolated instance method 'isolated()' can not be referenced from a non-isolated context}}
847+
isolated() // expected-warning{{actor-isolated instance method 'isolated()' can not be referenced from a non-isolated deinit; this is an error in Swift 6}}
848848
mutableState += 1 // okay
849849
nonisolated()
850850
}
@@ -860,7 +860,7 @@ actor SomeActorWithInits {
860860
}
861861

862862

863-
func isolated() { } // expected-note 9 {{calls to instance method 'isolated()' from outside of its actor context are implicitly asynchronous}}
863+
func isolated() { } // expected-note 8 {{calls to instance method 'isolated()' from outside of its actor context are implicitly asynchronous}}
864864
nonisolated func nonisolated() {}
865865
}
866866

test/Concurrency/flow_isolation.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,7 @@ actor EscapeArtist {
584584
actor Ahmad {
585585
nonisolated func f() {}
586586
var prop: Int = 0
587+
var computedProp: Int { 10 }
587588

588589
init(v1: Void) {
589590
Task.detached { self.f() } // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}}
@@ -596,6 +597,14 @@ actor Ahmad {
596597
f()
597598
prop += 1 // expected-warning {{cannot access property 'prop' here in non-isolated initializer; this is an error in Swift 6}}
598599
}
600+
601+
deinit {
602+
// expected-warning@+2 {{actor-isolated property 'computedProp' can not be referenced from a non-isolated deinit; this is an error in Swift 6}}
603+
// expected-note@+1 {{after accessing property 'computedProp', only non-isolated properties of 'self' can be accessed from a deinit}}
604+
let x = computedProp
605+
606+
prop = x // expected-warning {{cannot access property 'prop' here in deinitializer; this is an error in Swift 6}}
607+
}
599608
}
600609

601610
@available(SwiftStdlib 5.5, *)
@@ -622,4 +631,25 @@ actor Rain {
622631

623632
defer { _ = hollerBack(self) }
624633
}
634+
635+
deinit {
636+
x = 1
637+
}
638+
}
639+
640+
@available(SwiftStdlib 5.5, *)
641+
actor DeinitExceptionForSwift5 {
642+
var x: Int = 0
643+
644+
func cleanup() {
645+
x = 0
646+
}
647+
648+
deinit {
649+
// expected-warning@+2 {{actor-isolated instance method 'cleanup()' can not be referenced from a non-isolated deinit; this is an error in Swift 6}}
650+
// expected-note@+1 {{after calling instance method 'cleanup()', only non-isolated properties of 'self' can be accessed from a deinit}}
651+
cleanup()
652+
653+
x = 1 // expected-warning {{cannot access property 'x' here in deinitializer; this is an error in Swift 6}}
654+
}
625655
}

0 commit comments

Comments
 (0)