-
Notifications
You must be signed in to change notification settings - Fork 43
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
Implement finish (step-out) operation in debugger. #618
Conversation
ed2045c
to
e4f633e
Compare
Can I have a question? |
src/debugger/Debugger.cpp
Outdated
if (record->isDeclarativeEnvironmentRecord()) { | ||
DeclarativeEnvironmentRecord* declarativeRecord = record->asDeclarativeEnvironmentRecord(); | ||
if (declarativeRecord->isFunctionEnvironmentRecord()) { |
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.
These if conditional statements could be merged into one statement.
BTW is it possible to put a breakpoint inside eval code except function invoked during eval execution? |
When you are in the eval code (after parsing), you can put breakpoints in there because they are regular functions (they have their own source code and byte code). However the IDE part is not simple for that, because the name of eval code is the same and any number of evals can be executed. If the IDE opens a separate editor for each eval code, it can insert single breakpoints for them. E.g.:
This code creates twenty different functions with the same source code. Each has their own byte code and source code, so the IDE can set separate breakpoints for each of them (the IDE can open twenty editor tabs, but these editors are not backed by real files). Opening hundreds of tabs for the user can be difficult to understand, but that is another story. Is this what you wanted to know? |
Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
e4f633e
to
1245761
Compare
Regarding yield / async the debugger should stop after the next instruction (similar to return) unless you prefer something different. There was an unhandled code path which prevented it, but I fixed it (and a new test is added). |
Sounds cool! |
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.
LGTM
Stopped at tools/debugger/tests/do_finish.js:38 | ||
(escargot-debugger) b do_finish.js:41 | ||
Breakpoint 2 at tools/debugger/tests/do_finish.js:41 (in gen() at line:40, col:1) | ||
(escargot-debugger) f | ||
Stopped at breakpoint:2 tools/debugger/tests/do_finish.js:41 (in gen() at line:40, col:1) |
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 have one more question.
For the above case, when "finish" is executed on the global code debugger runs the program until the next breakpoint encountered inside gen()
instead of finishing the program.
But from the description, "finish" is defined as follow.
finish : Continue running until just after function in the selected stack frame returns.
I'm wondering what is the correct operation when another breakpoint is met during the execution of "finish" command. Should we stop at the breakpoint or ignore it?
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.
In my experience breakpoints always takes priority over anything. So if a breakpoint is encountered the execution stops regardless of the last command.
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: we can do this differently of course, but this is my experience with debuggers.
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.
Your answer makes everything clear! Thanks.
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.
LGTM
#617 should land first.
The finish operation requires to find the ExecutionState of the current function. The best idea I have is getting the FunctionEnvironment of the current state, and check its parent states until I find a different FunctionEnvironment. It works with
try-catch
andwith
, but does not work witheval
, i.e. the debugger does not just exit from eval, but it also exits from the function called theeval
. However this can be considered correct depending on how you want to handle this case.Anyway if you have a better suggestion for this operation, please let me know.