-
Notifications
You must be signed in to change notification settings - Fork 375
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
WIP: inject closure-calls for loop externs in closures #1780
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1780 +/- ##
===========================================
- Coverage 55.94% 45.75% -10.20%
===========================================
Files 422 460 +38
Lines 64269 69518 +5249
===========================================
- Hits 35956 31808 -4148
- Misses 25483 35049 +9566
+ Partials 2830 2661 -169 ☔ View full report in Codecov by Sentry. |
// nothing to transform | ||
return n, TRANS_CONTINUE | ||
} | ||
panic("!!! ITS HAPPENING (insert ron paul jazz hands) !!!") |
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.
NOTE: I haven't actually tested the running of this logic, yet.
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.
"TestSearchEfficiency" is triggered by this but the tests have been passing because the closure is called synchronously. good sign that it is picked up by this panic, and probably a good one to focus on first.
// nothing to transform | ||
return n, TRANS_CONTINUE | ||
} | ||
panic("!!! ITS HAPPENING (insert ron paul jazz hands) !!!") |
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.
ROFL! Epic panic message.
working on #2429 instead. |
This is an alternative implementation to #1768 and #1585 for #1135.
This should be much more performant overall, as loop externs are rare.
UPDATE: noticed that "TestSearchEfficiency" is affected but the runtime is not because the closure is called synchronously.