-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[5.5] Fix SILGen of access to isolated static properties #38190
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
[5.5] Fix SILGen of access to isolated static properties #38190
Conversation
When accessing a static property isolated to a global-actor, such as MainActor, a `hop_to_executor` was not being emitted. This was caught by an assertion I had left to catch such cases, but of course in release builds this will lead to incorrect SIL, etc. This issue revealed some other problems with how the implementation was done for other kinds of accesses starting from a static property, e.g., emitting a redundant hop for the same access. This was fixed by modelling the actor-isolation placed into a component as only being accessible by consuming it from the component. This prevents the emission of the hops more than once. Thus, the regression test was updated to catch unexpected hop_to_executor instructions. Resolves rdar://78292384
Added an implicit CHECK-NOT to catch any unexpected hop_to_executor instructions in the output of SILGen.
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed this before on main, looks good and is important to fix, LGTM 👍
Build failed |
@swift-ci please test Linux platform |
@swift-ci please nominate |
Build failed |
@swift-ci please clean test and merge |
@swift-ci please clean test linux platform |
Explanation: Accesses to actor-isolated static properties, such as those isolated to the MainActor, were being miscompiled, which led to missing synchronization and "hopping" to the main thread.
Risk: Low.
Original PRs: #38114
Radar/SR Issue: rdar://78292384
Reviewed By: Konrad Malawski
Testing: Regression tests are included