[Runtime] Fix infinite recursion in protocol conformance checking on class Sub: Super<Sub>. #38515
+119
−47
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
mikeash
force-pushed
the
fix-protocol-conformance-superclass-recursion
branch
2 times, most recently
from
93fcb02 to
ca3c1d3
Compare
mikeash
force-pushed
the
fix-protocol-conformance-superclass-recursion
branch
from
ca3c1d3 to
0397ab3
Compare
mikeash
changed the title
|
@swift-ci please test |
…class Sub: Super<Sub>.
Conformance checks now walk the superclass chain in two stages: stage 1 only walks superclasses that have already been instantiated. When there's a negative result and there's an uninstantiated superclass, then stage 2 will walk the uninstantiated superclasses.
The infinite recursion would occur in this scenario:
class Super<T: P> {}
class Sub: Super<Sub>, P {}
Instantiating the metadata for Super requires looking up the conformance for Sub: P. Conformance checking for Sub would instantiate the metadata for Super to check for a Super: P conformance.
The compiler does not allow the conformance to come from a superclass in this situation. This does not compile:
class Super<T: P>: P {}
class Sub: Super<Sub> {}
Therefore it's not necessary to look at Super when finding the conformance for Sub: P in this particular case. The trick is knowing when to skip Super.
We do need to instantiate Super in the general case, otherwise we can get false negatives. This was addressed in a80fe85, which walks the full superclass chain during conformance checks, even if the superclass has not yet been instantiated. Unfortunately, that causes this infinite recursion.
This fix modifies that fix to make superclass instantiation conditional. The result is the ability to choose between the old algorithm (which skipped uninstantiated superclasses, albeit somewhat by accident) and the new one. A small wrapper then runs the check with the old algorithm, and then only if the old algorithm fails and there is an uninstantiated superclass, it runs with the new one.
Uninstantiated superclasses are uncommon and transient (you only run into this while metadata is in the process of being constructed) so 99.9999% of the time we'll just run the first stage and be done, and performance should remain the same as before.
rdar://80532245
|
Build failed |
mikeash
force-pushed
the
fix-protocol-conformance-superclass-recursion
branch
from
0397ab3 to
b37252d
Compare
|
@swift-ci please test |
|
Build failed |
|
@swift-ci please test linux platform |
|
Build failed |
|
@swift-ci please smoke test macOS platform |
|
@swift-ci please smoke test OS X platform |
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.
Conformance checks now walk the superclass chain in two stages: stage 1 only walks superclasses that have already been instantiated. When there's a negative result and there's an uninstantiated superclass, then stage 2 will walk the uninstantiated superclasses.
The infinite recursion would occur in this scenario:
Instantiating the metadata for Super requires looking up the conformance for Sub: P. Conformance checking for Sub would instantiate the metadata for Super to check for a Super: P conformance.
The compiler does not allow the conformance to come from a superclass in this situation. This does not compile:
Therefore it's not necessary to look at Super when finding the conformance for Sub: P in this particular case. The trick is knowing when to skip Super.
We do need to instantiate Super in the general case, otherwise we can get false negatives. This was addressed in a80fe85, which walks the full superclass chain during conformance checks, even if the superclass has not yet been instantiated. Unfortunately, that causes this infinite recursion.
This fix modifies that fix to make superclass instantiation conditional. The result is the ability to choose between the old algorithm (which skipped uninstantiated superclasses, albeit somewhat by accident) and the new one. A small wrapper then runs the check with the old algorithm, and then only if the old algorithm fails and there is an uninstantiated superclass, it runs with the new one.
Uninstantiated superclasses are uncommon and transient (you only run into this while metadata is in the process of being constructed) so 99.9999% of the time we'll just run the first stage and be done, and performance should remain the same as before.
rdar://80532245