Summary: | JS Debugger doesn't handle 'for' loops correctly | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kevin McCullough <kmccullough> | ||||||
Component: | Web Inspector (Deprecated) | Assignee: | Timothy Hatcher <timothy> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achicu, olaru, timothy | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Kevin McCullough
2008-09-24 14:02:52 PDT
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.
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.
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.
(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. > 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.
> 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.
> 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.
|