Bug 1432682 - Part 1: Remove the hack that causes the bad behavior. r=jimb.

The hack caused bytecode for block declaration instantiation to be assigned the
location of the first statement inside the block. Unfortunately it made the
source view of the debugger client seem out of sync with the Scopes panel: when
paused after hitting a breakpoint on that line or stepping there, the source
panel showed our location as being inside the block, but the Scopes panel did
not show a block scope.

Two server tests required fixes (also r=jimb, in a separate patch in the same
bug).

test_stepping-08.js assumes that stepping into a function stops at the first
statement in the function. This is usually true. However, now we are removing a
hack, such that our actual behavior for this *particular* function is to stop
at the opening curly brace. This causes the test to fail, without anything
really being broken.

The test is intended to test the interaction of stepping and breakpoints, so
the fix that stays truest to the purpose of the test is to change the debuggee
here to a function with no prologue instructions, so that we don't stop at the
opening brace.

test_blackboxing-01.js is a similar story.

--HG--
extra : rebase_source : 7afc6cc039f313889ee08cdd93ce114691efa1e9
extra : histedit_source : dc274b7cefbb96574c8207a78db05d80238d291d

devtools/server/tests/unit/test_blackboxing-01.js

@@ -96,7 +96,7 @@ function evalCode() {
   /* eslint-disable */
   Components.utils.evalInSandbox(
     "" + function doStuff(k) { // line 1
-      let arg = 15;            // line 2 - Step in here
+      var arg = 15;            // line 2 - Step in here
       k(arg);                  // line 3
     },                         // line 4
     gDebuggee,

devtools/server/tests/unit/test_stepping-08.js

@@ -60,8 +60,8 @@ function evaluateTestCode() {
     }                                   //  4
                                         //  5
     function innerFunction() {          //  6
-      let x = 0;                        //  7
-      let y = 72;                       //  8
+      var x = 0;                        //  7
+      var y = 72;                       //  8
       return x+y;                       //  9
     }                                   // 10
     outerFunction();                    // 11

js/src/frontend/BytecodeEmitter.cpp

@@ -895,7 +895,7 @@ BytecodeEmitter::EmitterScope::enterLexical(BytecodeEmitter* bce, ScopeKind kind
     if (!ensureCache(bce))
         return false;
 
-    // Marks all names as closed over if the the context requires it. This
+    // Marks all names as closed over if the context requires it. This
     // cannot be done in the Parser as we may not know if the context requires
     // all bindings to be closed over until after parsing is finished. For
     // example, legacy generators require all bindings to be closed over but
@@ -6831,24 +6831,12 @@ BytecodeEmitter::emitLexicalScope(ParseNode* pn)
     if (pn->isEmptyScope())
         return emitLexicalScopeBody(body);
 
-    // Update line number notes before emitting TDZ poison in
-    // EmitterScope::enterLexical to avoid spurious pausing on seemingly
-    // non-effectful lines in Debugger.
-    //
-    // For example, consider the following code.
-    //
-    // L1: {
-    // L2:   let x = 42;
-    // L3: }
-    //
-    // If line number notes were not updated before the TDZ poison, the TDZ
-    // poison bytecode sequence of 'uninitialized; initlexical' will have line
-    // number L1, and the Debugger will pause there.
+    // We are about to emit some bytecode for what the spec calls "declaration
+    // instantiation". Assign these instructions to the opening `{` of the
+    // block. (Using the location of each declaration we're instantiating is
+    // too weird when stepping in the debugger.)
     if (!ParseNodeRequiresSpecialLineNumberNotes(body)) {
-        ParseNode* pnForPos = body;
-        if (body->isKind(ParseNodeKind::StatementList) && body->pn_head)
-            pnForPos = body->pn_head;
-        if (!updateLineNumberNotes(pnForPos->pn_pos.begin))
+        if (!updateSourceCoordNotes(pn->pn_pos.begin))
             return false;
     }
 

js/src/jit-test/tests/debug/Frame-onStep-19.js

@@ -18,7 +18,7 @@ testStepping(
         }
       })                         // line 7
     `,
-    [2, 3, 7]);
+    [1, 2, 3, 7]);
 
 // Stopping at the ClassDeclaration on line 8 is fine. For that matter,
 // stopping on line 5 wouldn't be so bad if we did it after line 3 and before
@@ -38,4 +38,4 @@ testStepping(
       }                 // 10
       f
     `,
-    [2, 3, 8, 9, 10]);
+    [1, 2, 3, 8, 9, 10]);