Skip to content

Commit

Permalink
[trace-view] Fixed skipping same line instructions (#19812)
Browse files Browse the repository at this point in the history
## Description 

This PR implements a more principled way to handle skipping instructions
located on the same source code line for a smoother user experience.

The main issue is handling call instructions, particularly if more than
one of them is located on the same source code line. If not handled
properly, the following effects could take place
- if a call instruction was preceded by another instruction on the same
source code line, when executing `step` action, the user would have to
click `step` button twice on the same line to actually step into the
call making an impression that the debugger is somehow stuck (or at
least stuttering)
- if more than one call instruction was on the same line, two things
could happen:
- executing the `next` action would not skip over all calls but only
over a single one instead (the user would have to click `next` button
multiple times to get to the next line)
- executing the `step` would cause control flow to immediately enter the
subsequent call on the same line instead of waiting for another `step`
command from the user

This PR also removes code related to stepping backwards in the trace as
it's not being used nor maintained making a false impression that it's
simply disabled and ready to roll

## Test plan 

Tested manually that the scenarios described above are handled correctly
  • Loading branch information
awelc authored Oct 11, 2024
1 parent cc2a65f commit 54dc4d0
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 198 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,7 @@ export class MoveDebugSession extends LoggingDebugSession {
): void {
let terminate = false;
try {
terminate = this.runtime.step(
/* next */ true,
/* stopAtCloseFrame */ false,
/* nextLineSkip */ true
);
terminate = this.runtime.step(/* next */ true, /* stopAtCloseFrame */ false);
} catch (err) {
response.success = false;
response.message = err instanceof Error ? err.message : String(err);
Expand All @@ -400,11 +396,7 @@ export class MoveDebugSession extends LoggingDebugSession {
): void {
let terminate = false;
try {
terminate = this.runtime.step(
/* next */ false,
/* stopAtCloseFrame */ false,
/* nextLineSkip */ true
);
terminate = this.runtime.step(/* next */ false, /* stopAtCloseFrame */ false);
} catch (err) {
response.success = false;
response.message = err instanceof Error ? err.message : String(err);
Expand All @@ -420,7 +412,7 @@ export class MoveDebugSession extends LoggingDebugSession {
_args: DebugProtocol.StepOutArguments
): void {
try {
const steppedOut = this.runtime.stepOut();
const steppedOut = this.runtime.stepOut(/* next */ false);
if (!steppedOut) {
logger.log(`Cannot step out`);
}
Expand All @@ -431,46 +423,13 @@ export class MoveDebugSession extends LoggingDebugSession {
this.sendResponse(response);
}

protected stepBackRequest(
response: DebugProtocol.StepBackResponse,
_args: DebugProtocol.StepBackArguments
): void {
try {
const steppedBack = this.runtime.stepBack();
if (!steppedBack) {
logger.log(`Cannot step back`);
}
} catch (err) {
response.success = false;
response.message = err instanceof Error ? err.message : String(err);
}
this.sendResponse(response);
}

protected continueRequest(
response: DebugProtocol.ContinueResponse,
_args: DebugProtocol.ContinueArguments
): void {
let terminate = false;
try {
terminate = this.runtime.continue(/* reverse */ false);
} catch (err) {
response.success = false;
response.message = err instanceof Error ? err.message : String(err);
}
if (terminate) {
this.sendEvent(new TerminatedEvent());
}
this.sendResponse(response);
}

protected reverseContinueRequest(
response: DebugProtocol.ReverseContinueResponse,
_args: DebugProtocol.ReverseContinueArguments
): void {
let terminate = false;
try {
terminate = this.runtime.continue(/* reverse */ true);
terminate = this.runtime.continue();
} catch (err) {
response.success = false;
response.message = err instanceof Error ? err.message : String(err);
Expand Down
Loading

0 comments on commit 54dc4d0

Please sign in to comment.