Bug 1370648 - use final token as end location of statement list; r=jimb
authorTom Tromey <tom@tromey.com>
Fri, 14 Jul 2017 13:29:52 -0600
changeset 372687 34a05b4aae38f0d19cc4770b844e71185fe10321
parent 372686 3758e58e78460d53b3c58dac3c41c6749256fdd2
child 372688 cdc0c0802e54710b6f60de5bb709b76a70eb3fb7
push id47947
push userttromey@mozilla.com
push dateThu, 03 Aug 2017 17:18:37 +0000
treeherderautoland@34a05b4aae38 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1370648
milestone57.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 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
@@ -4189,18 +4189,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();");
 });