Closed Bug 1172572 Opened 5 years ago Closed 1 year ago

debugger will not step in to a function after stepping out of another function

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(firefox70 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: bugzilla, Assigned: jlast)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [debugger-mvp])

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150525141253

Steps to reproduce:

function a() {
  b();
}
function b() {
  console.log("b");
}
function c() {
  console.log("c");
}
debugger;
c(a());

In the code above, the debugger is paused at the last line (the line after the debugger keyword). 
I want to step into function c.
I select Step In.
The debugger steps in to function a, which is not the one I wanted, so I select Step Out until I am back at the same line after the debugger keyword.
I press Step In.

Tested in Firefox 41.0a1 (2015-06-08)


Actual results:

The debugger stepped over the call to function c.


Expected results:

The debugger should step in to function c.
Component: Untriaged → Developer Tools: Debugger
In this bug, with the current fx-team, what I see is that after stepping in
to "b", I have to click "step out" twice.  This causes "c" to execute (clearly
visible in the console).

However, if I "step out" once and then "step in", I end up back at the
"c(a())" line; and then stepping in again takes me to "c".


Once the patch from bug 1013219 (and associated bugs) is applied, the behavior is better,
but perhaps still not ideal.

With that patch, clicking "step out" in "b" takes me to the "}" at the end of b.
Then, "step in" at that point takes me directly to "c".

If I click "step out" twice from "b", I end up at the end of the outermost
function (I wrapped the whole thing in an outer function so I could debug it
repeatedly a bit more easily).
With my latest patch what happens is:

* step into a
* step out -> stopped at "}" that closes a
* Here, step out -> outer "}" (I wrapped everything in an outer function)
  However, if instead of step out you do step in or step over
  -> back to the line c(a())
* Then step in goes to c

So, pretty good.

I tend to think that "step out" when paused at the "}" should
act like a single step.  At least, that is what I was expecting.
Was this bug addressed by the other stepping-related patches that we landed, or is this bug still open?
Flags: needinfo?(ttromey)
This one at least depends on bug 1013219, which I haven't finished yet.
If the behavior described in comment #2 is what we want, then this could
be a dup.  Otherwise there is more work to be done.
Depends on: 1013219
Flags: needinfo?(ttromey)
Product: Firefox → DevTools

STR

  1. go to https://appear.in/firefox-debugger
  2. step-in

ER: inside of C
AR: inside of A

I think this is because when a call is at the beginning of a statement we place the breakpoint at the statement..

NOTE: chrome places the bp at the call.

NOTE: I can be persuaded that what we're doing is right because we stop at the beginning, but now that we have column bps i feel like we can be more opinionated. e.g. if the user wants to stop at a() they can.

Blocks: dbg-68
Priority: -- → P2
Version: 38 Branch → unspecified

Did you post the wrong link in your STR? :P

Flags: needinfo?(jlaster)

ha looks like the case :)
https://dbg-step-in.glitch.me/

Flags: needinfo?(jlaster)
Blocks: dbg-69
No longer blocks: dbg-68
Blocks: dbg-control

Yeah this seems more like a question of what we want to do than anything. This is expected given the rules we laid out for always treating the first opcode in an statement-level expression as the place to stop, but if we want to change that to special-case it if the first position is a call and used the call opcode instead that seems fine to me.

All of the SpiderMonkey position stuff is gone from my head again so I'd have to dig in to see how complicated it would be, but it might not be too bad?

I think using the call opcode makes sense.

One alternative we could consider would be to put the call marker on the ( when a function call is specifically at the start like this so when you do

   foo(bar());
// ^  ^^

so the first ^ would be the expression start, the second would be the call to foo and the third would be the call to bar. That way we still keep the current behavior of always having the expressions tart be a breakable location. Personally I do like that we currently do that. That said, I don't feel super strongly necessarily.

that seems reasonable, i don't think people will notice that call has that special case. I'm ambivalent between these two options.

Whiteboard: [debugger-mvp]
Blocks: 1552532
No longer blocks: 1552532
Assignee: nobody → jlaster

Logan, do you think this might be a nice first C++ bug for me?

Flags: needinfo?(lsmyth)
Flags: needinfo?(lsmyth)
Status: NEW → ASSIGNED
Blocks: dbg-70
No longer blocks: dbg-69
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9973154c79e5
teach debugger to step into calls at the beginning of statements. r=jimb
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
Attachment #9079173 - Attachment is obsolete: true
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0d73f35c5025
teach debugger to step into calls at the beginning of statements (part 2). r=loganfsmyth
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c81079d1320
teach debugger to step into calls at the beginning of statements (part 2). r=loganfsmyth
QA Whiteboard: [qa-70b-p2]
Flags: needinfo?(jlaster)
You need to log in before you can comment on or make changes to this bug.