Skip to content

[region-isolation] When determining isolation from a class_method, check for global actor isolation first. #75016

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jul 7, 2024

Before this change in the following code, we would say that message is isolated to the actor instead of the global actor isolation of the actor's method:

class Message { ... }

actor MessageHolder {
  @MainActor func hold(_ message: Message) { ... }
}

@MainActor
func sendMessage() async {
    let messageHolder = MessageHolder()
    let message = Message()
    // We identified messageHolder.hold as being MessageHolder isolated
    // instead of main actor isolated.
    messageHolder.hold(message)
    Task { @MainActor in print(message) }
}

I also used this as an opportunity to simplify the logic in this part of the code. Specifically, I had made it so that multiple interesting cases were handled in the same conditional statement in a manner that it made it hard to know which cases were actually being handled and why it was correct. Now I split that into two separate if statements with comments that make it clear what we are actually trying to pattern match against.

rdar://130980933

…eck for global actor isolation first.

Before this change in the following code, we would say that message is isolated to the actor instead of the global actor isolation of the actor's method:

```swift
class Message { ... }

actor MessageHolder {
  @mainactor func hold(_ message: Message) { ... }
}

@mainactor
func sendMessage() async {
    let messageHolder = MessageHolder()
    let message = Message()
    // We identified messageHolder.hold as being MessageHolder isolated
    // instead of main actor isolated.
    messageHolder.hold(message)
    Task { @mainactor in print(message) }
}
```

I also used this as an opportunity to simplify the logic in this part of the
code. Specifically, I had made it so that multiple interesting cases were
handled in the same conditional statement in a manner that it made it hard to
know which cases were actually being handled and why it was correct. Now I split
that into two separate if statements with comments that make it clear what we
are actually trying to pattern match against.

rdar://130980933
@gottesmm
Copy link
Contributor Author

gottesmm commented Jul 7, 2024

@swift-ci smoke test

@gottesmm gottesmm merged commit 9f8e5e8 into swiftlang:main Jul 7, 2024
3 checks passed
@gottesmm gottesmm deleted the pr-d4640c7e2919faea7bc7eb96ebd8af643108e2d8 branch July 7, 2024 16:58
Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late LGTM, looks ok

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