Skip to content

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

Conversation

DougGregor
Copy link
Member

  • 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.
@DougGregor DougGregor requested a review from a team as a code owner June 3, 2025 06:46
@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test macOS

Copy link
Contributor

@slavapestov slavapestov left a 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 {
Copy link
Contributor

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)
Copy link
Contributor

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

@DougGregor
Copy link
Member Author

I'll address Slava's comments via a follow-up PR on main

@DougGregor DougGregor merged commit ed29e1f into swiftlang:release/6.2 Jun 4, 2025
5 checks passed
@DougGregor DougGregor deleted the break-isolated-conformance-reference-cycle-6.2 branch June 4, 2025 17:26
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.

3 participants