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

Conversation

kavon
Copy link
Member

@kavon kavon commented Feb 9, 2022

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:

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.

resolves rdar://88666799

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.
@kavon
Copy link
Member Author

kavon commented Feb 9, 2022

@swift-ci please smoke test and merge

1 similar comment
@kavon
Copy link
Member Author

kavon commented Feb 10, 2022

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 5ecfe2a into swiftlang:main Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants