Skip to content

permit isolated-member references in actor deinits with warning #41305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4479,7 +4479,9 @@ NOTE(note_distributed_actor_system_conformance_missing_adhoc_requirement,none,
ERROR(override_implicit_unowned_executor,none,
"cannot override an actor's 'unownedExecutor' property that wasn't "
"explicitly defined", ())

ERROR(actor_isolated_from_deinit,none,
"actor-isolated %0 %1 can not be referenced from a non-isolated deinit",
(DescriptiveDeclKind, DeclName))
ERROR(actor_isolated_non_self_reference,none,
"actor-isolated %0 %1 can not be "
"%select{referenced|mutated|used 'inout'}2 "
Expand Down
18 changes: 16 additions & 2 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2676,12 +2676,14 @@ namespace {
/// \param refCxt the context in which the member reference happens.
/// \param baseActor the actor referenced in the base of the member access.
/// \param member the declaration corresponding to the accessed member.
/// \param memberLoc the source location of the reference to the member.
///
/// \returns true iff the member access is permitted in Sema because it will
/// be verified later by flow-isolation.
bool checkedByFlowIsolation(DeclContext const *refCxt,
ReferencedActor &baseActor,
ValueDecl const *member) {
ValueDecl const *member,
SourceLoc memberLoc) {

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

// In Swift 5, we were allowing all members to be referenced from a
// deinit, but that will not be valid in Swift 6+, so warn about it.
if (!refCxt->getASTContext().isSwiftVersionAtLeast(6)) {
if (isa<DestructorDecl>(fnDecl) && member->isInstanceMember()) {
auto &diags = refCxt->getASTContext().Diags;
diags.diagnose(memberLoc, diag::actor_isolated_from_deinit,
member->getDescriptiveKind(),
member->getName()).warnUntilSwiftVersion(6);
return true;
}
}

return false;
}

Expand Down Expand Up @@ -2820,7 +2834,7 @@ namespace {
// access an isolated member on `self`. If that case applies, then we
// can skip checking.
if (checkedByFlowIsolation(getDeclContext(), isolatedActor,
member))
member, memberLoc))
return false;

// An escaping partial application of something that is part of
Expand Down
4 changes: 2 additions & 2 deletions test/Concurrency/actor_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ actor SomeActorWithInits {
let _ = self.nonSendable // OK only through typechecking, not SIL.

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


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

Expand Down
30 changes: 30 additions & 0 deletions test/Concurrency/flow_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ actor EscapeArtist {
actor Ahmad {
nonisolated func f() {}
var prop: Int = 0
var computedProp: Int { 10 }

init(v1: Void) {
Task.detached { self.f() } // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}}
Expand All @@ -596,6 +597,14 @@ actor Ahmad {
f()
prop += 1 // expected-warning {{cannot access property 'prop' here in non-isolated initializer; this is an error in Swift 6}}
}

deinit {
// expected-warning@+2 {{actor-isolated property 'computedProp' can not be referenced from a non-isolated deinit; this is an error in Swift 6}}
// expected-note@+1 {{after accessing property 'computedProp', only non-isolated properties of 'self' can be accessed from a deinit}}
let x = computedProp

prop = x // expected-warning {{cannot access property 'prop' here in deinitializer; this is an error in Swift 6}}
}
}

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

defer { _ = hollerBack(self) }
}

deinit {
x = 1
}
}

@available(SwiftStdlib 5.5, *)
actor DeinitExceptionForSwift5 {
var x: Int = 0

func cleanup() {
x = 0
}

deinit {
// expected-warning@+2 {{actor-isolated instance method 'cleanup()' can not be referenced from a non-isolated deinit; this is an error in Swift 6}}
// expected-note@+1 {{after calling instance method 'cleanup()', only non-isolated properties of 'self' can be accessed from a deinit}}
cleanup()

x = 1 // expected-warning {{cannot access property 'x' here in deinitializer; this is an error in Swift 6}}
}
}