Skip to content

GSB: When looking at protocol refinements, only consider other sources on 'Self' #37466

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

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented May 17, 2021

In a protocol requirement signature, a conformance requirement on 'Self'
can only be considered redundant if it is derived entirely from other
sources only involving 'Self' as a subject type.

What this means in practice is that we will never try to build
a witness table for an inherited conformance from an associated
conformance.

This is important since witness tables for inherited conformances
are realized eagerly before the witness table for the original
conformance, and allowing more complex requirement sources for
inherited conformances could introduce cycles at runtime.

What used to happen is that we would emit a bogus 'redundant conformance'
warning if a refined protocol conformance was also reachable via a
same-type requirement involving 'Self'. Fixing this warning by removing
the requirement could produce code that type checked, however it could
crash at runtime.

In addition to the runtime issue, such 'overly minimized' protocols
could also break name lookup at compile-time, since name lookup
only consults the inheritance clause of a protocol declaration instead
of computing its generic signature to determine the set of refined
protocols.

Now, we will no longer emit the bogus redundant conformance warnings.
Instead, we produce a warning if a protocol is 'overly minimized'.

Since the 'overly minimized' protocols that were obtained by fixing
the bogus warning would in some cases still work, we cannot diagnose
the new violation as an error, to preserve source and ABI compatibility.

Currently this warning fires for the SIMDScalar protocol in the
standard library, and _ErrorCodeProtocol in the Foundation overlay.
We should see if we can fix the protocols in an ABI-compatible way,
or just add a hack to muffle the warning.

Fixes https://bugs.swift.org/browse/SR-8198 / rdar://problem/77358480,
https://bugs.swift.org/browse/SR-10941 / rdar://problem/77358477,
https://bugs.swift.org/browse/SR-11670 / rdar://problem/77358476.

@slavapestov slavapestov force-pushed the ban-implicit-protocol-refinement branch 2 times, most recently from 55c3d51 to 7457fd7 Compare May 18, 2021 01:42
@slavapestov slavapestov requested a review from DougGregor May 18, 2021 01:43
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov requested a review from rjmccall May 18, 2021 01:53
…s on 'Self'

In a protocol requirement signature, a conformance requirement on 'Self'
can only be considered redundant if it is derived entirely from other
sources only involving 'Self' as a subject type.

What this means in practice is that we will never try to build
a witness table for an inherited conformance from an associated
conformance.

This is important since witness tables for inherited conformances
are realized eagerly before the witness table for the original
conformance, and allowing more complex requirement sources for
inherited conformances could introduce cycles at runtime.

What used to happen is that we would emit a bogus 'redundant conformance'
warning if a refined protocol conformance was also reachable via a
same-type requirement involving 'Self'. Fixing this warning by removing
the requirement could produce code that type checked, however it could
crash at runtime.

In addition to the runtime issue, such 'overly minimized' protocols
could also break name lookup at compile-time, since name lookup
only consults the inheritance clause of a protocol declaration instead
of computing its generic signature to determine the set of refined
protocols.

Now, we will no longer emit the bogus redundant conformance warnings.
Instead, we produce a warning if a protocol is 'overly minimized'.

Since the 'overly minimized' protocols that were obtained by fixing
the bogus warning would in some cases still work, we cannot diagnose
the new violation as an error, to preserve source and ABI compatibility.

Currently this warning fires for the SIMDScalar protocol in the
standard library, and _ErrorCodeProtocol in the Foundation overlay.
We should see if we can fix the protocols in an ABI-compatible way,
or just add a hack to muffle the warning.

Fixes https://bugs.swift.org/browse/SR-8198 / rdar://problem/77358480,
https://bugs.swift.org/browse/SR-10941 / rdar://problem/77358477,
https://bugs.swift.org/browse/SR-11670 / rdar://problem/77358476.
@slavapestov slavapestov force-pushed the ban-implicit-protocol-refinement branch from 7457fd7 to 3e1e9fa Compare May 18, 2021 01:57
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov merged commit a6df5ac into swiftlang:main May 18, 2021
slavapestov added a commit to slavapestov/swift that referenced this pull request Nov 10, 2021
…Rule()

With this change the RequirementMachine's minimization behavior with
protocol refinement rules matches the GenericSignatureBuilder.

See swiftlang#37466 for a full explanation
of what's going on here.
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