Remove ProtocolConformanceRef::getInherited() #11724
Merged
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
The SIL witness_method instruction takes a protocol method and a protocol conformance. IRGen lowers this as a load of a function pointer from a fixed offset in the witness table for the conformance. IRGen also handled the case where the conformance is to a protocol that refines the protocol containing the requirement, by calling ProtocolConformanceRef::getInherited() to map the conformance stored in the instruction to the correct conformance for the protocol requirement being called. It appears that this possibilty is not exercised through our test suite, and if this does come up it is better to fix SILGen and the optimizer to construct witness_method calls using exact conformances only instead. Furthermore, ProtocolConformanceRef::getInherited() only handles a single level of refinement. So for example, if a protocol R refines Q which refines R, we would crash when lowering a witness_method instruction that calls P.foo() with a conformance to R. Therefore, code that emits witness_method instructions with a conformance to a protocol that doesn't match the requirement was likely going to do the wrong thing in this case anyway. Moving the assertion earlier in the pipeline will help shake out these cases.
Instead of using an out parameter and a tuple return value, return everything in the tuple.
Given an existential type E, this creates a new canonical generic signature <T : E>. If E is a protocol composition containing members of protocol, class or AnyObject type, the resulting generic signature will split up the T : E requirement into a canonical and minimal set of conformance, superclass and layout requirements. If/when we implement generalized existentials, existential types will be defined in terms of an arbitrary set of generic requirements applied to a single generic parameter; at that point, this method will be replaced with a generic signature stored inside the type itself. For now, this will be used to simplify some logic in the SILOptimizer.
This addresses the unsupported case outlined in the FIXME.
We have an init_existential_{addr,ref} instruction which
constructs an existential of type R from a value whose
concrete type conforms to R.
Later on, we have a witness_method invoking a method of P,
where R refines Q refines P.
In order to give the witness_method instruction the correct
conformance after substituting the concrete type, we would
call ProtocolConformanceRef::getInherited().
However, this only supported a single level of protocol
refinement, so we would just bail out in the unsupported
case. Instead, this patch builds a SubstitutionMap from the
generic signature <T : R>, and uses the SubstitutionMap to
perform the conformance lookup of T to P.
…ited() This codepath consumed the result of getConformanceAndConcreteType(), which already maps inherited conformances to exact conformances.
Again, let's just assert that we're emitting a reference to a witness table for the exact conformance provided. The inherited conformance case would not have handled indirect refinement, where R refines Q refines P, and we're asked to emit a witness table reference for P given a conformance to R.
slavapestov
force-pushed
the
inherited-conformances-are-confusing
branch
from
9cb7a61 to
96822b1
Compare
|
@swift-ci Please smoke test |
|
@slavapestov Awesome! Thanks for fixing it so quickly, Slava! |
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.
In various places, we would sometimes be given a conformance of a concrete type to a protocol Q, and asked to produce a witness table entry for a requirement defined in protocol P, where Q refines P. The
ProtocolConformanceRef::getInherited()utility method was used to handle this case, except it only handled direct refinement. If Q refined P directly, it worked, but if Q refined P0 which refined P, it would fail with an assertion.It is better to either pass in the exact conformance needed, or use a SubstitutionMap to map the refined conformance to the base conformance using a ConformanceAccessPath. This PR refactors a couple of places in the SIL optimizer and IRGen to do things the right way, and then removes
ProtocolConformanceRef::getInherited()altogether.