From de4f3d3eff50f0cdd53a6cdf9d4a389d575b17aa Mon Sep 17 00:00:00 2001 From: vogelheim Date: Fri, 11 Mar 2016 04:02:37 -0800 Subject: [PATCH] Fix expression positions for for-loops. FullCodegen generates 2 statement positions for the loop init block, like so: for(var i = 0; i.... ^ ^ This change removes the first of those, updates unit tests, and removes text expectations for Ignition. --- An alternative would be to emulate the existing behaviour in Ignition, but: - The new behaviour seems more logical, - Ignition generates no bytecodes for the 'var', meaning there is no code position to attach the break position to. BUG=v8:4690 LOG=Y Review URL: https://codereview.chromium.org/1784883002 Cr-Commit-Position: refs/heads/master@{#34717} --- src/full-codegen/full-codegen.cc | 1 - test/cctest/cctest.status | 5 ----- test/cctest/test-debug.cc | 16 ++++++++-------- test/mjsunit/debug-stepin-accessor-ic.js | 2 +- test/mjsunit/es6/debug-stepnext-for.js | 4 ++-- test/mjsunit/mjsunit.status | 1 - 6 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/full-codegen/full-codegen.cc b/src/full-codegen/full-codegen.cc index 87fb279421..6c40d26af2 100644 --- a/src/full-codegen/full-codegen.cc +++ b/src/full-codegen/full-codegen.cc @@ -1202,7 +1202,6 @@ void FullCodeGenerator::VisitForStatement(ForStatement* stmt) { Iteration loop_statement(this, stmt); if (stmt->init() != NULL) { - SetStatementPosition(stmt->init()); Visit(stmt->init()); } diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 360e70526f..cb35e90a68 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -514,11 +514,6 @@ 'test-debug/DebugStepForIn': [FAIL], 'test-debug/DebugStepDoWhile': [FAIL], 'test-debug/DebugConditional': [FAIL], - 'test-debug/DebugStepForContinue': [FAIL], - 'test-debug/DebugStepKeyedStoreLoop': [FAIL], - 'test-debug/DebugStepForBreak': [FAIL], - 'test-debug/DebugStepKeyedLoadLoop': [FAIL], - 'test-debug/DebugStepNamedStoreLoop': [FAIL], # BUG(4333). Function name inferrer does not work for ES6 clases. 'test-func-name-inference/UpperCaseClass': [TIMEOUT], diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index 1dc77ea617..2f7a05d214 100644 --- a/test/cctest/test-debug.cc +++ b/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,7 @@ 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 +3293,7 @@ TEST(DebugStepForContinue) { v8::Local 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 +3301,7 @@ TEST(DebugStepForContinue) { v8::Local 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 +3347,7 @@ TEST(DebugStepForBreak) { v8::Local 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 +3355,7 @@ TEST(DebugStepForBreak) { v8::Local 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); diff --git a/test/mjsunit/debug-stepin-accessor-ic.js b/test/mjsunit/debug-stepin-accessor-ic.js index 5599d7a1fc..66c0580fd6 100644 --- a/test/mjsunit/debug-stepin-accessor-ic.js +++ b/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); diff --git a/test/mjsunit/es6/debug-stepnext-for.js b/test/mjsunit/es6/debug-stepnext-for.js index dcca001d61..9d5641a4a3 100644 --- a/test/mjsunit/es6/debug-stepnext-for.js +++ b/test/mjsunit/es6/debug-stepnext-for.js @@ -108,8 +108,8 @@ 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: init, condition, body, next, condition, body, ... + "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, ... diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index ac182d9375..ff6b802047 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -772,7 +772,6 @@ 'regress/regress-crbug-119800': [FAIL], 'regress/regress-opt-after-debug-deopt': [FAIL], 'regress/regress-crbug-568477-2': [FAIL], - 'debug-stepin-accessor-ic': [FAIL], # TODO(rmcilroy,4765): assertion failures in LiveEdit tests. 'debug-liveedit-restart-frame': [FAIL],