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

Revert of [debugger] break at each initialization in a multiple var declaration. (patchset #1 id:1 of https://codereview.chromium.org/1768973002/ )

Reason for revert:
[Sheriff] Breaks layout tests:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/5160

Original issue's description:
> [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}

TBR=rossberg@chromium.org,yangguo@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4807

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

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

src/parsing/pattern-rewriter.cc

@@ -230,20 +230,22 @@ 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, value->position());
-      value = NULL;  // zap the value to avoid the unnecessary assignment
+      initialize =
+          factory()->NewCallRuntime(Runtime::kInitializeConstGlobal, arguments,
+                                    descriptor_->initialization_pos);
     } 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, RelocInfo::kNoPosition),
-          zone());
+      arguments->Add(factory()->NewNumberLiteral(language_mode,
+                                                 descriptor_->declaration_pos),
+                     zone());
 
       // Be careful not to assign a value to the global variable if
       // we're in a with. The initialization value should not
@@ -251,11 +253,12 @@ 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, value->position());
-        value = NULL;  // zap the value to avoid the unnecessary assignment
+        initialize =
+            factory()->NewCallRuntime(Runtime::kInitializeVarGlobal, arguments,
+                                      descriptor_->declaration_pos);
       } else {
         initialize = NULL;
       }
@@ -279,7 +282,8 @@ 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() : value->position();
+    int pos =
+        IsSubPattern() ? pattern->position() : descriptor_->initialization_pos;
     Assignment* assignment =
         factory()->NewAssignment(Token::INIT, proxy, value, pos);
     block_->statements()->Add(
@@ -296,7 +300,8 @@ 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() : value->position();
+    int pos =
+        IsSubPattern() ? pattern->position() : descriptor_->initialization_pos;
     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(45, break_point_hit_count);
+  CHECK_EQ(44, 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(45, break_point_hit_count);
+  CHECK_EQ(44, 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(66, break_point_hit_count);
+  CHECK_EQ(65, break_point_hit_count);
 
   v8::Debug::SetDebugEventListener(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded(env->GetIsolate());
@@ -2886,7 +2886,8 @@ static void DoDebugStepNamedStoreLoop(int expected) {
 
 
 // Test of the stepping mechanism for named load in a loop.
-TEST(DebugStepNamedStoreLoop) { DoDebugStepNamedStoreLoop(35); }
+TEST(DebugStepNamedStoreLoop) { DoDebugStepNamedStoreLoop(34); }
+
 
 // Test the stepping mechanism with different ICs.
 TEST(DebugStepLinearMixedICs) {
@@ -3293,7 +3294,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(63, break_point_hit_count);
+  CHECK_EQ(62, break_point_hit_count);
 
   // Looping 100 times.
   step_action = StepIn;
@@ -3301,7 +3302,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(558, break_point_hit_count);
+  CHECK_EQ(557, break_point_hit_count);
 
   // Get rid of the debug event listener.
   v8::Debug::SetDebugEventListener(isolate, nullptr);
@@ -3347,7 +3348,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(65, break_point_hit_count);
+  CHECK_EQ(64, break_point_hit_count);
 
   // Looping 100 times.
   step_action = StepIn;
@@ -3355,7 +3356,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(605, break_point_hit_count);
+  CHECK_EQ(604, 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(87, break_count);
+assertEquals(86, break_count);
 assertNull(exception);

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

@@ -48,8 +48,7 @@ assertNull(exception);
 
 assertEquals([
   "a0",
-  "b44",
-  "b13",
+  "b0",
   "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, init, condition, body, next, condition, body, ...
-  "k7","k15","k20","K4","k26","k20","K4","k26","k20","K4","k26","k20",
+  // For-var: var decl, condition, body, next, condition, body, ...
+  "k7","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, ...
-  "m15","m20","M4","m26","m20","M4","m26","m20","M4","m26","m20",
+  "m7","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","b13","f18b13","d2f18b13","e0f18b13","g2b13","h0b13","c0"],
+assertEquals(["a0","b0","f18b13","d2f18b13","e0f18b13","g2b13","h0b13","c0"],
              log);

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

@@ -54,8 +54,8 @@ assertEquals(42, get);
 
 assertEquals([
   "a0",
-  "b17", "h4b20", "i2b20",  // [[Has]]
-  "c15", "j4c15", "k2c15",  // [[Get]]
-  "d0", "l4d11", "m2d11",   // [[Set]]
+  "b0", "h4b20", "i2b20",                           // [[Has]]
+  "c0", "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("var baz = [B0]bar;") > 0);
-assertTrue(Debug.showBreakPoints(g).indexOf("var baz = [B0]bar;") > 0);
+assertTrue(Debug.showBreakPoints(f).indexOf("[B0]var baz = bar;") > 0);
+assertTrue(Debug.showBreakPoints(g).indexOf("[B0]var baz = bar;") > 0);
 
 assertEquals(2, listened);
 assertNull(exception);