Remove CONST_LEGACY VariableMode

src/compiler/ast-graph-builder.cc

 // Copyright 2014 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/compiler/ast-graph-builder.h"
 
 #include "src/ast/scopes.h"
 #include "src/compiler.h"
 #include "src/compiler/ast-loop-assignment-analyzer.h"
 #include "src/compiler/control-builders.h"
@@ skipping 517 matching lines @@
 
   // Initialize the incoming context.
   ContextScope incoming(this, scope, GetFunctionContext());
 
   // Initialize control scope.
   ControlScope control(this);
 
   // TODO(mstarzinger): For now we cannot assume that the {this} parameter is
   // not {the_hole}, because for derived classes {this} has a TDZ and the
   // JSConstructStubForDerived magically passes {the_hole} as a receiver.
-  if (scope->has_this_declaration() && scope->receiver()->is_const_mode()) {
+  if (scope->has_this_declaration() && scope->receiver()->mode() == CONST) {
     env.RawParameterBind(0, jsgraph()->TheHoleConstant());
   }
 
   // Build local context only if there are context allocated variables.
   if (scope->num_heap_slots() > 0) {
     // Push a new inner context scope for the current activation.
     Node* inner_context = BuildLocalActivationContext(GetFunctionContext());
     ContextScope top_context(this, scope, inner_context);
     CreateGraphBody(stack_check);
   } else {
@@ skipping 2883 matching lines @@
     case VariableLocation::UNALLOCATED: {
       // Global var, const, or let variable.
       Handle<Name> name = variable->name();
       Node* store = BuildGlobalStore(name, value, feedback);
       PrepareFrameState(store, bailout_id, combine);
       return store;
     }
     case VariableLocation::PARAMETER:
     case VariableLocation::LOCAL:
       // Local var, const, or let variable.
-      if (mode == CONST_LEGACY && op != Token::INIT) {
-        // Non-initializing assignment to legacy const is
-        // - exception in strict mode.
-        // - ignored in sloppy mode.
-        if (is_strict(language_mode())) {
-          return BuildThrowConstAssignError(bailout_id);
-        }
-        return value;
-      } else if (mode == LET && op == Token::INIT) {
+      if (mode == LET && op == Token::INIT) {
         // No initialization check needed because scoping guarantees it. Note
         // that we still perform a lookup to keep the variable live, because
         // baseline code might contain debug code that inspects the variable.
         Node* current = environment()->Lookup(variable);
         CHECK_NOT_NULL(current);
       } else if (mode == CONST && op == Token::INIT) {
         // Perform an initialization check for const {this} variables.
         // Note that the {this} variable is the only const variable being able
         // to trigger bind operations outside the TDZ, via {super} calls.
         Node* current = environment()->Lookup(variable);
         if (current->op() != the_hole->op() && variable->is_this()) {
           value = BuildHoleCheckElseThrow(current, variable, value, bailout_id);
         }
       } else if (IsLexicalVariableMode(mode) && op != Token::INIT) {

[Toon Verwaest, 2016/08/18 07:05:26]

If I understood correctly, there was some unhappiness about the change in
structure here. This used to explicitly handle all 4 combinations separately:
LET && INIT
CONST && INIT
LET && !INIT
CONST && !INIT

now the !INIT cases are partially merged and it's not as clear anymore. To
enable sharing + keeping the structure clear I'd suggest a helper function for
the shareable part.

[adamk, 2016/08/18 18:40:18]

Yeah, I've reverted that change on master, and updated this change to be based
on current master.

         // Perform an initialization check for lexically declared variables.
         Node* current = environment()->Lookup(variable);
         if (variable->binding_needs_init()) {
           if (current->op() == the_hole->op()) {
             return BuildThrowReferenceError(variable, bailout_id);
           } else if (current->opcode() == IrOpcode::kPhi) {
             BuildHoleCheckThenThrow(current, variable, value, bailout_id);
           }
         }
         if (mode == CONST) {
-          return BuildThrowConstAssignError(bailout_id);
+          if (variable->throw_on_const_assignment(language_mode())) {

[Toon Verwaest, 2016/08/18 07:05:26]

What about returning a ShouldThrow flag from throw_on_const_assignment, passing
it into BuildThrowConstAssignError, which should probably be renamed to
BuildAssignToConst() in that case? That introduces a guarantee that we won't
forget to check this case if we need to introduce a new
BuildThrowConstAssignError conditionally on throw_on_const_assignment. That's
the pattern we use in the runtime now (see e.g., WriteToReadOnlyProperty).

[adamk, 2016/08/18 18:40:18]

I'm worried that still runs afoul of making each case more explicit. I've
instead left this here, with a check for the variable being a
SLOPPY_FUNCTION_NAME. That makes it clearer that this is a special case, but one
that has its own logic separate from "normal" CONST handling.

+            return BuildThrowConstAssignError(bailout_id);
+          }
+          return value;
         }
       }
       environment()->Bind(variable, value);
       return value;
     case VariableLocation::CONTEXT: {
       // Context variable (potentially up the context chain).
       int depth = current_scope()->ContextChainLength(variable->scope());
-      if (mode == CONST_LEGACY && op != Token::INIT) {
-        // Non-initializing assignment to legacy const is
-        // - exception in strict mode.
-        // - ignored in sloppy mode.
-        if (is_strict(language_mode())) {
-          return BuildThrowConstAssignError(bailout_id);
-        }
-        return value;
-      } else if (mode == CONST && op == Token::INIT) {
+      if (mode == CONST && op == Token::INIT) {
         // Perform an initialization check for const {this} variables.
         // Note that the {this} variable is the only const variable being able
         // to trigger bind operations outside the TDZ, via {super} calls.
         if (variable->is_this()) {
           const Operator* op =
               javascript()->LoadContext(depth, variable->index(), false);
           Node* current = NewNode(op, current_context());
           value = BuildHoleCheckElseThrow(current, variable, value, bailout_id);
         }
       } else if (IsLexicalVariableMode(mode) && op != Token::INIT) {

[Toon Verwaest, 2016/08/18 07:05:26]

the same here

         // Perform an initialization check for lexically declared variables.
         if (variable->binding_needs_init()) {
           const Operator* op =
               javascript()->LoadContext(depth, variable->index(), false);
           Node* current = NewNode(op, current_context());
           value = BuildHoleCheckThenThrow(current, variable, value, bailout_id);
         }
         if (mode == CONST) {
-          // Assignment to const is exception in all modes.
-          return BuildThrowConstAssignError(bailout_id);
+          if (variable->throw_on_const_assignment(language_mode())) {
+            return BuildThrowConstAssignError(bailout_id);
+          } else {
+            return value;
+          }
         }
       }
       const Operator* op = javascript()->StoreContext(depth, variable->index());
       return NewNode(op, current_context(), value);
     }
     case VariableLocation::LOOKUP: {
       // Dynamic lookup of context variable (anywhere in the chain).
       Handle<Name> name = variable->name();
       Node* store = BuildDynamicStore(name, value);
       PrepareFrameState(store, bailout_id, combine);
@@ skipping 798 matching lines @@
     // Phi does not exist yet, introduce one.
     value = NewPhi(inputs, value, control);
     value->ReplaceInput(inputs - 1, other);
   }
   return value;
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8