-
Notifications
You must be signed in to change notification settings - Fork 10.5k
GSB: Formalize the old hack where we rebuild a signature that had redundant conformance requirements #36454
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
slavapestov
merged 4 commits into
swiftlang:main
from
slavapestov:rebuild-signature-without-redundant-conformances
Mar 18, 2021
Merged
GSB: Formalize the old hack where we rebuild a signature that had redundant conformance requirements #36454
slavapestov
merged 4 commits into
swiftlang:main
from
slavapestov:rebuild-signature-without-redundant-conformances
Mar 18, 2021
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16f894a
to
509592f
Compare
This depends on #36460 first landing. |
509592f
to
23863e5
Compare
@swift-ci Please smoke test |
@swift-ci Please test source compatibility |
23863e5
to
b19f22a
Compare
@swift-ci Please smoke test |
@swift-ci Please test source compatibility |
@swift-ci Please smoke test macOS |
…rement implied a redundant requirement Recall the meaning of an explicit requirement here: - If the requirement is part of a root SCC, it is redundant unless it is the 'best' requirement from that root SCC. - If the requirement is part of a non-root SCC, it is always redundant. Instead of computing the set of redundant requirements, we build a mapping. The value in the mapping stores the set of root requirements that imply the given redundant requirement. This mapping is computed by traversing the graph from each root, recording which components can be reached from each root. For now, I'm using this to fix rdar://problem/65263302. After fixing that bug, this will also allow us to radically simplify the various callers of checkConstraintList().
b19f22a
to
f1e517e
Compare
…undant conformance requirements When constructing a generic signature, any redundant explicit requirements are dropped from the final signature. We would assume this operation is idempotent, that is, building a new GenericSignatureBuilder from the resulting minimized signature produces an equivalent GenericSignatureBuilder to the original one. Unfortunately, this is not true in the case of conformance requirements. Namely, if a conformance requirement is made redundant by a superclass or concrete same-type requirement, then dropping the conformance requirement changes the canonical type computation. For example, consider the following: public protocol P { associatedtype Element } public class C<O: P>: P { public typealias Element = O.Element } public func toe<T, O, E>(_: T, _: O, _: E, _: T.Element) where T : P, O : P, O.Element == T.Element, T : C<E> {} In the generic signature of toe(), the superclass requirement 'T : C<E>' implies the conformance requirement 'T : P' because C conforms to P. However, the presence of the conformance requirement makes it so that T.Element is the canonical representative, so previously this signature was minimized down to: <T : C<E>, O : P, T.Element == O.Element> If we build the signature again from the above requirements, then we see that T.Element is no longer the canonical representative; instead, T.Element canonicalizes as E.Element. For this reason, we must rebuild the signature to get the correct canonical type computation. I realized that this is not an artifact of incorrect design in the current GSB; my new rewrite system formalism would produce the same result. Rather, it is a subtle consequence of the specification of our minimization algorithm, and therefore it must be formalized in this manner. We used to sort-of do this with the HadAnyRedundantRequirements hack, but it was both overly broad (we only need to rebuild if a conformance requirement was implied by a superclass or concrete same-type requirement) and not sufficient (when rebuilding, we need to strip any bound associated types from our requirements to ensure the canonical type anchors are re-computed). Fixes rdar://problem/65263302, rdar://problem/75010156, rdar://problem/75171977.
bab3ed4
to
504d4f5
Compare
@swift-ci Please smoke test |
@swift-ci Please test source compatibility |
@swift-ci Please smoke test macOS |
@swift-ci Please smoke test Linux |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When constructing a generic signature, any redundant explicit requirements
are dropped from the final signature.
We would assume this operation is idempotent, that is, building a new
GenericSignatureBuilder from the resulting minimized signature produces
an equivalent GenericSignatureBuilder to the original one.
Unfortunately, this is not true in the case of conformance requirements.
Namely, if a conformance requirement is made redundant by a superclass
or concrete same-type requirement, then dropping the conformance
requirement changes the canonical type computation.
For example, consider the following:
In the generic signature of toe(), the superclass requirement 'T : C'
implies the conformance requirement 'T : P' because C conforms to P.
However, the presence of the conformance requirement makes it so that
T.Element is the canonical representative, so previously this signature
was minimized down to:
If we build the signature again from the above requirements, then we
see that T.Element is no longer the canonical representative; instead,
T.Element canonicalizes as E.Element.
For this reason, we must rebuild the signature to get the correct
canonical type computation.
I realized that this is not an artifact of incorrect design in the
current GSB; my new rewrite system formalism would produce the same
result. Rather, it is a subtle consequence of the specification of our
minimization algorithm, and therefore it must be formalized in this
manner.
We used to sort-of do this with the HadAnyRedundantRequirements hack,
but it was both overly broad (we only need to rebuild if a conformance
requirement was implied by a superclass or concrete same-type
requirement) and not sufficient (when rebuilding, we need to strip any
bound associated types from our requirements to ensure the canonical
type anchors are re-computed).
Fixes rdar://problem/65263302, rdar://problem/75010156,
rdar://problem/75171977.