-
Notifications
You must be signed in to change notification settings - Fork 474
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
AVM: fix early eval exits for Debugger #4719
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4719 +/- ##
==========================================
- Coverage 54.37% 53.38% -1.00%
==========================================
Files 414 414
Lines 53504 53501 -3
==========================================
- Hits 29095 28559 -536
- Misses 22002 22497 +495
- Partials 2407 2445 +38
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
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.
@jannotti Thanks for fixing - I confirmed TestDryrunEarlyExit
fails in master
as intended.
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.
The changes make sense
When the Debugger is set, we were using a defer() to set the debugstate on every exit from eval(). Unfortunately, we exited for some conditions before the debugState was setup, so there was a nil pointer dereference while trying to communicate the error to the Debugger.
This moves those error checks down until after the debugState is setup. It also has the tiny advantage of not setting up the defer() unless it is needed (cx.Debugger is set)
(This contains some final specs changes approved by Foundation as well.)