Skip to content

Rework diagnostics for conformance isolation failures #80190

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

Conversation

DougGregor
Copy link
Member

A protocol conformance can be ill-formed due to isolation mismatches
between witnesses and requirements, or with associated conformances.
Previously, such failures would be emitted as a number of separate
errors (downgraded to warnings in Swift 5), one for each witness and
potentially an extra for associated conformances. The rest was a
potential flood of diagnostics that was hard to sort through.

Collect all of the isolation-related problems for a given conformance
together and produce a single error (downgraded to a warning when
appropriate) that describes the overall issue. That error will have up
to three notes suggesting specific courses of action:

  • Isolating the conformance (when the experimental feature is enabled)
  • Marking the witnesses as 'nonisolated' where needed
  • Marking the conformance as '@preconcurrency'

The diagnostic also has notes to point out the witnesses/associated
conformances that have isolation problems. There is a new educational
note that also describes these options.

The diagnostic looks like this:

13 | 
14 | @MainActor
15 | final class C: P {
   |             |  |- error: conformance of 'C' to protocol 'P' crosses into main actor-isolated code and can cause data races [#conformance-isolation]
   |             |  |- note: isolate this conformance to the main actor with '@MainActor'
   |             |  `- note: turn data races into runtime errors with '@preconcurrency'
   |             `- note: mark all declarations used in the conformance 'nonisolated'
16 |   var v = 3
   |       `- note: main actor-isolated property 'v' cannot be used to satisfy nonisolated requirement from protocol 'P'
17 |   
18 |   init() {}
19 | 
20 |   func f() { }
   |        `- note: main actor-isolated instance method 'f()' cannot be used to satisfy nonisolated requirement from protocol 'P'
21 |   func g() { }
   |        `- note: main actor-isolated instance method 'g()' cannot be used to satisfy nonisolated requirement from protocol 'P'
22 | }

We give the same treatment to missing 'distributed' on witnesses to a
distributed protocol.

When diagnosing an isolation mismatch between a requirement and witness,
we would produce notes on the requirement itself suggesting the addition of
`async`. This is almost never what you want to do, and is often so far
away from the actual conforming type as to be useless. Remove this note,
and the non-function fallback that just points at the requirement, because
they are unhelpful.

This is staging for a rework of the way we deal with conformance-level
actor isolation problems.
… structure

Rather than emitting isolation-related diagnostics for conformances, such
as witnesses that have incompatible isolation or associated conformances
that are differently isolated, capture all of those diagnostics in a
single side table and diagnose them all together.

This refactoring doesn't change the way we actually diagnose the
issue. That comes next.
A protocol conformance can be ill-formed due to isolation mismatches
between witnesses and requirements, or with associated conformances.
Previously, such failures would be emitted as a number of separate
errors (downgraded to warnings in Swift 5), one for each witness and
potentially an extra for associated conformances. The rest was a
potential flood of diagnostics that was hard to sort through.

Collect all of the isolation-related problems for a given conformance
together and produce a single error (downgraded to a warning when
appropriate) that describes the overall issue. That error will have up
to three notes suggesting specific courses of action:
* Isolating the conformance (when the experimental feature is enabled)
* Marking the witnesses as 'nonisolated' where needed
*

The diagnostic also has notes to point out the witnesses/associated
conformances that have isolation problems. There is a new educational
note that also describes these options.

We give the same treatment to missing 'distributed' on witnesses to a
distributed protocol.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor DougGregor enabled auto-merge March 21, 2025 05:13
@DougGregor DougGregor merged commit 8fd02d7 into swiftlang:main Mar 21, 2025
3 checks passed
@DougGregor DougGregor deleted the conformance-isolation-diagnostics branch March 21, 2025 15:25
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.

1 participant