Bug 21073 – JS Debugger doesn't handle 'for' loops correctly

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Kevin McCullough]

9/24/08 1:55 PM Kevin McCullough:
* SUMMARY
The behavior of VS and XCode is as follows.

Assume a breakpoint is on a line of code that contains a single 'for' loop with all three statements; the initializer, condition, and increment.
1) When execution first comes to the for loop, it breaks and none of the statements have executed yet
2) If the user chooses to continue, then the breakpoint is never hit again and execution does not break again
3) If the user chooses to step over, then execution goes to the next line (so the initializer and the condition execute)
4) If the user chooses to step into, then execution goes into the first statement (initializer) and then the second (condition)
5) If the user steps through the loop and back to the line with the 'for' on it, they can step into the statements as they execute (i.e. the increment and condition)

Currently we support 1 and 2 because 'for' loops emit a debug hook at the beginning of emit code; however emitting a debug hook for each statement will solve 3-5, but regress on 1 and 2 since we will break each time we hit any statement on that line.

Tim had a suggestion where we need to differentiate debug hooks between breaking per-line and breaking per-statement, currently we only support breaking per statement.

<rdar://problem/6243923>

[Alexandru Chiculita]

Created attachment 23848 [details]
The patch is a fix for correctly emitting debugging hooks in nodes.cpp. 

After looking in the grammar.y file I think you can have a better level of control of when the debugger must stop. It's as simple as adding emitDebugHooks in each debuggable statement emitCode method. That will, also, remove the need for two more virtual function calls (isBlock and isLoop).

Also changed the position of some debug hooks in ForNode, ForInNode and DoWhileNode.

[Kevin McCullough]

Comment on attachment 23848 [details]
The patch is a fix for correctly emitting debugging hooks in nodes.cpp. 

Looks good however be aware that this will not fully fix the issue described in this bug.

It will keep #1 & #3, but regress #2 in that subsequent "continues" will hit the break point each time through the loop, where in VS and XCode it would not.

It will improve #4 & #5 in that you can now step over to the line with the conditional of the for loop but you cannot step into the initialize or increment statements of the for loop.

This patch would be fine without fixing all of these issues, however it would also be good if we had a manual test that exercised each of the statement nodes so that we could see the behavior and check against regressions.

Please make a test.

[Horia Olaru]

Created attachment 27611 [details]
updated patch including manual tests

This is the above patch, submitted by Alex, brought up to date with the repository code.
It also includes four manual tests for the 'for', 'for-in', 'while' and 'do-while' loops.

The only above step that regresses is #2, as would be expected considering the statement based debug hooks and the line based breakpoints.

[Kevin McCullough]

(In reply to comment #3)
> The only above step that regresses is #2, as would be expected considering the
> statement based debug hooks and the line based breakpoints.
> 

So when a user hits continue on a for loop, do they have to press it 2 times (either once for the initializer and once for the conditional, or once for the increment and once for the conditional)?

I would hate for a user to have to press continue 20 times for a loop that only executes 10 times.  Especially if they are used to the behavior where they would only have to press continue once; after the first time the breakpoint is hit.

[Horia Olaru]

> So when a user hits continue on a for loop, do they have to press it 2 times
> (either once for the initializer and once for the conditional, or once for the
> increment and once for the conditional)?

No! It simply regresses #2 - when hitting continue, the debugger will pause again on the 'for' line, after going through the loop once. This is opposed to the expected behavior that the breakpoint on the 'for' is not hit again.

Otherwise put, if the loop executes 10 time, the user has to click continue 10 times. The user should only have to click continue once to skip the breakpoint at the beginning.

[Kevin McCullough]

> Otherwise put, if the loop executes 10 time, the user has to click continue 10
> times. The user should only have to click continue once to skip the breakpoint
> at the beginning.
> 

I actually prefer that behavior in other IDEs but I don't get to call the shots.  Does the subsequent breakpoints all happen before any statement in the 'for' loop?  I.E. before the condition or increment or does it occur between the increment and condition statements?

r+ on this btw.

[Horia Olaru]

> I actually prefer that behavior in other IDEs but I don't get to call the
> shots.  Does the subsequent breakpoints all happen before any statement in the
> 'for' loop?  I.E. before the condition or increment or does it occur between
> the increment and condition statements?

Yes! You can step into first the increment, and then the condition. Stepping over will step over both.

[Timothy Hatcher]

Landed in r41342.