[interpreter] Logically separate hole-checking and const assignment errors

In addition, make use of Variable::binding_needs_init() in addition to
VariableMode when deciding whether to do hole checking in variable assignment.

R=rmcilroy@chromium.org

Review-Url: https://codereview.chromium.org/2227203002
Cr-Commit-Position: refs/heads/master@{#38555}

src/interpreter/bytecode-generator.cc

@@ -2010,30 +2010,16 @@ void BytecodeGenerator::BuildThrowIfNotHole(Handle<String> name) {
   builder()->Bind(&no_reference_error);
 }
 
-void BytecodeGenerator::BuildThrowReassignConstant(Handle<String> name) {
-  // TODO(mythria): This will be replaced by a new bytecode that throws an
-  // appropriate error depending on the whether the value is a hole or not.
-  BytecodeLabel const_assign_error;
-  builder()->JumpIfNotHole(&const_assign_error);
-  BuildThrowReferenceError(name);
-  builder()
-      ->Bind(&const_assign_error)
-      .CallRuntime(Runtime::kThrowConstAssignError, Register(), 0);
-}
-
 void BytecodeGenerator::BuildHoleCheckForVariableAssignment(Variable* variable,
                                                             Token::Value op) {
-  VariableMode mode = variable->mode();
-  DCHECK(mode != CONST_LEGACY);
-  if (mode == CONST && op != Token::INIT) {
-    // Non-intializing assignments to constant is not allowed.
-    BuildThrowReassignConstant(variable->name());
-  } else if (mode == LET && op != Token::INIT) {
-    // Perform an initialization check for let declared variables.
+  DCHECK(variable->mode() != CONST_LEGACY);
+  if (op != Token::INIT) {
+    // Perform an initialization check for let/const declared variables.
     // E.g. let x = (x = 20); is not allowed.
     BuildThrowIfHole(variable->name());
   } else {
-    DCHECK(variable->is_this() && mode == CONST && op == Token::INIT);
+    DCHECK(variable->is_this() && variable->mode() == CONST &&
+           op == Token::INIT);
     // Perform an initialization check for 'this'. 'this' variable is the
     // only variable able to trigger bind operations outside the TDZ
     // via 'super' calls.
@@ -2048,9 +2034,9 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
   RegisterAllocationScope assignment_register_scope(this);
   BytecodeLabel end_label;
   bool hole_check_required =
-      (mode == LET && op != Token::INIT) ||
-      (mode == CONST && op != Token::INIT) ||
-      (mode == CONST && op == Token::INIT && variable->is_this());
+      variable->binding_needs_init() &&
+      ((IsLexicalVariableMode(mode) && op != Token::INIT) ||
+       (mode == CONST && op == Token::INIT && variable->is_this()));
   switch (variable->location()) {
     case VariableLocation::PARAMETER:
     case VariableLocation::LOCAL: {
@@ -2061,16 +2047,6 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
         destination = Register(variable->index());
       }
 
-      if (mode == CONST_LEGACY && op != Token::INIT) {
-        if (is_strict(language_mode())) {
-          builder()->CallRuntime(Runtime::kThrowConstAssignError, Register(),
-                                 0);
-        }
-        // Non-initializing assignments to legacy constants are ignored
-        // in sloppy mode. Break here to avoid storing into variable.
-        break;
-      }
-
       if (hole_check_required) {
         // Load destination to check for hole.
         Register value_temp = register_allocator()->NewRegister();
@@ -2081,6 +2057,17 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
         BuildHoleCheckForVariableAssignment(variable, op);
         builder()->LoadAccumulatorWithRegister(value_temp);
       }
+
+      if ((mode == CONST || mode == CONST_LEGACY) && op != Token::INIT) {
+        if (mode == CONST || is_strict(language_mode())) {
+          builder()->CallRuntime(Runtime::kThrowConstAssignError, Register(),
+                                 0);
+        }
+        // Non-initializing assignments to legacy constants are ignored
+        // in sloppy mode. Break here to avoid storing into variable.
+        break;
+      }
+
       builder()->StoreAccumulatorInRegister(destination);
       break;
     }
@@ -2117,16 +2104,6 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
         builder()->LoadAccumulatorWithRegister(value_temp);
       }
 
-      if (mode == CONST_LEGACY && op != Token::INIT) {
-        if (is_strict(language_mode())) {
-          builder()->CallRuntime(Runtime::kThrowConstAssignError, Register(),
-                                 0);
-        }
-        // Non-initializing assignments to legacy constants are ignored
-        // in sloppy mode. Break here to avoid storing into variable.
-        break;
-      }
-
       if (hole_check_required) {
         // Load destination to check for hole.
         Register value_temp = register_allocator()->NewRegister();
@@ -2138,6 +2115,16 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
         builder()->LoadAccumulatorWithRegister(value_temp);
       }
 
+      if ((mode == CONST || mode == CONST_LEGACY) && op != Token::INIT) {
+        if (mode == CONST || is_strict(language_mode())) {
+          builder()->CallRuntime(Runtime::kThrowConstAssignError, Register(),
+                                 0);
+        }
+        // Non-initializing assignments to legacy constants are ignored
+        // in sloppy mode. Break here to avoid storing into variable.
+        break;
+      }
+
       builder()->StoreContextSlot(context_reg, variable->index());
       break;
     }

src/interpreter/bytecode-generator.h

@@ -113,7 +113,6 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   void BuildAbort(BailoutReason bailout_reason);
   void BuildThrowIfHole(Handle<String> name);
   void BuildThrowIfNotHole(Handle<String> name);
-  void BuildThrowReassignConstant(Handle<String> name);
   void BuildThrowReferenceError(Handle<String> name);
   void BuildHoleCheckForVariableLoad(Variable* variable);
   void BuildHoleCheckForVariableAssignment(Variable* variable, Token::Value op);

test/cctest/interpreter/bytecode_expectations/ConstVariable.golden

@@ -59,7 +59,7 @@ snippet: "
 "
 frame size: 3
 parameter count: 1
-bytecode array length: 31
+bytecode array length: 30
 bytecodes: [
                 B(LdaTheHole),
                 B(Star), R(0),
@@ -72,7 +72,7 @@ bytecodes: [
                 B(Star), R(2),
   /*   48 E> */ B(CallRuntime), U16(Runtime::kThrowReferenceError), R(2), U8(1),
                 B(CallRuntime), U16(Runtime::kThrowConstAssignError), R(0), U8(0),
-                B(Mov), R(1), R(0),
+                B(Star), R(0),
                 B(LdaUndefined),
   /*   55 S> */ B(Return),
 ]
@@ -88,7 +88,7 @@ snippet: "
 "
 frame size: 3
 parameter count: 1
-bytecode array length: 35
+bytecode array length: 32
 bytecodes: [
                 B(LdaTheHole),
                 B(Star), R(0),
@@ -103,7 +103,6 @@ bytecodes: [
                 B(Star), R(2),
   /*   50 E> */ B(CallRuntime), U16(Runtime::kThrowReferenceError), R(2), U8(1),
                 B(CallRuntime), U16(Runtime::kThrowConstAssignError), R(0), U8(0),
-                B(Mov), R(1), R(0),
                 B(LdaUndefined),
   /*   56 S> */ B(Return),
 ]

test/cctest/interpreter/bytecode_expectations/ConstVariableContextSlot.golden

@@ -70,7 +70,7 @@ snippet: "
 "
 frame size: 4
 parameter count: 1
-bytecode array length: 47
+bytecode array length: 42
 bytecodes: [
                 B(CreateFunctionContext), U8(1),
                 B(PushContext), R(1),
@@ -87,8 +87,6 @@ bytecodes: [
                 B(Star), R(3),
                 B(CallRuntime), U16(Runtime::kThrowReferenceError), R(3), U8(1),
                 B(CallRuntime), U16(Runtime::kThrowConstAssignError), R(0), U8(0),
-                B(Ldar), R(2),
-                B(StaContextSlot), R(context), U8(4),
   /*   47 E> */ B(StaContextSlot), R(context), U8(4),
                 B(LdaUndefined),
   /*   80 S> */ B(Return),
@@ -106,7 +104,7 @@ snippet: "
 "
 frame size: 4
 parameter count: 1
-bytecode array length: 49
+bytecode array length: 44
 bytecodes: [
                 B(CreateFunctionContext), U8(1),
                 B(PushContext), R(1),
@@ -125,8 +123,6 @@ bytecodes: [
                 B(Star), R(3),
                 B(CallRuntime), U16(Runtime::kThrowReferenceError), R(3), U8(1),
                 B(CallRuntime), U16(Runtime::kThrowConstAssignError), R(0), U8(0),
-                B(Ldar), R(2),
-                B(StaContextSlot), R(context), U8(4),
                 B(LdaUndefined),
   /*   82 S> */ B(Return),
 ]