-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Split conformance isolation request to eliminate a reference cycle #81934
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
Split conformance isolation request to eliminate a reference cycle #81934
Conversation
DougGregor
commented
Jun 3, 2025
- Explanation: Inference of conformance isolation needs to check whether all of the witnesses are nonisolated. However, witness checking looks at conformance isolation, causing a reference cycle in Swift 6 when inference of isolated conformances is enabled. This change breaks the reference cycle.
- Scope: Limited to the conformance checker when isolated conformance inference or default main actor are enabled.
- Issues: rdar://152461344
- Original PRs: Split conformance isolation request to eliminate a reference cycle #81933
- Risk: Low. Behavior only changes when one of the aforementioned upcoming features are enabled.
- Testing: CI, project demonstrating the issue.
- Reviewers:
Inference of conformance isolation needs to check whether all of the witnesses are nonisolated. However, witness checking looks at conformance isolation. To break this reference cycle, split the conformance isolation request into two requests: a "raw" request that looks at explicitly-specified isolation, and the existing one that also performs inference. The existing one builds on the "raw" one, as does a separate path for the conformance checker. Fixes rdar://152461344.
@swift-ci please test |
@swift-ci please test macOS |
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.
Two optional but recommended suggestions, but LGTM for 6.2
Bits.ProtocolConformance.IsComputedNonisolated = false; | ||
} | ||
|
||
bool isRawConformanceInferred() const { |
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.
isRawIsolationInferred maybe?
if (!rootNormal) | ||
return ActorIsolation::forNonisolated(false); | ||
|
||
if (conformance != rootNormal) |
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.
It might be better to do this 'unwrapping' of specialized conformances before evaluating the request, to avoid polluting the cache with tons of duplicates that only differ by substitution map
I'll address Slava's comments via a follow-up PR on main |