Static resolution of outer variables in eval code.

src/scopes.cc

Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index f9cebcddaafff5532c4aac23ab7af06dd9a7c220..356b578d2256ed172a3271107b34b4ed61fee08c 100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -148,6 +148,8 @@ Scope::Scope(Scope* inner_scope,
   SetDefaults(type, NULL, scope_info);
   if (!scope_info.is_null()) {
     num_heap_slots_ = scope_info_->ContextLength();
+  } else if (is_with_scope()) {
+    num_heap_slots_ = Context::MIN_CONTEXT_SLOTS;
   }
   AddInnerScope(inner_scope);
 }
@@ -165,6 +167,7 @@ Scope::Scope(Scope* inner_scope, Handle<String> catch_variable_name)
   SetDefaults(CATCH_SCOPE, NULL, Handle<ScopeInfo>::null());
   AddInnerScope(inner_scope);
   ++num_var_or_const_;
+  num_heap_slots_ = Context::MIN_CONTEXT_SLOTS;
   Variable* variable = variables_.Declare(this,
                                           catch_variable_name,
                                           VAR,
@@ -201,14 +204,16 @@ void Scope::SetDefaults(ScopeType type,
   scope_info_ = scope_info;
   start_position_ = RelocInfo::kNoPosition;
   end_position_ = RelocInfo::kNoPosition;
+  if (!scope_info.is_null()) {
+    scope_calls_eval_ = scope_info->CallsEval();
+    strict_mode_flag_ =
+        scope_info->IsStrictMode() ? kStrictMode : kNonStrictMode;
+  }
 }
 
 
-Scope* Scope::DeserializeScopeChain(CompilationInfo* info,
-                                    Scope* global_scope) {
+Scope* Scope::DeserializeScopeChain(Context* context, Scope* global_scope) {
   // Reconstruct the outer scope chain from a closure's context chain.
-  ASSERT(!info->closure().is_null());
-  Context* context = info->closure()->context();
   Scope* current_scope = NULL;
   Scope* innermost_scope = NULL;
   bool contains_with = false;
@@ -356,10 +361,6 @@ Variable* Scope::LocalLookup(Handle<String> name) {
     return result;
   }
   // If we have a serialized scope info, we might find the variable there.
-  //
-  // We should never lookup 'arguments' in this scope as it is implicitly
-  // present in every scope.
-  ASSERT(*name != *isolate_->factory()->arguments_symbol());
   // There should be no local slot with the given name.
   ASSERT(scope_info_->StackSlotIndex(*name) < 0);
 
@@ -372,11 +373,7 @@ Variable* Scope::LocalLookup(Handle<String> name) {
     mode = VAR;
     init_flag = kCreatedInitialized;
     index = scope_info_->ParameterIndex(*name);
-    if (index < 0) {
-      // Check the function name.
-      index = scope_info_->FunctionContextSlotIndex(*name, &mode);
-      if (index < 0) return NULL;
-    }
+    if (index < 0) return NULL;
   }
 
   Variable* var =
@@ -391,6 +388,23 @@ Variable* Scope::LocalLookup(Handle<String> name) {
 }
 
 
+Variable* Scope::LookupFunctionVar(Handle<String> name) {
+  if (function_ != NULL && function_->name().is_identical_to(name)) {
+    return function_->var();
+  } 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, &mode);
+    if (index < 0) return NULL;
+    Variable* var = DeclareFunctionVar(name, mode);
+    var->AllocateTo(Variable::CONTEXT, index);
+    return var;
+  } else {
+    return NULL;
+  }
+}
+
+
 Variable* Scope::Lookup(Handle<String> name) {
   for (Scope* scope = this;
        scope != NULL;
@@ -559,7 +573,7 @@ void Scope::CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
 }
 
 
-void Scope::AllocateVariables(Handle<Context> context) {
+void Scope::AllocateVariables(Handle<Context> calling_context) {
   ASSERT(outer_scope_ == NULL);  // eval or global scopes only
 
   // 1) Propagate scope information.
@@ -569,16 +583,19 @@ void Scope::AllocateVariables(Handle<Context> context) {
   // this information in the ScopeInfo and then use it here (by traversing
   // the call chain stack, at compile time).
 
-  bool outer_scope_calls_non_strict_eval = false;
-  if (!is_global_scope()) {
-    context->ComputeEvalScopeInfo(&outer_scope_calls_non_strict_eval);
+  Scope* global_scope;
+  if (is_global_scope()) {
+    global_scope = this;
+  } else {
+    global_scope = new Scope(NULL, GLOBAL_SCOPE);
+    Scope* calling_scope =
+        DeserializeScopeChain(*calling_context, global_scope);

[Kevin Millikin (Chromium), 2011/11/10 12:55:21]

I don't like this.  It doesn't seem consistent.  For lazily-compiled code, we
deserialize right up front in the parser, so the parser always has the correct
scope chain.

I think of eval code as being the same as lazy compilation, so it seems better
to deserialize earlier: somewhere before Parser::DoParseProgram parses the
program body, inspect the compilation info to see if it's an eval call and
deserialize there.

[Steven, 2011/11/11 08:35:04]

Done.

+    calling_scope->AddInnerScope(this);
   }
-  PropagateScopeInfo(outer_scope_calls_non_strict_eval);
+  global_scope->PropagateScopeInfo(false);
 
   // 2) Resolve variables.
-  Scope* global_scope = NULL;
-  if (is_global_scope()) global_scope = this;
-  ResolveVariablesRecursively(global_scope, context);
+  ResolveVariablesRecursively(global_scope);
 
   // 3) Allocate variables.
   AllocateVariablesRecursively();
@@ -832,7 +849,6 @@ Variable* Scope::NonLocal(Handle<String> name, VariableMode mode) {
 
 
 Variable* Scope::LookupRecursive(Handle<String> name,
-                                 Handle<Context> context,
                                  BindingKind* binding_kind) {
   ASSERT(binding_kind != NULL);
   // Try to find the variable in this scope.
@@ -849,20 +865,17 @@ Variable* Scope::LookupRecursive(Handle<String> name,
   // We did not find a variable locally. Check against the function variable,
   // if any. We can do this for all scopes, since the function variable is
   // only present - if at all - for function scopes.
-  //
-  // This lookup corresponds to a lookup in the "intermediate" scope sitting
-  // between this scope and the outer scope. (ECMA-262, 3rd., requires that
-  // the name of named function literal is kept in an intermediate scope
-  // in between this scope and the next outer scope.)
   *binding_kind = UNBOUND;
-  if (function_ != NULL && function_->name().is_identical_to(name)) {
-    var = function_->var();
+  var = LookupFunctionVar(name);
+  if (var != NULL) {
     *binding_kind = BOUND;
   } else if (outer_scope_ != NULL) {
-    var = outer_scope_->LookupRecursive(name, context, binding_kind);
+    var = outer_scope_->LookupRecursive(name, binding_kind);
     if (*binding_kind == BOUND && (is_function_scope() || is_with_scope())) {
       var->ForceContextAllocation();
     }
+  } else {
+    ASSERT(is_global_scope());
   }
 
   if (is_with_scope()) {
@@ -874,16 +887,6 @@ Variable* Scope::LookupRecursive(Handle<String> name,
     // object).
     *binding_kind = DYNAMIC_LOOKUP;
     return NULL;
-  } else if (is_eval_scope()) {
-    // No local binding was found, no 'with' statements have been encountered
-    // and the code is executed as part of a call to 'eval'. The calling context
-    // contains scope information that we can use to determine if the variable
-    // is global, i.e. the calling context chain does not contain a binding and
-    // no 'with' contexts.
-    ASSERT(*binding_kind == UNBOUND);
-    *binding_kind = context->GlobalIfNotShadowedByEval(name)
-        ? UNBOUND_EVAL_SHADOWED : DYNAMIC_LOOKUP;
-    return NULL;
   } else if (calls_non_strict_eval()) {
     // A variable binding may have been found in an outer scope, but the current
     // scope makes a non-strict 'eval' call, so the found variable may not be
@@ -900,7 +903,6 @@ Variable* Scope::LookupRecursive(Handle<String> name,
 
 
 void Scope::ResolveVariable(Scope* global_scope,
-                            Handle<Context> context,
                             VariableProxy* proxy) {
   ASSERT(global_scope == NULL || global_scope->is_global_scope());
 
@@ -910,7 +912,7 @@ void Scope::ResolveVariable(Scope* global_scope,
 
   // Otherwise, try to resolve the variable.
   BindingKind binding_kind;
-  Variable* var = LookupRecursive(proxy->name(), context, &binding_kind);
+  Variable* var = LookupRecursive(proxy->name(), &binding_kind);
   switch (binding_kind) {
     case BOUND:
       // We found a variable binding.
@@ -951,18 +953,17 @@ void Scope::ResolveVariable(Scope* global_scope,
 }
 
 
-void Scope::ResolveVariablesRecursively(Scope* global_scope,
-                                        Handle<Context> context) {
+void Scope::ResolveVariablesRecursively(Scope* global_scope) {
   ASSERT(global_scope == NULL || global_scope->is_global_scope());
 
   // Resolve unresolved variables for this scope.
   for (int i = 0; i < unresolved_.length(); i++) {
-    ResolveVariable(global_scope, context, unresolved_[i]);
+    ResolveVariable(global_scope, unresolved_[i]);
   }
 
   // Resolve unresolved variables for inner scopes.
   for (int i = 0; i < inner_scopes_.length(); i++) {
-    inner_scopes_[i]->ResolveVariablesRecursively(global_scope, context);
+    inner_scopes_[i]->ResolveVariablesRecursively(global_scope);
   }
 }