[Interpreter] Add support for for..in.

src/interpreter/bytecode-generator.cc

Index: src/interpreter/bytecode-generator.cc
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index a8407294a7c112cbef29fcd734722faf65331657..21dffab62de34e1bf0992ae2ee34013ead24230e 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -613,8 +613,120 @@ void BytecodeGenerator::VisitForStatement(ForStatement* stmt) {
 }
 
 
+void BytecodeGenerator::VisitForInAssignment(Expression* expr,
+                                             FeedbackVectorSlot slot) {
+  DCHECK(expr->IsValidReferenceExpression());
+
+  // Evaluate assignment starting with the value to be stored in the
+  // accumulator.
+  Property* property = expr->AsProperty();
+  LhsKind assign_type = Property::GetAssignType(property);
+  switch (assign_type) {
+    case VARIABLE: {
+      Variable* variable = expr->AsVariableProxy()->var();
+      VisitVariableAssignment(variable, slot);
+      break;
+    }
+    case NAMED_PROPERTY: {
+      TemporaryRegisterScope temporary_register_scope(builder());
+      Register value = temporary_register_scope.NewRegister();
+      builder()->StoreAccumulatorInRegister(value);
+      Register object = VisitForRegisterValue(property->obj());
+      size_t name_index = builder()->GetConstantPoolEntry(
+          property->key()->AsLiteral()->AsPropertyName());
+      builder()->StoreNamedProperty(object, name_index, feedback_index(slot),
+                                    language_mode());
+      break;
+    }
+    case KEYED_PROPERTY: {
+      TemporaryRegisterScope temporary_register_scope(builder());
+      Register value = temporary_register_scope.NewRegister();
+      builder()->StoreAccumulatorInRegister(value);
+      Register object = VisitForRegisterValue(property->obj());
+      Register key = VisitForRegisterValue(property->key());
+      builder()->LoadAccumulatorWithRegister(value);
+      builder()->StoreKeyedProperty(object, key, feedback_index(slot),
+                                    language_mode());
+      break;
+    }
+    case NAMED_SUPER_PROPERTY:
+    case KEYED_SUPER_PROPERTY:
+      UNIMPLEMENTED();
+  }
+}
+
+
 void BytecodeGenerator::VisitForInStatement(ForInStatement* stmt) {
-  UNIMPLEMENTED();
+  // TODO(oth): For now we need a parent scope for paths that end up
+  // in VisitLiteral which can allocate in the parent scope. A future
+  // CL in preparation will a StatementResultScope that will remove

[rmcilroy, 2015/10/28 13:46:22]

/s/will a Statement.../will add a Statement.../

[oth, 2015/10/28 22:50:40]

Done.

+  // the need for this EffectResultScope.
+  EffectResultScope result_scope(this);
+
+  if (stmt->subject()->IsNullLiteral() ||
+      stmt->subject()->IsUndefinedLiteral(isolate())) {
+    // ForIn generates lots of code, skip if it wouldn't produce any effects.
+    return;
+  }
+
+  LoopBuilder loop_builder(builder());
+  ControlScopeForIteration control_scope(this, stmt, &loop_builder);
+
+  // Prepare the state for executing ForIn.
+  builder()->EnterBlock();
+  VisitForAccumulatorValue(stmt->subject());
+  loop_builder.BreakIfUndefined();
+  loop_builder.BreakIfNull();
+
+  builder()->CastAccumulatorToJSObject();
+  Register receiver = execution_result()->NewRegister();
+  builder()->StoreAccumulatorInRegister(receiver);
+  builder()->CallRuntime(Runtime::kGetPropertyNamesFast, receiver, 1);
+  builder()->ForInPrepare(receiver);
+  Register for_in_state = execution_result()->NewRegister();

[rmcilroy, 2015/10/28 13:46:22]

nit - I would put Register declarations at the top of the (whitespace
deliniated) code blocks where they are first used. Just personal preference
though, so feel free to ignore me.

[oth, 2015/10/28 22:50:40]

Done.

+  builder()
+      ->StoreAccumulatorInRegister(for_in_state)
+      .LoadUndefined()
+      .CompareOperation(Token::Value::EQ_STRICT, for_in_state,
+                        language_mode_strength());

[rmcilroy, 2015/10/28 13:46:22]

Can you use BreakIfUndefined here instead?

[oth, 2015/10/28 22:50:40]

Done.

+  loop_builder.BreakIfTrue();
+
+  // The loop.
+  BytecodeLabel condition_label, break_label, continue_label;
+  Register index = receiver;  // re-using register as receiver no longer used.

[rmcilroy, 2015/10/28 13:46:22]

nit - /s/re/Re

[oth, 2015/10/28 22:50:40]

Done.

+  builder()->LoadLiteral(Smi::FromInt(0));
+
+  // Check loop termination (accumulator holds index).
+  builder()
+      ->Bind(&condition_label)
+      .StoreAccumulatorInRegister(index)
+      .ForInDone(for_in_state);
+  loop_builder.BreakIfTrue();
+
+  // Get the next item.
+  builder()->LoadAccumulatorWithRegister(index).ForInNext(for_in_state);

[rmcilroy, 2015/10/28 13:46:22]

nit - could you require ForInNext to take a register for the index to avoid
having to do LoadAccumulatorWithRegister here (one less byte, and one less
dispatch)?

[oth, 2015/10/28 22:50:40]

Done.

+
+  // Start again if the item, currently in the accumulator, is undefined.
+  loop_builder.ContinueIfUndefined();
+
+  // Store the value in the each variable.
+  VisitForInAssignment(stmt->each(), stmt->EachFeedbackSlot());
+  // NB the user's loop variable will be assigned the value of each so
+  // even an empty body will have this assignment.
+  Visit(stmt->body());

[rmcilroy, 2015/10/28 13:46:22]

VisitForEffect?

[oth, 2015/10/28 22:50:40]

stmt->body() is a statement rather than expression so not applicable here.

[rmcilroy, 2015/10/29 10:53:45]

Acknowledged.

+
+  // Increment the index and start loop again.
+  builder()
+      ->Bind(&continue_label)
+      .LoadAccumulatorWithRegister(index)
+      .CountOperation(Token::Value::ADD, language_mode_strength())
+      .Jump(&condition_label);
+
+  // End of loop
+  builder()->Bind(&break_label).LeaveBlock();
+
+  loop_builder.SetBreakTarget(break_label);
+  loop_builder.SetContinueTarget(continue_label);
 }