When source mapping, a user line is not the same as a line that spidermonkey sees and so often all code is on a single line (minified code) and so only a couple offsets are exposed by the API, but we really want to set a breakpoint on an arbitrary expression inside the line (whatever is source mapped to). Example: Minified code: function foo(){var x=3,y=4,z=9;} Prettified and source mapped: function foo() { var x = 3, y = 4, z = 9; } We should be able to get an offset for |y = 4| if a user sets a breakpoint on that line, but the Debugger API won't give us one.

Not sure exactly how related this is, but here is |dis| of the ugly and pretty versions: js> function foo(){var x=3,y=4,z=9;} function foo(){var x=3,y=4,z=9;} js> dis(foo) dis(foo) flags: loc op ----- -- main: 00000: int8 3 00002: setlocal 0 00006: pop 00007: int8 4 00009: setlocal 1 00013: pop 00014: int8 9 00016: setlocal 2 00020: pop 00021: retrval Source notes: ofs line pc delta desc args ---- ---- ----- ------ -------- ------ 0: 3 21 [ 21] xdelta 1: 3 21 [ 0] colspan 28 js> function bar() { function bar() { var x = 3, var x = 3, y = 4, y = 4, z = 9; z = 9; } } js> dis(bar) dis(bar) flags: loc op ----- -- main: 00000: int8 3 00002: setlocal 0 00006: pop 00007: int8 4 00009: setlocal 1 00013: pop 00014: int8 9 00016: setlocal 2 00020: pop 00021: retrval Source notes: ofs line pc delta desc args ---- ---- ----- ------ -------- ------ 0: 5 0 [ 0] newline 1: 6 7 [ 7] newline 2: 7 14 [ 7] newline 3: 8 21 [ 7] colspan 3 My understanding is that this reflects what the Debugger API is giving us as well.

I was hoping this would be a quick fix in |DebuggerScript_getAllColumnOffsets| where I could just optionally loosen the offset filtering. However, it appears that for function foo(){var x=3,y=4,z=9;} every offset's position is reported as line 1, column 0 so that won't work. The location information comes from the source notes via |BytecodeRangeWithPosition| so it seems that the source notes need to have finer granularity. Eddy, how difficult is this likely? Pointers/tips for implementation? Thanks.

The parse tree has source position information on every node. It's js/src/frontend/BytecodeEmitter.cpp that builds the source codes as it builds the byte code stream. See the calls to, for example, NewSrcNote.

Err, "builds the source codes" should be "builds the source notes". I think "expression granularity" is probably not quite right; something like x = y + z contains *five* expressions. You probably mean largest-containing-expression. But then that's almost exactly the same as "statement": you just need an exception for 'for (A;B;C) D' , saying that each of A, B, C, and D should be distinguished. This will have some memory impact.

There seem to be 2 types of expressions that need better source notes ATM: 1. multiple variable declaration+initialization with the same var 2. multiple expressions separated with commas (something minifiers often do) Trying to find other cases we need to handle as well.

This patch adds more source notes for: * Each declared variable in a single var statement. * Each sub-expression in a comma joined expression (eg "e(),f(),g(),h()"). * Each property of array and object literals. Try push: https://tbpl.mozilla.org/?tree=Try&rev=871d705d203c

Looks like I need to update some existing assertions in the jit tests.

Now without printfs and with passing jit tests! https://tbpl.mozilla.org/?tree=Try&rev=978879208669

Comment on attachment 8410330 [details] [diff] [review] more-offsets.patch Review of attachment 8410330 [details] [diff] [review]: ----------------------------------------------------------------- Patch looks good to me

sorry had to back this out since it might have caused https://tbpl.mozilla.org/php/getParsedLog.php?id=38385566&tree=Mozilla-Inbound

This should fix those errors. https://tbpl.mozilla.org/?tree=Try&rev=6f13f50624f5

Comment on attachment 8411923 [details] [diff] [review] more-offsets.patch Carrying over ejpbruel's previous r+.

Try push isn't complete, but it looks pretty good. The failures before weren't intermittent or anything, and we have all the mochitests that have completed thus far (about 2/3 at a glance) passing. I think this is ready to land again.