Bug 984696 - Save more detailed source notes so that Debugger.Script.prototype.getAllColumnOffsets can offer more for source mapped and/or pretty printed JS debugging. r=ejpbruel
authorNick Fitzgerald <fitzgen@mozilla.com>
Thu, 24 Apr 2014 09:32:00 +0200
changeset 180101 6d4e3460c1f827b5f7b53b9e35e3fa263fdbc15b
parent 180100 bd83aff8800a96cf11c5369da465012cea54165b
child 180102 3b6b08c9a5a0659511cdd03e05e0498252ea7ed4
push id26653
push usercbook@mozilla.com
push dateFri, 25 Apr 2014 10:50:28 +0000
treeherdermozilla-central@2b02d933c39a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersejpbruel
bugs984696
milestone31.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 984696 - Save more detailed source notes so that Debugger.Script.prototype.getAllColumnOffsets can offer more for source mapped and/or pretty printed JS debugging. r=ejpbruel
dom/events/test/test_error_events.html
js/src/frontend/BytecodeEmitter.cpp
js/src/jit-test/tests/debug/Object-deleteProperty-error-02.js
js/src/jit-test/tests/debug/Script-getAllColumnOffsets-01.js
js/src/jit-test/tests/debug/Script-getAllColumnOffsets-02.js
js/src/jit-test/tests/debug/Script-getAllColumnOffsets-03.js
js/src/jit-test/tests/debug/Script-getAllColumnOffsets-04.js
js/src/jit-test/tests/debug/Script-getAllColumnOffsets-05.js
--- a/dom/events/test/test_error_events.html
+++ b/dom/events/test/test_error_events.html
@@ -32,18 +32,18 @@
     [ "Event filename", errorEvent.filename, location.href ],
     [ "Callback filename", file, location.href ],
     [ "Event line number", errorEvent.lineno, 27 ],
     [ "Callback line number", line, 27 ],
     [ "Event message", errorEvent.message, "Error: hello" ],
     [ "Callback message", msg, "Error: hello" ],
     [ "Event error-object", errorEvent.error, thrown],
     [ "Callback error-object", error, thrown ],
-    [ "Event column", errorEvent.colno, 0 ], // Sadly not correct right now
-    [ "Callback column", column, 0 ]
+    [ "Event column", errorEvent.colno, 6 ], // Sadly not correct right now
+    [ "Callback column", column, 6 ]
   ]);
 </script>
 <script>
   var workerLocation = location.protocol + "//" + location.host +
     location.pathname.replace("test_error_events.html", "error_event_worker.js");
   var eventFileTest = async_test("Worker event filename");
   var eventLineTest = async_test("Worker event line number");
   var eventMessageTest = async_test("Worker event message");
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -3431,16 +3431,18 @@ static bool
 EmitVariables(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn, VarEmitOption emitOption,
               bool isLet = false)
 {
     JS_ASSERT(pn->isArity(PN_LIST));
     JS_ASSERT(isLet == (emitOption == PushInitialValues));
 
     ParseNode *next;
     for (ParseNode *pn2 = pn->pn_head; ; pn2 = next) {
+        if (!UpdateSourceCoordNotes(cx, bce, pn2->pn_pos.begin))
+            return false;
         next = pn2->pn_next;
 
         ParseNode *pn3;
         if (!pn2->isKind(PNK_NAME)) {
             if (pn2->isKind(PNK_ARRAY) || pn2->isKind(PNK_OBJECT)) {
                 /*
                  * Emit variable binding ops, but not destructuring ops.  The
                  * parser (see Parser::variables) has ensured that our caller
@@ -5899,16 +5901,19 @@ EmitObject(ExclusiveContext *cx, Bytecod
     if (bce->script->compileAndGo()) {
         gc::AllocKind kind = GuessObjectGCKind(pn->pn_count);
         obj = NewBuiltinClassInstance(cx, &JSObject::class_, kind, TenuredObject);
         if (!obj)
             return false;
     }
 
     for (ParseNode *pn2 = pn->pn_head; pn2; pn2 = pn2->pn_next) {
+        if (!UpdateSourceCoordNotes(cx, bce, pn2->pn_pos.begin))
+            return false;
+
         /* Emit an index for t[2] for later consumption by JSOP_INITELEM. */
         ParseNode *pn3 = pn2->pn_left;
         bool isIndex = false;
         if (pn3->isKind(PNK_NUMBER)) {
             if (!EmitNumberOp(cx, pn3->pn_dval, bce))
                 return false;
             isIndex = true;
         } else {
@@ -6060,16 +6065,18 @@ EmitArray(ExclusiveContext *cx, Bytecode
     // minimum possible final size.
     SET_UINT24(pc, count - nspread);
 
     ParseNode *pn2 = pn;
     jsatomid atomIndex;
     if (nspread && !EmitNumberOp(cx, 0, bce))
         return false;
     for (atomIndex = 0; pn2; atomIndex++, pn2 = pn2->pn_next) {
+        if (!UpdateSourceCoordNotes(cx, bce, pn2->pn_pos.begin))
+            return false;
         if (pn2->isKind(PNK_ELISION)) {
             if (Emit1(cx, bce, JSOP_HOLE) < 0)
                 return false;
         } else {
             ParseNode *expr = pn2->isKind(PNK_SPREAD) ? pn2->pn_kid : pn2;
             if (!EmitTree(cx, bce, expr))
                 return false;
         }
@@ -6367,16 +6374,18 @@ frontend::EmitTree(ExclusiveContext *cx,
 
       case PNK_LABEL:
         ok = EmitLabeledStatement(cx, bce, &pn->as<LabeledStatement>());
         break;
 
       case PNK_COMMA:
       {
         for (ParseNode *pn2 = pn->pn_head; ; pn2 = pn2->pn_next) {
+            if (!UpdateSourceCoordNotes(cx, bce, pn2->pn_pos.begin))
+                return false;
             if (!EmitTree(cx, bce, pn2))
                 return false;
             if (!pn2->pn_next)
                 break;
             if (Emit1(cx, bce, JSOP_POP) < 0)
                 return false;
         }
         break;
--- a/js/src/jit-test/tests/debug/Object-deleteProperty-error-02.js
+++ b/js/src/jit-test/tests/debug/Object-deleteProperty-error-02.js
@@ -3,21 +3,17 @@ var dbg = Debugger(g);
 dbg.onDebuggerStatement = function (frame) {
     try {
         frame.arguments[0].deleteProperty("x");
     } catch (exc) {
         assertEq(exc instanceof ReferenceError, true);
         assertEq(exc.message, "diaf");
         assertEq(exc.fileName, "fail");
         assertEq(exc.lineNumber, 4);
-
-        // Arrant nonsense?  Sure -- but different from lineNumber is all this
-        // test exists to verify.  If you're the person to make column numbers
-        // actually work, change this accordingly.
-        assertEq(exc.columnNumber, 0);
+        assertEq(exc.columnNumber, 20);
         return;
     }
     throw new Error("deleteProperty should throw");
 };
 
 g.evaluate("function h(obj) { debugger; } \n" +
            "h(new Proxy({}, \n" +
            "            { deleteProperty: function () { \n" +
--- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-01.js
+++ b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-01.js
@@ -11,9 +11,9 @@ Debugger(global).onDebuggerStatement = f
             }
         });
     });
 };
 
 global.log = '';
 global.eval("function f(n) { for (var i = 0; i < n; ++i) log += '. '; log += '! '; } debugger;");
 global.f(3);
-assertEq(global.log, "21 32 44 . 39 32 44 . 39 32 44 . 39 32 57 ! 69 ");
+assertEq(global.log, "25 32 44 . 39 32 44 . 39 32 44 . 39 32 57 ! 69 ");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-02.js
@@ -0,0 +1,21 @@
+// getColumnOffsets correctly places multiple variable declarations.
+
+var global = newGlobal();
+Debugger(global).onDebuggerStatement = function (frame) {
+    var script = frame.eval("f").return.script;
+    script.getAllColumnOffsets().forEach(function (offset) {
+        script.setBreakpoint(offset.offset, {
+            hit: function (frame) {
+                assertEq(offset.lineNumber, 1);
+                global.log += offset.columnNumber + " ";
+            }
+        });
+    });
+};
+
+global.log = '';
+global.eval("function f(n){var w0,x1=3,y2=4,z3=9} debugger;");
+global.f(3);
+
+// Should have hit each variable declared.
+assertEq(global.log, "18 21 26 31 33 ");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-03.js
@@ -0,0 +1,20 @@
+// getColumnOffsets correctly places comma separated expressions.
+
+var global = newGlobal();
+Debugger(global).onDebuggerStatement = function (frame) {
+    var script = frame.eval("f").return.script;
+    script.getAllColumnOffsets().forEach(function (offset) {
+        script.setBreakpoint(offset.offset, {
+            hit: function (frame) {
+                assertEq(offset.lineNumber, 1);
+                global.log += offset.columnNumber + " ";
+            }
+        });
+    });
+};
+
+global.log = '';
+global.eval("function f(n){print(n),print(n),print(n)} debugger;");
+global.f(3);
+// Should hit each call that was separated by commas.
+assertEq(global.log, "14 23 32 40 ");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-04.js
@@ -0,0 +1,20 @@
+// getColumnOffsets correctly places object properties.
+
+var global = newGlobal();
+Debugger(global).onDebuggerStatement = function (frame) {
+    var script = frame.eval("f").return.script;
+    script.getAllColumnOffsets().forEach(function (offset) {
+        script.setBreakpoint(offset.offset, {
+            hit: function (frame) {
+                assertEq(offset.lineNumber, 1);
+                global.log += offset.columnNumber + " ";
+            }
+        });
+    });
+};
+
+global.log = '';
+global.eval("function f(n){var o={a:1,b:2,c:3}} debugger;");
+global.f(3);
+// Should hit each property in the object.
+assertEq(global.log, "18 21 25 29 19 ");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-05.js
@@ -0,0 +1,20 @@
+// getColumnOffsets correctly places array properties.
+
+var global = newGlobal();
+Debugger(global).onDebuggerStatement = function (frame) {
+    var script = frame.eval("f").return.script;
+    script.getAllColumnOffsets().forEach(function (offset) {
+        script.setBreakpoint(offset.offset, {
+            hit: function (frame) {
+                assertEq(offset.lineNumber, 1);
+                global.log += offset.columnNumber + " ";
+            }
+        });
+    });
+};
+
+global.log = '';
+global.eval("function f(n){var a=[1,2,3]} debugger;");
+global.f(3);
+// Should hit each item in the array.
+assertEq(global.log, "18 21 23 25 19 ");