Skip to content

[6.0][ConformanceLookup] Don't allow skipping inherited unavailable conformances in favor of explicit available ones. #75223

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

hborla
Copy link
Member

@hborla hborla commented Jul 13, 2024

  • Explanation: This change fixes an issue where the compiler did not issue redundant conformance warnings when a client tries to add a retroactive @unchecked Sendable conformance when the library has already added an unavailable Sendable conformance.

    Note that this change still does not warn if the original unavailable conformance is declared with @_nonSendable due to the way @_nonSendable expansion happens during implicit Sendable derivation, after no explicit conformance has been found.

  • Scope: Only impacts redundant conformances where at least one of the conformances is unavailable.

  • Issues: rdar://124423524

  • Original PRs: [ConformanceLookup] Don't allow skipping inherited unavailable conformances in favor of explicit available ones. #75135

  • Risk: Low; the redundant conformance diagnostics added by this change are downgraded to warnings unconditionally. This allows libraries to stage in unavailable conformances to Sendable later on, and clients that have already added a retroactive @unchecked Sendable conformance will receive a warning when they rebuild. Conformance lookup will still choose the available conformance to avoid breaking source compatibility in places where the client code uses the type in a way that requires an available conformance to Sendable.

  • Testing: Added new tests. Passed source compatibility on the main PR.

  • Reviewers: @ktoso

hborla added 4 commits July 12, 2024 19:15
…mances

in favor of explicit available ones.

The type checker does not support the notion of multiple protocol
conformances; there can only be one conformance, and if that conformance
is unavailable, you cannot specify your own available conformance. This
is important for Sendable checking; if a framework specifies that a type
is explicitly not Sendable with an unavailable Sendable conformance,
clients cannot ignore Sendable violations involving that type. If a
superclass wants to allow subclasses to add a Sendable conformance, it
should not declare an unavailable Sendable conformance.

(cherry picked from commit 7356fe8)
…rom the

defining module, and diagnose redundant Sendable conformances.

We still allow re-stating inherited unchecked Sendable conformances in
subclasses because inherited Sendable conformances are surprising when
they opt out of static checking. Otherwise, warning on redundant Sendable
conformances nudges programmers toward cleaning up unnecessary retroactive
Sendable conformances over time as libraries incrementally add the
conformances directly.

(cherry picked from commit 85b66d1)
Sendable conformances for source compatibility.

If conformance lookup always prefers the conformance from the defining module,
libraries introducing unavailable Sendable conformances can break source in
clients that declare retroactive Sendable conformances. Instead, still prefer
the available conformance, and always diagnose the client conformance as
redundant (as a warning). Then, when the retroactive conformance is removed,
the errors will surface, so the programmer has to take explicit action to
experience the source break.

(cherry picked from commit b139770)
@hborla hborla requested a review from a team as a code owner July 13, 2024 02:17
@hborla
Copy link
Member Author

hborla commented Jul 13, 2024

@swift-ci please test

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