[debugger] break at each initialization in a multiple var declaration.

R=rossberg@chromium.org
BUG=v8:4807
LOG=N

Committed: https://crrev.com/ee98147ea0d994bcfb0d14d43f490ee828d2426f
Cr-Commit-Position: refs/heads/master@{#34531}

Review URL: https://codereview.chromium.org/1768973002

Cr-Commit-Position: refs/heads/master@{#34540}

src/parsing/pattern-rewriter.cc

@@ -230,22 +230,20 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
 
     if (IsImmutableVariableMode(descriptor_->mode)) {
       arguments->Add(value, zone());
-      value = NULL;  // zap the value to avoid the unnecessary assignment
-
       // Construct the call to Runtime_InitializeConstGlobal
       // and add it to the initialization statement block.
       // Note that the function does different things depending on
       // the number of arguments (1 or 2).
-      initialize =
-          factory()->NewCallRuntime(Runtime::kInitializeConstGlobal, arguments,
-                                    descriptor_->initialization_pos);
+      initialize = factory()->NewCallRuntime(Runtime::kInitializeConstGlobal,
+                                             arguments, value->position());
+      value = NULL;  // zap the value to avoid the unnecessary assignment
     } else {
       // Add language mode.
       // We may want to pass singleton to avoid Literal allocations.
       LanguageMode language_mode = initialization_scope->language_mode();
-      arguments->Add(factory()->NewNumberLiteral(language_mode,
-                                                 descriptor_->declaration_pos),
-                     zone());
+      arguments->Add(
+          factory()->NewNumberLiteral(language_mode, RelocInfo::kNoPosition),
+          zone());
 
       // Be careful not to assign a value to the global variable if
       // we're in a with. The initialization value should not
@@ -253,12 +251,11 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
       // which is why we need to generate a separate assignment node.
       if (value != NULL && !descriptor_->scope->inside_with()) {
         arguments->Add(value, zone());
-        value = NULL;  // zap the value to avoid the unnecessary assignment
         // Construct the call to Runtime_InitializeVarGlobal
         // and add it to the initialization statement block.
-        initialize =
-            factory()->NewCallRuntime(Runtime::kInitializeVarGlobal, arguments,
-                                      descriptor_->declaration_pos);
+        initialize = factory()->NewCallRuntime(Runtime::kInitializeVarGlobal,
+                                               arguments, value->position());
+        value = NULL;  // zap the value to avoid the unnecessary assignment
       } else {
         initialize = NULL;
       }
@@ -282,8 +279,7 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
     DCHECK_NOT_NULL(proxy->var());
     DCHECK_NOT_NULL(value);
     // Add break location for destructured sub-pattern.
-    int pos =
-        IsSubPattern() ? pattern->position() : descriptor_->initialization_pos;
+    int pos = IsSubPattern() ? pattern->position() : value->position();
     Assignment* assignment =
         factory()->NewAssignment(Token::INIT, proxy, value, pos);
     block_->statements()->Add(
@@ -300,8 +296,7 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
     // property).
     VariableProxy* proxy = initialization_scope->NewUnresolved(factory(), name);
     // Add break location for destructured sub-pattern.
-    int pos =
-        IsSubPattern() ? pattern->position() : descriptor_->initialization_pos;
+    int pos = IsSubPattern() ? pattern->position() : value->position();
     Assignment* assignment =
         factory()->NewAssignment(Token::INIT, proxy, value, pos);
     block_->statements()->Add(

test/cctest/test-debug.cc

@@ -2747,7 +2747,7 @@ TEST(DebugStepKeyedLoadLoop) {
   foo->Call(context, env->Global(), kArgc, args).ToLocalChecked();
 
   // With stepping all break locations are hit.
-  CHECK_EQ(44, break_point_hit_count);
+  CHECK_EQ(45, break_point_hit_count);
 
   v8::Debug::SetDebugEventListener(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded(env->GetIsolate());
@@ -2797,7 +2797,7 @@ TEST(DebugStepKeyedStoreLoop) {
   foo->Call(context, env->Global(), kArgc, args).ToLocalChecked();
 
   // With stepping all break locations are hit.
-  CHECK_EQ(44, break_point_hit_count);
+  CHECK_EQ(45, break_point_hit_count);
 
   v8::Debug::SetDebugEventListener(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded(env->GetIsolate());
@@ -2842,7 +2842,7 @@ TEST(DebugStepNamedLoadLoop) {
   foo->Call(context, env->Global(), 0, NULL).ToLocalChecked();
 
   // With stepping all break locations are hit.
-  CHECK_EQ(65, break_point_hit_count);
+  CHECK_EQ(66, break_point_hit_count);
 
   v8::Debug::SetDebugEventListener(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded(env->GetIsolate());
@@ -2886,8 +2886,7 @@ static void DoDebugStepNamedStoreLoop(int expected) {
 
 
 // Test of the stepping mechanism for named load in a loop.
-TEST(DebugStepNamedStoreLoop) { DoDebugStepNamedStoreLoop(34); }
-
+TEST(DebugStepNamedStoreLoop) { DoDebugStepNamedStoreLoop(35); }
 
 // Test the stepping mechanism with different ICs.
 TEST(DebugStepLinearMixedICs) {
@@ -3294,7 +3293,7 @@ TEST(DebugStepForContinue) {
   v8::Local<v8::Value> argv_10[argc] = {v8::Number::New(isolate, 10)};
   result = foo->Call(context, env->Global(), argc, argv_10).ToLocalChecked();
   CHECK_EQ(5, result->Int32Value(context).FromJust());
-  CHECK_EQ(62, break_point_hit_count);
+  CHECK_EQ(63, break_point_hit_count);
 
   // Looping 100 times.
   step_action = StepIn;
@@ -3302,7 +3301,7 @@ TEST(DebugStepForContinue) {
   v8::Local<v8::Value> argv_100[argc] = {v8::Number::New(isolate, 100)};
   result = foo->Call(context, env->Global(), argc, argv_100).ToLocalChecked();
   CHECK_EQ(50, result->Int32Value(context).FromJust());
-  CHECK_EQ(557, break_point_hit_count);
+  CHECK_EQ(558, break_point_hit_count);
 
   // Get rid of the debug event listener.
   v8::Debug::SetDebugEventListener(isolate, nullptr);
@@ -3348,7 +3347,7 @@ TEST(DebugStepForBreak) {
   v8::Local<v8::Value> argv_10[argc] = {v8::Number::New(isolate, 10)};
   result = foo->Call(context, env->Global(), argc, argv_10).ToLocalChecked();
   CHECK_EQ(9, result->Int32Value(context).FromJust());
-  CHECK_EQ(64, break_point_hit_count);
+  CHECK_EQ(65, break_point_hit_count);
 
   // Looping 100 times.
   step_action = StepIn;
@@ -3356,7 +3355,7 @@ TEST(DebugStepForBreak) {
   v8::Local<v8::Value> argv_100[argc] = {v8::Number::New(isolate, 100)};
   result = foo->Call(context, env->Global(), argc, argv_100).ToLocalChecked();
   CHECK_EQ(99, result->Int32Value(context).FromJust());
-  CHECK_EQ(604, break_point_hit_count);
+  CHECK_EQ(605, break_point_hit_count);
 
   // Get rid of the debug event listener.
   v8::Debug::SetDebugEventListener(isolate, nullptr);

test/mjsunit/debug-multiple-var-decl.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug
+
+// Test we break at every assignment in a var-statement with multiple
+// variable declarations.
+
+var exception = null;
+var log = []
+
+function f() {
+  var l1 = 1,    // l
+      l2,        // m
+      l3 = 3;    // n
+  let l4,        // o
+      l5 = 5,    // p
+      l6 = 6;    // q
+  const l7 = 7,  // r
+        l8 = 8,  // s
+        l9 = 9;  // t
+  return 0;      // u
+}                // v
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    var line = exec_state.frame(0).sourceLineText();
+    var col = exec_state.frame(0).sourceColumn();
+    print(line);
+    var match = line.match(/\/\/ (\w)$/);
+    assertEquals(2, match.length);
+    log.push(match[1] + col);
+    if (match[1] != "v") {
+      exec_state.prepareStep(Debug.StepAction.StepIn);
+    }
+  } catch (e) {
+    exception = e;
+  }
+}
+
+var Debug = debug.Debug;
+Debug.setListener(listener);
+
+debugger;        // a
+var g1 = 1,      // b
+    g2 = 2,      // c
+    g3;          // d
+let g4 = 4,      // e
+    g5,          // f
+    g6 = 6;      // g
+const g7 = 7,    // h
+      g8 = 8,    // i
+      g9 = f();  // j
+
+Debug.setListener(null);
+
+assertNull(exception);
+
+// Note that let declarations, if not explicitly initialized, implicitly
+// initialize to undefined.
+
+var expected = [
+  "a0",               // debugger statement
+  "b9","c9",          // global var
+  "e9","f4","g9",     // global let
+  "h11","i11","j11",  // global const
+  "l11","n11",        // local var
+  "o6","p11","q11",   // local let
+  "r13","s13","t13",  // local const
+  "u2","v0",          // return
+];
+assertEquals(expected, log);

test/mjsunit/debug-stepin-accessor-ic.js

@@ -45,5 +45,5 @@ debugger;                         // Break
 f();                              // Break
 
 Debug.setListener(null);          // Break
-assertEquals(86, break_count);
+assertEquals(87, break_count);
 assertNull(exception);

test/mjsunit/es6/debug-stepin-string-template.js

@@ -48,7 +48,8 @@ assertNull(exception);
 
 assertEquals([
   "a0",
-  "b0",
+  "b44",
+  "b13",
   "d2b13",
   "e0b13",
   "b25",

test/mjsunit/es6/debug-stepnext-for.js

@@ -108,12 +108,12 @@ var expected = [
   "i12","i10","i11","I4","i11","I4","i11","I4","i11",
   // For-of-let: [Symbol.iterator](), next(), body, next(), ...
   "j16","j14","j15","J4","j15","J4","j15","J4","j15",
-  // For-var: var decl, condition, body, next, condition, body, ...
-  "k7","k20","K4","k26","k20","K4","k26","k20","K4","k26","k20",
+  // For-var: var decl, init, condition, body, next, condition, body, ...
+  "k7","k15","k20","K4","k26","k20","K4","k26","k20","K4","k26","k20",
   // For: init, condition, body, next, condition, body, ...
   "l7","l16","L4","l22","l16","L4","l22","l16","L4","l22","l16",
   // For-let: init, condition, body, next, condition, body, ...
-  "m7","m20","M4","m26","m20","M4","m26","m20","M4","m26","m20",
+  "m15","m20","M4","m26","m20","M4","m26","m20","M4","m26","m20",
   // Exit.
   "y0","z0",
 ]

test/mjsunit/harmony/debug-stepin-default-parameters.js

@@ -42,5 +42,5 @@ Debug.setListener(null);            // c
 assertNull(exception);
 assertEquals("default", result);
 
-assertEquals(["a0","b0","f18b13","d2f18b13","e0f18b13","g2b13","h0b13","c0"],
+assertEquals(["a0","b13","f18b13","d2f18b13","e0f18b13","g2b13","h0b13","c0"],
              log);

test/mjsunit/harmony/debug-stepin-proxies.js

@@ -54,8 +54,8 @@ assertEquals(42, get);
 
 assertEquals([
   "a0",
-  "b0", "h4b20", "i2b20",                           // [[Has]]
-  "c0", "j4c15", "k2c15",                           // [[Get]]
-  "d0", "l4d11", "m2d11",                           // [[Set]]
+  "b17", "h4b20", "i2b20",  // [[Has]]
+  "c15", "j4c15", "k2c15",  // [[Get]]
+  "d0", "l4d11", "m2d11",   // [[Set]]
   "g0"
 ], log);

test/mjsunit/regress/regress-debug-deopt-while-recompile.js

@@ -77,8 +77,8 @@ f();
 g();
 
 // Assert that break point is set at expected location.
-assertTrue(Debug.showBreakPoints(f).indexOf("[B0]var baz = bar;") > 0);
-assertTrue(Debug.showBreakPoints(g).indexOf("[B0]var baz = bar;") > 0);
+assertTrue(Debug.showBreakPoints(f).indexOf("var baz = [B0]bar;") > 0);
+assertTrue(Debug.showBreakPoints(g).indexOf("var baz = [B0]bar;") > 0);
 
 assertEquals(2, listened);
 assertNull(exception);