Bug 1370648 - use final token as end location of statement list; r?jimb draft
authorTom Tromey <tom@tromey.com>
Fri, 14 Jul 2017 13:29:52 -0600
changeset 619805 8906337888ee96394374c3aed8639cf62faebb46
parent 615350 4a683249128b690ce5fea932305dee8460efd3f8
child 641006 8cb0f2c85c7c74d798b9e9a03810c40250f6e22d
push id71820
push userbmo:ttromey@mozilla.com
push dateWed, 02 Aug 2017 16:50:42 +0000
reviewersjimb
bugs1370648
milestone56.0a1
Bug 1370648 - use final token as end location of statement list; r?jimb This changes the parser to use the final token of a statement list as it's end location. This works around some confusing behavior, such as a breakpoint firing on the marked line: <script> if (1 !== 1) { console.log("dead code!?"); // set breakpoint here } </script> MozReview-Commit-ID: 3Sk1ERw5Q6z
devtools/client/debugger/new/test/mochitest/examples/doc-scripts.html
devtools/server/tests/unit/test_breakpoint-22.js
devtools/server/tests/unit/test_get-executable-lines.js
devtools/server/tests/unit/test_stepping-01.js
devtools/server/tests/unit/test_stepping-02.js
devtools/server/tests/unit/test_stepping-05.js
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/FullParseHandler.h
js/src/frontend/Parser.cpp
js/src/frontend/SyntaxParseHandler.h
js/src/jit-test/tests/debug/Frame-onStep-08.js
js/src/jit-test/tests/debug/Frame-onStep-17.js
js/src/jit-test/tests/debug/Frame-onStep-18.js
js/src/jit-test/tests/debug/Script-lineCount.js
js/src/jit-test/tests/debug/Script-startLine.js
--- a/devtools/client/debugger/new/test/mochitest/examples/doc-scripts.html
+++ b/devtools/client/debugger/new/test/mochitest/examples/doc-scripts.html
@@ -10,12 +10,12 @@
   <body>
     <script src="simple1.js"></script>
     <script src="simple2.js"></script>
     <script src="long.js"></script>
     <script>
       // This inline script allows this HTML page to show up as a
       // source. It also needs to introduce a new global variable so
       // it's not immediately garbage collected.
-      function inline_script() { var x = 5; }
+      inline_script = function () { var x = 5; };
     </script>
   </body>
 </html>
--- a/devtools/server/tests/unit/test_breakpoint-22.js
+++ b/devtools/server/tests/unit/test_breakpoint-22.js
@@ -43,17 +43,17 @@ const test = Task.async(function* () {
   let location = {
     line: gDebuggee.line0 + 2
   };
 
   let [res, ] = yield setBreakpoint(source, location);
   ok(!res.error);
 
   let location2 = {
-    line: gDebuggee.line0 + 5
+    line: gDebuggee.line0 + 7
   };
 
   yield source.setBreakpoint(location2).then(_ => {
     do_throw("no code shall not be found the specified line or below it");
   }, reason => {
     do_check_eq(reason.error, "noCodeAtLineColumn");
     ok(reason.message);
   });
@@ -65,11 +65,11 @@ const test = Task.async(function* () {
 function evalCode() {
   // Start a new script
   Components.utils.evalInSandbox(`
 var line0 = Error().lineNumber;
 function some_function() {
   // breakpoint is valid here -- it slides one line below (line0 + 2)
 }
 debugger;
-// no breakpoint is allowed here (line0 + 5)
+// no breakpoint is allowed after the EOF (line0 + 6)
 `, gDebuggee);
 }
--- a/devtools/server/tests/unit/test_get-executable-lines.js
+++ b/devtools/server/tests/unit/test_get-executable-lines.js
@@ -35,17 +35,17 @@ function run_test() {
 function test_executable_lines() {
   gThreadClient.addOneTimeListener("newSource", function _onNewSource(evt, packet) {
     do_check_eq(evt, "newSource");
 
     gThreadClient.getSources(function ({error, sources}) {
       do_check_true(!error);
       let source = gThreadClient.source(sources[0]);
       source.getExecutableLines(function (lines) {
-        do_check_true(arrays_equal([2, 5, 7, 8, 10, 12, 14, 16], lines));
+        do_check_true(arrays_equal([2, 5, 7, 8, 10, 12, 14, 16, 17], lines));
         finishClient(gClient);
       });
     });
   });
 
   let code = readFile("sourcemapped.js");
 
   Components.utils.evalInSandbox(code, gDebuggee, "1.8",
--- a/devtools/server/tests/unit/test_stepping-01.js
+++ b/devtools/server/tests/unit/test_stepping-01.js
@@ -74,11 +74,11 @@ function test_simple_stepping() {
     });
     gThreadClient.stepOver();
   });
 
   /* eslint-disable */
   gDebuggee.eval("var line0 = Error().lineNumber;\n" +
                  "debugger;\n" +   // line0 + 1
                  "var a = 1;\n" +  // line0 + 2
-                 "var b = 2;\n");  // line0 + 3
+                 "var b = 2;");    // line0 + 3
   /* eslint-enable */
 }
--- a/devtools/server/tests/unit/test_stepping-02.js
+++ b/devtools/server/tests/unit/test_stepping-02.js
@@ -74,11 +74,11 @@ function test_simple_stepping() {
     });
     gThreadClient.stepIn();
   });
 
   /* eslint-disable */
   gDebuggee.eval("var line0 = Error().lineNumber;\n" +
                  "debugger;\n" +   // line0 + 1
                  "var a = 1;\n" +  // line0 + 2
-                 "var b = 2;\n");  // line0 + 3
+                 "var b = 2;");    // line0 + 3
   /* eslint-enable */
 }
--- a/devtools/server/tests/unit/test_stepping-05.js
+++ b/devtools/server/tests/unit/test_stepping-05.js
@@ -76,17 +76,17 @@ function test_stepping_last() {
     });
     gThreadClient.stepIn();
   });
 
   /* eslint-disable */
   gDebuggee.eval("var line0 = Error().lineNumber;\n" +
                  "debugger;\n" +   // line0 + 1
                  "var a = 1;\n" +  // line0 + 2
-                 "var b = 2;\n");  // line0 + 3
+                 "var b = 2;");    // line0 + 3
   /* eslint-enable */
 }
 
 function test_next_pause() {
   gThreadClient.addOneTimeListener("paused", function (event, packet) {
     // Check the return value.
     do_check_eq(packet.type, "paused");
     // Before fixing bug 785689, the type was resumeLimit.
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4983,16 +4983,19 @@ BytecodeEmitter::emitScript(ParseNode* b
 
         if (!lexicalEmitterScope.leave(this))
             return false;
     } else {
         if (!emitTree(body))
             return false;
     }
 
+    if (!updateSourceCoordNotes(body->pn_pos.end))
+        return false;
+
     if (!emit1(JSOP_RETRVAL))
         return false;
 
     if (!emitterScope.leave(this))
         return false;
 
     if (!JSScript::fullyInitFromEmitter(cx, script, this))
         return false;
--- a/js/src/frontend/FullParseHandler.h
+++ b/js/src/frontend/FullParseHandler.h
@@ -450,16 +450,21 @@ class FullParseHandler
         if (isFunctionStmt(stmt)) {
             // PNX_FUNCDEFS notifies the emitter that the block contains
             // body-level function definitions that should be processed
             // before the rest of nodes.
             list->pn_xflags |= PNX_FUNCDEFS;
         }
     }
 
+    void setListEndPosition(ParseNode* list, const TokenPos& pos) {
+        MOZ_ASSERT(list->isKind(PNK_STATEMENTLIST));
+        list->pn_pos.end = pos.end;
+    }
+
     void addCaseStatementToList(ParseNode* list, ParseNode* casepn) {
         MOZ_ASSERT(list->isKind(PNK_STATEMENTLIST));
         MOZ_ASSERT(casepn->isKind(PNK_CASE));
         MOZ_ASSERT(casepn->pn_right->isKind(PNK_STATEMENTLIST));
 
         list->append(casepn);
 
         if (casepn->pn_right->pn_xflags & PNX_FUNCDEFS)
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -4185,18 +4185,24 @@ Parser<ParseHandler, CharT>::statementLi
     uint32_t statementBegin = 0;
     for (;;) {
         TokenKind tt = TOK_EOF;
         if (!tokenStream.peekToken(&tt, TokenStream::Operand)) {
             if (tokenStream.isEOF())
                 isUnexpectedEOF_ = true;
             return null();
         }
-        if (tt == TOK_EOF || tt == TOK_RC)
+        if (tt == TOK_EOF || tt == TOK_RC) {
+            TokenPos pos;
+            if (!tokenStream.peekTokenPos(&pos, TokenStream::Operand)) {
+                return null();
+            }
+            handler.setListEndPosition(pn, pos);
             break;
+        }
         if (afterReturn) {
             if (!tokenStream.peekOffset(&statementBegin, TokenStream::Operand))
                 return null();
         }
         Node next = statementListItem(yieldHandling, canHaveDirectives);
         if (!next) {
             if (tokenStream.isEOF())
                 isUnexpectedEOF_ = true;
--- a/js/src/frontend/SyntaxParseHandler.h
+++ b/js/src/frontend/SyntaxParseHandler.h
@@ -298,16 +298,17 @@ class SyntaxParseHandler
     Node newYieldExpression(uint32_t begin, Node value) { return NodeGeneric; }
     Node newYieldStarExpression(uint32_t begin, Node value) { return NodeGeneric; }
     Node newAwaitExpression(uint32_t begin, Node value) { return NodeGeneric; }
 
     // Statements
 
     Node newStatementList(const TokenPos& pos) { return NodeGeneric; }
     void addStatementToList(Node list, Node stmt) {}
+    void setListEndPosition(Node list, const TokenPos& pos) {}
     void addCaseStatementToList(Node list, Node stmt) {}
     MOZ_MUST_USE bool prependInitialYield(Node stmtList, Node gen) { return true; }
     Node newEmptyStatement(const TokenPos& pos) { return NodeEmptyStatement; }
 
     Node newExportDeclaration(Node kid, const TokenPos& pos) {
         return NodeGeneric;
     }
     Node newExportFromDeclaration(uint32_t begin, Node exportSpecSet, Node moduleSpec) {
--- a/js/src/jit-test/tests/debug/Frame-onStep-08.js
+++ b/js/src/jit-test/tests/debug/Frame-onStep-08.js
@@ -17,13 +17,13 @@ dbg.onEnterFrame = function (frame) {
 };
 
 g.eval("one = 1;\n" +
        "two = 2;\n" +
        "three = 3;\n" +
        "four = 4;\n");
 assertEq(g.four, 4);
 
-// Breakpoints hit on all four lines.
-assertEq(log.replace(/[^B]/g, ''), 'BBBB');
+// Breakpoints hit on all four lines, plus the final line.
+assertEq(log.replace(/[^B]/g, ''), 'BBBBB');
 
 // onStep was called between each pair of breakpoints.
 assertEq(/BB/.exec(log), null);
--- a/js/src/jit-test/tests/debug/Frame-onStep-17.js
+++ b/js/src/jit-test/tests/debug/Frame-onStep-17.js
@@ -19,15 +19,15 @@ dbg.onDebuggerStatement = function (fram
     }
   };
 };
 
 function testOne(decl, loopKind) {
   let body = "var array = [2, 4, 6];\ndebugger;\nfor (" + decl + " iter " +
       loopKind + " array) {\n  print(iter);\n}\n";
   g.eval(body);
-  assertEq(log, "12121212");
+  assertEq(log, "12121214");
 }
 
 for (let decl of ["", "var", "let"]) {
   testOne(decl, "in");
   testOne(decl, "of");
 }
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Frame-onStep-18.js
@@ -0,0 +1,22 @@
+// Regression test for bug 1370648.
+
+let g = newGlobal();
+
+let dbg = Debugger(g);
+let lines = [0, 0, 0, 0, 0];
+dbg.onDebuggerStatement = function (frame) {
+  let dLine = frame.script.getOffsetLocation(frame.offset).lineNumber;
+  lines[0] = 1;
+  frame.onStep = function () {
+    lines[frame.script.getOffsetLocation(this.offset).lineNumber - dLine] = 1;
+  };
+}
+
+let s = `
+      debugger;                 // 0
+      if (1 !== 1) {            // 1
+        print("dead code!?");   // 2
+      }                         // 3
+`;
+g.eval(s);
+assertEq(lines.join(""), "11001");
--- a/js/src/jit-test/tests/debug/Script-lineCount.js
+++ b/js/src/jit-test/tests/debug/Script-lineCount.js
@@ -14,10 +14,10 @@ function test(scriptText, expectedLineCo
   g.evaluate(scriptText);
   assertEq(found, true);
 }
 
 src = 'var a = (function(){\n' + // 0
       'var b = 9;\n' +           // 1
       'console.log("x", b);\n'+  // 2
       'return b;\n' +            // 3
-      '})();\n';                 // 4
+      '})();';                   // 4
 test(src, 5);
--- a/js/src/jit-test/tests/debug/Script-startLine.js
+++ b/js/src/jit-test/tests/debug/Script-startLine.js
@@ -16,24 +16,24 @@ function test(f, manualCount) {
     assertEq(start, g.first);
     assertEq(count, g.last + 1 - g.first);
     print(start, count);
 }
 
 test(function () {
     g.eval("first = Error().lineNumber;\n" +
            "debugger;\n" +
-           "last = Error().lineNumber;\n");
+           "last = Error().lineNumber;");
 });
 
 test(function () {
     g.evaluate("first = Error().lineNumber;\n" +
                "debugger;\n" +
                Array(17000).join("\n") +
-               "last = Error().lineNumber;\n");
+               "last = Error().lineNumber;");
 });
 
 test(function () {
     g.eval("function f1() { first = Error().lineNumber\n" +
            "    debugger;\n" +
            "    last = Error().lineNumber; }\n" +
            "f1();");
 });