-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Normalize recursive types using Any #19003
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
Conversation
|
MichaReiser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
AlexWaygood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
|
Does this unblock the work on protocols? |
|
Will go ahead and merge this, but post-land review of the TypeVisitor change is welcome. (The callback accepted by |
Looks good, thank you! |
You might be able to get around that by having the Not a big deal either way, though! |
## Summary This just replaces one temporary solution to recursive protocols (the `SelfReference` mechanism) with another one (track seen types when recursively descending in `normalize` and replace recursive references with `Any`). But this temporary solution can handle mutually-recursive types, not just self-referential ones, and it's sufficient for the primer ecosystem and some other projects we are testing on to no longer stack overflow. The follow-up here will be to properly handle these self-references instead of replacing them with `Any`. We will also eventually need cycle detection on more recursive-descent type transformations and tests. ## Test Plan Existing tests (including recursive-protocol tests) and primer. Added mdtest for mutually-recursive protocols that stack-overflowed before this PR.
Yes, I considered interior mutability, but I don't think that's a good tradeoff. It sacrifices some performance (by adding the runtime reference tracking overhead of |
Summary
This just replaces one temporary solution to recursive protocols (the
SelfReferencemechanism) with another one (track seen types when recursively descending innormalizeand replace recursive references withAny). But this temporary solution can handle mutually-recursive types, not just self-referential ones, and it's sufficient for the primer ecosystem and some other projects we are testing on to no longer stack overflow.The follow-up here will be to properly handle these self-references instead of replacing them with
Any.We will also eventually need cycle detection on more recursive-descent type transformations and tests.
Test Plan
Existing tests (including recursive-protocol tests) and primer.
Added mdtest for mutually-recursive protocols that stack-overflowed before this PR.