Add function-var to variables_ so LookupRecursive doesn't need to special-case it

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 458 matching lines @@
     this_function_ =
         Declare(zone(), this, ast_value_factory->this_function_string(), CONST,
                 Variable::NORMAL, kCreatedInitialized);
   }
 }
 
 Variable* DeclarationScope::DeclareFunctionVar(const AstRawString* name) {
   DCHECK(is_function_scope());
   DCHECK_NULL(function_);
   VariableMode mode = is_strict(language_mode()) ? CONST : CONST_LEGACY;
-  function_ = new (zone())
-      Variable(this, name, mode, Variable::NORMAL, kCreatedInitialized);
+  bool preexists = variables_.Lookup(name);
+  if (preexists || calls_sloppy_eval()) {

[adamk, 2016/08/24 18:15:49]

This logic is pretty tricky (I find it trickier than the old lookup logic).

+    function_ = new (zone())

[adamk, 2016/08/24 20:22:48]

Maybe we should just skip creating the function_ variable if it's definitely
shadowed in the function? The parser already has to know a lot about how this
stuff works...what if it reacted to a nullptr return value of DeclareFunctionVar
by not creating the assignment?

That lets the calls_sloppy_eval use the Declare() path, with the NonLocal
becoming a DYNAMIC_LOCAL.

WDYT?

+        Variable(this, name, mode, Variable::NORMAL, kCreatedInitialized);
+    if (!preexists) NonLocal(name, DYNAMIC);
+  } else {
+    function_ = variables_.Declare(zone(), this, name, mode, Variable::NORMAL,
+                                   kCreatedInitialized);
+  }
   return function_;
 }
 
 Scope* Scope::FinalizeBlockScope() {
   DCHECK(is_block_scope());
 
   if (variables_.occupancy() > 0 ||
       (is_declaration_scope() && calls_sloppy_eval())) {
     return this;
   }
@@ skipping 120 matching lines @@
   int index = ScopeInfo::ContextSlotIndex(scope_info_, name_handle, &mode,
                                           &init_flag, &maybe_assigned_flag);
   if (index < 0) {
     location = VariableLocation::GLOBAL;
     index = ScopeInfo::ContextGlobalSlotIndex(scope_info_, name_handle, &mode,
                                               &init_flag, &maybe_assigned_flag);
   }
   if (index < 0) {
     // Check parameters.
     index = scope_info_->ParameterIndex(*name_handle);
-    if (index < 0) return NULL;
+    if (index < 0) {
+      index = scope_info_->FunctionContextSlotIndex(*name_handle, &mode);
+      if (index < 0) return nullptr;
+      Variable* var = AsDeclarationScope()->DeclareFunctionVar(name);
+      DCHECK_EQ(mode, var->mode());
+      var->AllocateTo(VariableLocation::CONTEXT, index);
+      return variables_.Lookup(name);
+    }
 
     mode = DYNAMIC;
     location = VariableLocation::LOOKUP;
     init_flag = kCreatedInitialized;
     // Be conservative and flag parameters as maybe assigned. Better information
     // would require ScopeInfo to serialize the maybe_assigned bit also for
     // parameters.
     maybe_assigned_flag = kMaybeAssigned;
   } else {
     DCHECK(location != VariableLocation::GLOBAL ||
            (is_script_scope() && IsDeclaredVariableMode(mode) &&
             !IsLexicalVariableMode(mode)));
   }
 
   Variable::Kind kind = Variable::NORMAL;
   if (location == VariableLocation::CONTEXT &&
       index == scope_info_->ReceiverContextSlotIndex()) {
     kind = Variable::THIS;
   }
   // TODO(marja, rossberg): Correctly declare FUNCTION, CLASS, NEW_TARGET, and
   // ARGUMENTS bindings as their corresponding Variable::Kind.
 
   Variable* var = variables_.Declare(zone(), this, name, mode, kind, init_flag,
                                      maybe_assigned_flag);
   var->AllocateTo(location, index);
   return var;
 }
 
-Variable* DeclarationScope::LookupFunctionVar(const AstRawString* name) {
-  if (function_ != nullptr && function_->raw_name() == name) {
-    return function_;
-  } else if (!scope_info_.is_null()) {
-    // If we are backed by a scope info, try to lookup the variable there.
-    VariableMode mode;
-    int index = scope_info_->FunctionContextSlotIndex(*(name->string()), &mode);
-    if (index < 0) return nullptr;
-    Variable* var = DeclareFunctionVar(name);
-    DCHECK_EQ(mode, var->mode());
-    var->AllocateTo(VariableLocation::CONTEXT, index);
-    return var;
-  } else {
-    return nullptr;
-  }
-}
-
-
 Variable* Scope::Lookup(const AstRawString* name) {
   for (Scope* scope = this;
        scope != NULL;
        scope = scope->outer_scope()) {
     Variable* var = scope->LookupLocal(name);
     if (var != NULL) return var;
   }
   return NULL;
 }
 
@@ skipping 560 matching lines @@
   }
 
   // Try to find the variable in this scope.
   Variable* var = LookupLocal(proxy->raw_name());
 
   // We found a variable and we are done. (Even if there is an 'eval' in this
   // scope which introduces the same variable again, the resulting variable
   // remains the same.)
   if (var != nullptr) return var;
 
-  // We did not find a variable locally. Check against the function variable, if
-  // any.
-  if (is_function_scope()) {
-    var = AsDeclarationScope()->LookupFunctionVar(proxy->raw_name());
-    if (var != nullptr) {
-      if (calls_sloppy_eval()) return NonLocal(proxy->raw_name(), DYNAMIC);
-      return var;
-    }
-  }
-
   if (outer_scope_ == outer_scope_end) {
     if (!declare_free) return nullptr;
     DCHECK(is_script_scope());
     // No binding has been found. Declare a variable on the global object.
     return AsDeclarationScope()->DeclareDynamicGlobal(proxy->raw_name(),
                                                       Variable::NORMAL);
   }
 
   DCHECK(!is_script_scope());
 
@@ skipping 405 matching lines @@
       function != nullptr && function->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