Commit de4f3d3e authored by vogelheim's avatar vogelheim Committed by Commit bot

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}
parent 690c7a85
......@@ -1202,7 +1202,6 @@ void FullCodeGenerator::VisitForStatement(ForStatement* stmt) {
Iteration loop_statement(this, stmt);
if (stmt->init() != NULL) {
SetStatementPosition(stmt->init());
Visit(stmt->init());
}
......
......@@ -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],
......
......@@ -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<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 +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(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<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 +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(605, break_point_hit_count);
CHECK_EQ(604, break_point_hit_count);
// Get rid of the debug event listener.
v8::Debug::SetDebugEventListener(isolate, nullptr);
......
......@@ -45,5 +45,5 @@ debugger; // Break
f(); // Break
Debug.setListener(null); // Break
assertEquals(87, break_count);
assertEquals(86, break_count);
assertNull(exception);
......@@ -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, ...
......
......@@ -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],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment