Skip to content

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

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
merged 4 commits into from
Jul 13, 2024

Conversation

hborla
Copy link
Member

@hborla hborla commented Jul 10, 2024

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.

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.

Resolves: rdar://124423524

hborla added 2 commits July 10, 2024 11:58
…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.
…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.
@hborla hborla force-pushed the unavailable-superclass-conformance branch from b44aac2 to 85b66d1 Compare July 12, 2024 03:33
@hborla hborla marked this pull request as ready for review July 12, 2024 03:34
Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tricky impl hah, looks okey

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.
@hborla hborla force-pushed the unavailable-superclass-conformance branch from cd9f333 to b139770 Compare July 12, 2024 06:03
@hborla
Copy link
Member Author

hborla commented Jul 12, 2024

@swift-ci please smoke test

@hborla
Copy link
Member Author

hborla commented Jul 12, 2024

@swift-ci please test source compatibility

@hborla
Copy link
Member Author

hborla commented Jul 12, 2024

@swift-ci please smoke test

@hborla
Copy link
Member Author

hborla commented Jul 12, 2024

@swift-ci please test source compatibility

@hborla
Copy link
Member Author

hborla commented Jul 13, 2024

The macOS smoke test is failing due to a driver test failure unrelated to this change. Looks like a revert is being tested at swiftlang/swift-driver#1662

@hborla
Copy link
Member Author

hborla commented Jul 13, 2024

@swift-ci please smoke test macOS

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.

2 participants