[Interpreter] Adds support for const/let variables to interpreter.

src/interpreter/bytecode-generator.cc

 // Copyright 2015 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.
 
 #include "src/interpreter/bytecode-generator.h"
 
 #include "src/ast/scopes.h"
 #include "src/compiler.h"
 #include "src/interpreter/bytecode-register-allocator.h"
 #include "src/interpreter/control-flow-builders.h"
@@ skipping 700 matching lines @@
           decl->fun(), info()->script(), info());
       // Check for stack-overflow exception.
       if (function.is_null()) return SetStackOverflow();
       globals()->push_back(variable->name());
       globals()->push_back(function);
       break;
     }
     case VariableLocation::PARAMETER:
     case VariableLocation::LOCAL: {
       VisitForAccumulatorValue(decl->fun());
-      VisitVariableAssignment(variable, FeedbackVectorSlot::Invalid());
+      DCHECK(variable->mode() == LET || variable->mode() == VAR);
+      VisitVariableAssignment(variable, Token::INIT,
+                              FeedbackVectorSlot::Invalid());
       break;
     }
     case VariableLocation::CONTEXT: {
       DCHECK_EQ(0, execution_context()->ContextChainDepth(variable->scope()));
       VisitForAccumulatorValue(decl->fun());
       builder()->StoreContextSlot(execution_context()->reg(),
                                   variable->index());
       break;
     }
     case VariableLocation::LOOKUP: {
@@ skipping 261 matching lines @@
                                              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);
+      VisitVariableAssignment(variable, Token::ASSIGN, slot);
       break;
     }
     case NAMED_PROPERTY: {
       RegisterAllocationScope register_scope(this);
       Register value = register_allocator()->NewRegister();
       builder()->StoreAccumulatorInRegister(value);
       Register object = VisitForRegisterValue(property->obj());
       Handle<String> name = property->key()->AsLiteral()->AsPropertyName();
       builder()->LoadAccumulatorWithRegister(value);
       builder()->StoreNamedProperty(object, name, feedback_index(slot),
@@ skipping 483 matching lines @@
     builder()->LoadAccumulatorWithRegister(literal);
   }
   execution_result()->SetResultInAccumulator();
 }
 
 
 void BytecodeGenerator::VisitVariableProxy(VariableProxy* proxy) {
   VisitVariableLoad(proxy->var(), proxy->VariableFeedbackSlot());
 }
 
+void BytecodeGenerator::BuildHoleCheckForVariableLoad(VariableMode mode,
+                                                      Handle<String> name) {
+  if (mode == CONST_LEGACY) {
+    BytecodeLabel end_label;
+    builder()->JumpIfNotHole(&end_label).LoadUndefined().Bind(&end_label);

[rmcilroy, 2016/01/26 14:37:28]

Newlines between builder operations (throughout).

[mythria, 2016/02/01 10:02:18]

I added newlines. cl format removes them. Shall I override cl format?

[rmcilroy, 2016/02/03 12:28:25]

I think we should override this if possible, it is pretty difficult to read
otherwise (another option would be to just call builder() at the start of every
line here)

+  } else if (mode == LET || mode == CONST) {
+    BuildThrowIfHole(name);
+  }
+}
 
 void BytecodeGenerator::VisitVariableLoad(Variable* variable,
                                           FeedbackVectorSlot slot,
                                           TypeofMode typeof_mode) {
+  VariableMode mode = variable->mode();
   switch (variable->location()) {
     case VariableLocation::LOCAL: {
       Register source(Register(variable->index()));
       builder()->LoadAccumulatorWithRegister(source);
+      BuildHoleCheckForVariableLoad(mode, variable->name());
       execution_result()->SetResultInAccumulator();
       break;
     }
     case VariableLocation::PARAMETER: {
       // The parameter indices are shifted by 1 (receiver is variable
       // index -1 but is parameter index 0 in BytecodeArrayBuilder).
       Register source = builder()->Parameter(variable->index() + 1);
       builder()->LoadAccumulatorWithRegister(source);
+      BuildHoleCheckForVariableLoad(mode, variable->name());
       execution_result()->SetResultInAccumulator();
       break;
     }
     case VariableLocation::GLOBAL:
     case VariableLocation::UNALLOCATED: {
       builder()->LoadGlobal(variable->name(), feedback_index(slot),
                             language_mode(), typeof_mode);
       execution_result()->SetResultInAccumulator();
       break;
     }
@@ skipping 12 matching lines @@
         // when this changes.
         builder()
             ->LoadAccumulatorWithRegister(execution_context()->reg())
             .StoreAccumulatorInRegister(context_reg);
         for (int i = 0; i < depth; ++i) {
           builder()
               ->LoadContextSlot(context_reg, Context::PREVIOUS_INDEX)
               .StoreAccumulatorInRegister(context_reg);
         }
       }
+
       builder()->LoadContextSlot(context_reg, variable->index());
+      BuildHoleCheckForVariableLoad(mode, variable->name());
       execution_result()->SetResultInAccumulator();
-      // TODO(rmcilroy): Perform check for uninitialized legacy const, const and
-      // let variables.
       break;
     }
     case VariableLocation::LOOKUP: {
       builder()->LoadLookupSlot(variable->name(), typeof_mode);
       execution_result()->SetResultInAccumulator();
       break;
     }
   }
 }
 
 
 void BytecodeGenerator::VisitVariableLoadForAccumulatorValue(
     Variable* variable, FeedbackVectorSlot slot, TypeofMode typeof_mode) {
   AccumulatorResultScope accumulator_result(this);
   VisitVariableLoad(variable, slot, typeof_mode);
 }
 
 
 Register BytecodeGenerator::VisitVariableLoadForRegisterValue(
     Variable* variable, FeedbackVectorSlot slot, TypeofMode typeof_mode) {
   RegisterResultScope register_scope(this);
   VisitVariableLoad(variable, slot, typeof_mode);
   return register_scope.ResultRegister();
 }
 
+void BytecodeGenerator::BuildThrowIfHole(Handle<String> name) {
+  Register name_reg = register_allocator()->NewRegister();
+  BytecodeLabel end_label;
+  builder()
+      ->JumpIfNotHole(&end_label)
+      .LoadLiteral(name)
+      .StoreAccumulatorInRegister(name_reg)
+      .CallRuntime(Runtime::kThrowReferenceError, name_reg, 1)
+      .Bind(&end_label);
+}
+
+void BytecodeGenerator::BuildThrowIfNotHole(Handle<String> name) {
+  Register name_reg = register_allocator()->NewRegister();
+  BytecodeLabel end_label;
+  builder()
+      ->JumpIfHole(&end_label)
+      .LoadLiteral(name)
+      .StoreAccumulatorInRegister(name_reg)
+      .CallRuntime(Runtime::kThrowReferenceError, name_reg, 1)
+      .Bind(&end_label);
+}
+
+void BytecodeGenerator::BuildThrowReassignConstant(Register destination,
+                                                   Handle<String> name) {
+  Register name_reg = register_allocator()->NewRegister();
+  BytecodeLabel else_label;
+  builder()
+      ->LoadLiteral(name)
+      .StoreAccumulatorInRegister(name_reg)
+      .LoadAccumulatorWithRegister(destination)
+      .JumpIfNotHole(&else_label)
+      .CallRuntime(Runtime::kThrowReferenceError, name_reg, 1)
+      .Bind(&else_label)
+      .CallRuntime(Runtime::kThrowConstAssignError, name_reg, 1);
+}
+
+void BytecodeGenerator::BuildThrowReassignConstant(Handle<String> name) {

[rmcilroy, 2016/01/26 14:37:28]

Could we merge both BuildThrowReassignConstant (and just always have the value
in a register)?

[mythria, 2016/02/01 10:02:18]

Done.

+  Register name_reg = register_allocator()->NewRegister();
+  BytecodeLabel else_label;
+  builder()
+      ->JumpIfNotHole(&else_label)
+      .LoadLiteral(name)

[rmcilroy, 2016/01/26 14:37:28]

Move LoadLiteral / StoreAccumulatorInRegister(name_reg) before jump (to avoid
doing it twice) ?

[mythria, 2016/02/01 10:02:18]

For throwing const reassign error, we don't need the variable name. It was a bug
and I fixed it. So we need these only in one part of the if. 

Done.

+      .StoreAccumulatorInRegister(name_reg)
+      .CallRuntime(Runtime::kThrowReferenceError, name_reg, 1)
+      .Bind(&else_label)
+      .LoadLiteral(name)
+      .StoreAccumulatorInRegister(name_reg)
+      .CallRuntime(Runtime::kThrowConstAssignError, name_reg, 1);
+}
 
 void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
+                                                Token::Value op,
                                                 FeedbackVectorSlot slot) {
+  VariableMode mode = variable->mode();
+  RegisterAllocationScope assignment_register_scope(this);
+  BytecodeLabel end_label;
+  bool hole_check_required =
+      (mode == CONST_LEGACY) || (mode == LET && op != Token::INIT) ||
+      (mode == CONST && op != Token::INIT) ||
+      (mode == CONST && op == Token::INIT && variable->is_this());
   switch (variable->location()) {
+    case VariableLocation::PARAMETER:
     case VariableLocation::LOCAL: {
-      // TODO(rmcilroy): support const mode initialization.
-      Register destination(variable->index());
-      builder()->StoreAccumulatorInRegister(destination);
-      break;
-    }
-    case VariableLocation::PARAMETER: {
-      // The parameter indices are shifted by 1 (receiver is variable
-      // index -1 but is parameter index 0 in BytecodeArrayBuilder).
-      Register destination(builder()->Parameter(variable->index() + 1));
-      builder()->StoreAccumulatorInRegister(destination);
+      Register destination;
+      if (VariableLocation::PARAMETER == variable->location()) {
+        destination = Register(builder()->Parameter(variable->index() + 1));
+      } else {
+        destination = Register(variable->index());
+      }
+
+      if (!hole_check_required) {
+        builder()->StoreAccumulatorInRegister(destination);
+      } else if (mode == CONST && op != Token::INIT) {

[rmcilroy, 2016/01/26 14:37:28]

As discussed, could you merge the code to do the hole check for this and the
context slot into a common helper function.

[mythria, 2016/02/01 10:02:18]

Done. I did not pull const_legacy && Token::Init to helper function. Because, we
need to store to destination conditionally. We have to pass cotext_reg and
destination registers and only one of them would be valid. Let me know if you
have any suggestions. 

+        // Assigning to constant is not allowed. Throw an appropriate
+        // exception. Accumulator is not restored back.
+        BuildThrowReassignConstant(destination, variable->name());
+      } else if (mode == CONST_LEGACY && op != Token::INIT) {
+        // TODO(mythria): Find a test case for strict mode legacy constants.

[rmcilroy, 2016/01/26 14:37:28]

Legacy const doesn't exist for strict mode, right?

[mythria, 2016/02/01 10:02:18]

Done. Yes, it was a old comment. Retained the DCHECK because we are not totally
sure and have to check with andreas.

+        DCHECK(!is_strict(language_mode()));
+      } else {
+        // Store accumulator to a temporary register and load destination to
+        // perform hole check.
+        BytecodeLabel end_label;
+        Register value_temp = register_allocator()->NewRegister();
+        builder()
+            ->StoreAccumulatorInRegister(value_temp)
+            .LoadAccumulatorWithRegister(destination);
+        if (mode == CONST_LEGACY && op == Token::INIT) {
+          // If the destination is already initialized silently ignore this
+          // assignment. Restore the accumulator.
+          builder()
+              ->JumpIfNotHole(&end_label)
+              .MoveRegister(value_temp, destination)
+              .Bind(&end_label)
+              .LoadAccumulatorWithRegister(value_temp);
+        } else if (mode == LET && op != Token::INIT) {
+          BuildThrowIfHole(variable->name());
+          // Move does not help here, because accumulator should be restored to
+          // value_temp as well.
+          builder()
+              ->LoadAccumulatorWithRegister(value_temp)
+              .StoreAccumulatorInRegister(destination);
+        } else {
+          DCHECK(variable->is_this() && mode == CONST && op == Token::INIT);
+          // Perform an initialization check for 'this'. 'this' variable is the
+          // only variable being able to trigger bind operations outside the TDZ
+          // via 'super' calls.
+          BuildThrowIfNotHole(variable->name());
+          builder()
+              ->LoadAccumulatorWithRegister(value_temp)
+              .StoreAccumulatorInRegister(destination);
+        }
+      }
       break;
     }
     case VariableLocation::GLOBAL:
     case VariableLocation::UNALLOCATED: {
       builder()->StoreGlobal(variable->name(), feedback_index(slot),
                              language_mode());
       break;
     }
     case VariableLocation::CONTEXT: {
-      // TODO(rmcilroy): support const mode initialization.
       int depth = execution_context()->ContextChainDepth(variable->scope());
       ContextScope* context = execution_context()->Previous(depth);
       Register context_reg;
+      Register value_temp;
+      bool value_in_accumulator = true;

[rmcilroy, 2016/01/26 14:37:28]

Let's not try to be smart with "value_in_accumulator" for now (particularly
since we will replace these by special bytecodes), let's just assume that it is
always in the accumulator (maintaining the LoadAccumulatorWithRegister in the
else branch) and have a new temporary register for the hole check which is
stored from the accumulator.

[mythria, 2016/02/01 10:02:18]

Done.

+
       if (context) {
         context_reg = context->reg();
       } else {
-        Register value_temp = register_allocator()->NewRegister();
+        value_temp = register_allocator()->NewRegister();
         context_reg = register_allocator()->NewRegister();
         // Walk the context chain to find the context at the given depth.
         // TODO(rmcilroy): Perform this work in a bytecode handler once we have
         // a generic mechanism for performing jumps in interpreter.cc.
         // TODO(mythria): Also update bytecode graph builder with correct depth
         // when this changes.
         builder()
             ->StoreAccumulatorInRegister(value_temp)
             .LoadAccumulatorWithRegister(execution_context()->reg())
             .StoreAccumulatorInRegister(context_reg);
         for (int i = 0; i < depth; ++i) {
           builder()
               ->LoadContextSlot(context_reg, Context::PREVIOUS_INDEX)
               .StoreAccumulatorInRegister(context_reg);
         }
-        builder()->LoadAccumulatorWithRegister(value_temp);
+        value_in_accumulator = false;
       }
-      builder()->StoreContextSlot(context_reg, variable->index());
+
+      if (!hole_check_required) {
+        if (!value_in_accumulator) {
+          builder()->LoadAccumulatorWithRegister(value_temp);
+        }
+        builder()->StoreContextSlot(context_reg, variable->index());
+      } else if (mode == CONST && op != Token::INIT) {
+        // Assigning to constant is not allowed. Throw an appropriate
+        // exception. Accumulator is not restored back.
+        builder()->LoadContextSlot(context_reg, variable->index());
+        BuildThrowReassignConstant(variable->name());
+      } else if (mode == CONST_LEGACY && op != Token::INIT) {
+        // TODO(mythria): Find a test case for strict mode legacy constants.

[rmcilroy, 2016/01/26 14:37:28]

ditto.

[mythria, 2016/02/01 10:02:18]

Done.

+        DCHECK(!is_strict(language_mode()));
+        // Accumulator has to be restored back.
+        if (!value_in_accumulator) {
+          builder()->LoadAccumulatorWithRegister(value_temp);
+        }
+      } else {
+        // Store accumulator to a temporary register and load destination to
+        // perform hole check.
+        if (value_in_accumulator) {
+          value_temp = register_allocator()->NewRegister();
+          builder()->StoreAccumulatorInRegister(value_temp);
+        }
+        builder()->LoadContextSlot(context_reg, variable->index());
+        BytecodeLabel end_label;
+        if (mode == CONST_LEGACY && op == Token::INIT) {
+          // If the destination is already initialized silently ignore this
+          // assignment and restore back the accumulator.

[rmcilroy, 2016/01/26 14:37:28]

Not sure what "restore back the accumulator" means here?

[mythria, 2016/02/01 10:02:18]

Done.

+          builder()
+              ->JumpIfNotHole(&end_label)
+              .LoadAccumulatorWithRegister(value_temp)
+              .StoreContextSlot(context_reg, variable->index())
+              .Bind(&end_label)
+              .LoadAccumulatorWithRegister(value_temp);
+        } else if (mode == LET && op != Token::INIT) {
+          BuildThrowIfHole(variable->name());
+          builder()
+              ->LoadAccumulatorWithRegister(value_temp)
+              .StoreContextSlot(context_reg, variable->index());
+        } else {
+          DCHECK(variable->is_this() && mode == CONST && op == Token::INIT);
+          // Perform an initialization check for 'this'. 'this' variable is the
+          // only variable being able to trigger bind operations outside the TDZ

[rmcilroy, 2016/01/26 14:37:28]

/s/variable being able/variable able

[mythria, 2016/02/01 10:02:18]

Done.

+          // via 'super' calls.
+          BuildThrowIfNotHole(variable->name());
+          builder()
+              ->LoadAccumulatorWithRegister(value_temp)
+              .StoreContextSlot(context_reg, variable->index());
+        }
+      }
       break;
     }
     case VariableLocation::LOOKUP: {
-      // TODO(mythria): Use Runtime::kInitializeLegacyConstLookupSlot for
-      // initializations of const declarations.
-      builder()->StoreLookupSlot(variable->name(), language_mode());
+      if (mode == CONST_LEGACY && op == Token::INIT) {
+        register_allocator()->PrepareForConsecutiveAllocations(3);
+        Register value = register_allocator()->NextConsecutiveRegister();
+        Register context = register_allocator()->NextConsecutiveRegister();
+        Register name = register_allocator()->NextConsecutiveRegister();
+
+        // InitializeLegacyConstLookupSlot runtime call returns the 'value'
+        // passed to it. So, accumulator will have its original contents when
+        // runtime call returns.
+        builder()
+            ->StoreAccumulatorInRegister(value)
+            .MoveRegister(execution_context()->reg(), context)
+            .LoadLiteral(variable->name())
+            .StoreAccumulatorInRegister(name)
+            .CallRuntime(Runtime::kInitializeLegacyConstLookupSlot, value, 3);
+      } else if (mode == CONST_LEGACY && op != Token::INIT) {
+        // TODO(mythria): Find a test case for strict mode. We should throw in
+        // strict mode.
+        DCHECK(!is_strict(language_mode()));
+      } else {
+        builder()->StoreLookupSlot(variable->name(), language_mode());
+      }
       break;
     }
   }
 }
 
 
 void BytecodeGenerator::VisitAssignment(Assignment* expr) {
   DCHECK(expr->target()->IsValidReferenceExpression());
   Register object, key;
   Handle<String> name;
@@ skipping 72 matching lines @@
     VisitForAccumulatorValue(expr->value());
   }
 
   // Store the value.
   FeedbackVectorSlot slot = expr->AssignmentSlot();
   switch (assign_type) {
     case VARIABLE: {
       // TODO(oth): The VisitVariableAssignment() call is hard to reason about.
       // Is the value in the accumulator safe? Yes, but scary.
       Variable* variable = expr->target()->AsVariableProxy()->var();
-      VisitVariableAssignment(variable, slot);
+      VisitVariableAssignment(variable, expr->op(), slot);
       break;
     }
     case NAMED_PROPERTY:
       builder()->StoreNamedProperty(object, name, feedback_index(slot),
                                     language_mode());
       break;
     case KEYED_PROPERTY:
       builder()->StoreKeyedProperty(object, key, feedback_index(slot),
                                     language_mode());
       break;
@@ skipping 405 matching lines @@
   }
 
   // Perform +1/-1 operation.
   builder()->CountOperation(expr->binary_op(), language_mode_strength());
 
   // Store the value.
   FeedbackVectorSlot feedback_slot = expr->CountSlot();
   switch (assign_type) {
     case VARIABLE: {
       Variable* variable = expr->expression()->AsVariableProxy()->var();
-      VisitVariableAssignment(variable, feedback_slot);
+      VisitVariableAssignment(variable, expr->op(), feedback_slot);
       break;
     }
     case NAMED_PROPERTY: {
       builder()->StoreNamedProperty(obj, name, feedback_index(feedback_slot),
                                     language_mode());
       break;
     }
     case KEYED_PROPERTY: {
       builder()->StoreKeyedProperty(obj, key, feedback_index(feedback_slot),
                                     language_mode());
@@ skipping 243 matching lines @@
 
   DCHECK(variable->IsContextSlot() || variable->IsStackAllocated());
 
   // Allocate and initialize a new arguments object and assign to the
   // {arguments} variable.
   CreateArgumentsType type =
       is_strict(language_mode()) || !info()->has_simple_parameters()
           ? CreateArgumentsType::kUnmappedArguments
           : CreateArgumentsType::kMappedArguments;
   builder()->CreateArguments(type);
-  VisitVariableAssignment(variable, FeedbackVectorSlot::Invalid());
+  VisitVariableAssignment(variable, Token::ASSIGN,
+                          FeedbackVectorSlot::Invalid());
 }
 
 
 void BytecodeGenerator::VisitThisFunctionVariable(Variable* variable) {
   if (variable == nullptr) return;
 
   // TODO(rmcilroy): Remove once we have tests which exercise this code path.
   UNIMPLEMENTED();
 
   // Store the closure we were called with in the given variable.
   builder()->LoadAccumulatorWithRegister(Register::function_closure());
-  VisitVariableAssignment(variable, FeedbackVectorSlot::Invalid());
+  VisitVariableAssignment(variable, Token::INIT, FeedbackVectorSlot::Invalid());
 }
 
 
 void BytecodeGenerator::VisitNewTargetVariable(Variable* variable) {
   if (variable == nullptr) return;
 
   // Store the new target we were called with in the given variable.
   builder()->LoadAccumulatorWithRegister(Register::new_target());
-  VisitVariableAssignment(variable, FeedbackVectorSlot::Invalid());
+  VisitVariableAssignment(variable, Token::INIT, FeedbackVectorSlot::Invalid());
 }
 
 
 void BytecodeGenerator::VisitFunctionClosureForContext() {
   AccumulatorResultScope accumulator_execution_result(this);
   Scope* closure_scope = execution_context()->scope()->ClosureScope();
   if (closure_scope->is_script_scope() ||
       closure_scope->is_module_scope()) {
     // Contexts nested in the native context have a canonical empty function as
     // their closure, not the anonymous closure containing the global code.
@@ skipping 51 matching lines @@
 }
 
 
 int BytecodeGenerator::feedback_index(FeedbackVectorSlot slot) const {
   return info()->feedback_vector()->GetIndex(slot);
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8