Allow more circular type definitions #31828
Closed
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.
Checking
jl_types_equal
against a potential wrapper when trying to normalize type parameters can result in an infinite recursion for circular type definitions. E.g.:Notably, the former only works due to an invalid fast-path, making it also recurse infinitely in #25796 and #31807 (without appropriate countermeasures). The culprit is that
jl_types_equal
needs torename_unionall
here, which tries to re-instantiate the same type (with another typevar), which comes to same test again. The obvious counter-measure is to avoidjl_types_equal
and instead use a heuristic that may miss opportunities for normalization, but will not lead to recursion. As a starter, I'm usingobviously_egal
here, which fixes above issue, but may be too restrictive, e.g.Tests still pass, but we probably miss some optimization opportunities this way. Though I have no idea how much impact this actually has, so it might even be ok like this.
Discussion of this started in #25796, but this issue is somewhat independent, so for the sake of more focused discussions, I'm opening this draft PR.