Skip to content

[6.0][Concurrency] Don't ignore mismatching isolation for overrides of Clang-imported superclass methods. #74238

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
Jun 9, 2024

Conversation

hborla
Copy link
Member

@hborla hborla commented Jun 9, 2024

  • Explanation: The actor isolation checker was ignoring mismatching isolation for overrides if the superclass is imported from ObjC. However, mismatching isolation for overrides is invalid, and there's no reason to ignore this mistake for ObjC-imported superclasses. This change is staged in behind complete concurrency checking. This change also removes a bogus fix-it to add global actor isolation to an override if it synchronously calls a global actor isolated method, which won't work.
  • Scope: Only impacts overrides where the superclass is imported from ObjC, where the override is isolated to a global actor and the superclass method is not.
  • Issue: rdar://124409467, adding global actor erroneously allowed when overriding an ObjC class's method #72238
  • Risk: Low; the change is staged in as a warning under -strict-concurrency=complete.
  • Testing: Added a new test.
  • Reviewer: TBD
  • Main branch PR: [Concurrency] Don't ignore mismatching isolation for overrides of Clang-imported superclass methods. #74237

Clang-imported superclass methods.

(cherry picked from commit 128a8bc)
@hborla hborla requested a review from a team as a code owner June 9, 2024 00:11
@hborla
Copy link
Member Author

hborla commented Jun 9, 2024

@swift-ci please test

@hborla hborla merged commit d9fefc8 into swiftlang:release/6.0 Jun 9, 2024
5 checks passed
@hborla hborla deleted the 6.0-override-isolation branch June 9, 2024 20:41
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