Remove two more special cases from Scope::MustAllocate(Variable*)

src/ast/scopes.cc

 // Copyright 2012 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/ast/scopes.h"
 
 #include <set>
 
 #include "src/accessors.h"
 #include "src/bootstrapper.h"
@@ skipping 1286 matching lines @@
   *binding_kind = UNBOUND;
   var =
       is_function_scope()
           ? AsDeclarationScope()->LookupFunctionVar(proxy->raw_name(), factory)
           : nullptr;
   if (var != NULL) {
     *binding_kind = BOUND;
   } else if (outer_scope_ != nullptr && this != max_outer_scope) {
     var = outer_scope_->LookupRecursive(proxy, binding_kind, factory,
                                         max_outer_scope);
-    if (*binding_kind == BOUND && (is_function_scope() || is_with_scope())) {
+    if (*binding_kind == BOUND && is_function_scope()) {
       var->ForceContextAllocation();
     }
   } else {
     DCHECK(is_script_scope() || this == max_outer_scope);
   }
 
   // "this" can't be shadowed by "eval"-introduced bindings or by "with" scopes.
   // TODO(wingo): There are other variables in this category; add them.
   bool name_can_be_shadowed = var == nullptr || !var->is_this();
 
   if (is_with_scope() && name_can_be_shadowed) {
     DCHECK(!already_resolved());
     // The current scope is a with scope, so the variable binding can not be
     // statically resolved. However, note that it was necessary to do a lookup
     // in the outer scope anyway, because if a binding exists in an outer scope,
     // the associated variable has to be marked as potentially being accessed
     // from inside of an inner with scope (the property may not be in the 'with'
     // object).
-    if (var != NULL && proxy->is_assigned()) var->set_maybe_assigned();
+    if (var != NULL) {
+      var->set_is_used();
+      var->ForceContextAllocation();
+      if (proxy->is_assigned()) var->set_maybe_assigned();
+    }
     *binding_kind = DYNAMIC_LOOKUP;
     return NULL;
   } else if (calls_sloppy_eval() && is_declaration_scope() &&
              !is_script_scope() && name_can_be_shadowed) {
     // A variable binding may have been found in an outer scope, but the current
     // scope makes a sloppy 'eval' call, so the found variable may not be
     // the correct one (the 'eval' may introduce a binding with the same name).
     // In that case, change the lookup result to reflect this situation.
     // Only scopes that can host var bindings (declaration scopes) need be
     // considered here (this excludes block and catch scopes), and variable
@@ skipping 145 matching lines @@
   }
 }
 
 
 bool Scope::MustAllocate(Variable* var) {
   DCHECK(var->location() != VariableLocation::MODULE);
   // Give var a read/write use if there is a chance it might be accessed
   // via an eval() call.  This is only possible if the variable has a
   // visible name.
   if ((var->is_this() || !var->raw_name()->IsEmpty()) &&
-      (var->has_forced_context_allocation() || scope_calls_eval_ ||
-       inner_scope_calls_eval_ || is_catch_scope() || is_block_scope() ||
+      (scope_calls_eval_ || inner_scope_calls_eval_ || is_catch_scope() ||
        is_script_scope())) {
     var->set_is_used();
     if (scope_calls_eval_ || inner_scope_calls_eval_) var->set_maybe_assigned();
   }
   // Global variables do not need to be allocated.
   return !var->IsGlobalObjectProperty() && var->is_used();

[neis, 2016/08/09 07:27:15]

Can you add DCHECK(!var->has_forced_context_allocation() || var->set_is_used())
here?

[adamk, 2016/08/09 16:25:18]

Done.

 }
 
 
 bool Scope::MustAllocateInContext(Variable* var) {
   // If var is accessed from an inner scope, or if there is a possibility
   // that it might be accessed from the current or an inner scope (through
   // an eval() call or a runtime with lookup), it must be allocated in the
   // context.
   //
   // Exceptions: If the scope as a whole has forced context allocation, all
@@ skipping 272 matching lines @@
       function != NULL && function->proxy()->var()->IsContextSlot();
   return num_heap_slots() - Context::MIN_CONTEXT_SLOTS - num_global_slots() -
          (is_function_var_in_context ? 1 : 0);
 }
 
 
 int Scope::ContextGlobalCount() const { return num_global_slots(); }
 
 }  // namespace internal
 }  // namespace v8