-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix crash checking spread element in loop #31098
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
@typescript-bot perf test this please |
Heya @andrewbranch, I've started to run the perf test suite on this PR at 89497fc. You can monitor the build here. It should now contribute to this PR's status checks. Update: The results are in! |
@weswigham didn't you have some idea to squash this class of bugs once and for all. I remember trying something a year or two ago that seemed to work but was scary in its own right. Until then, switching away from checkExpressionCached is the right thing. Edit: Can't find it. Maybe I didn't make a PR for it? |
@andrewbranch Here they are:Comparison Report - master..31098
System
Hosts
Scenarios
|
@sandersn @weswigham is this good to go or did you have any suggestions? |
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.
I'm good with the change but get @weswigham to sign off too, since he's looked at this more recently than I have.
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.
didn't you have some idea to squash this class of bugs once and for all. I remember trying something a year or two ago that seemed to work but was scary in its own right.
A structured cache for resolved types, this way each time we enter into a new context we push a new "layer" and if it's finalized (accepted as a correct and final result) we merge the cache layer down. It's not something we currently want because of the implied complexity, so these kinds of ad hoc fixes are all we have right now. T.T
Fixes #30804
I don't really understand this, but was tipped off by #25228. I'm open to suggestions for a better fix for this class of crash.