Preparse inner functions (new try)

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/ast/ast.h"
@@ skipping 1055 matching lines @@
   AllocateScopeInfosRecursively(info->isolate(), mode, outer_scope);
   // The debugger expects all shared function infos to contain a scope info.
   // Since the top-most scope will end up in a shared function info, make sure
   // it has one, even if it doesn't need a scope info.
   // TODO(jochen|yangguo): Remove this requirement.
   if (scope_info_.is_null()) {
     scope_info_ = ScopeInfo::Create(info->isolate(), zone(), this, outer_scope);
   }
 }
 
-bool Scope::AllowsLazyParsing() const {
-  // If we are inside a block scope, we must parse eagerly to find out how
-  // to allocate variables on the block scope. At this point, declarations may
-  // not have yet been parsed.
+bool Scope::AllowsLazyParsingWithoutUnresolvedVariables() const {
+  // If we are inside a block scope, we must find unresolved variables in the
+  // inner scopes to find out how to allocate variables on the block scope. At
+  // this point, declarations may not have yet been parsed.
   for (const Scope* s = this; s != nullptr; s = s->outer_scope_) {
     if (s->is_block_scope()) return false;
+    // TODO(marja): Refactor parsing modes: also add s->is_function_scope()
+    // here.
   }
   return true;
 }
 
 bool DeclarationScope::AllowsLazyCompilation() const {
   return !force_eager_compilation_;
 }
 
 bool DeclarationScope::AllowsLazyCompilationWithoutContext() const {
   if (force_eager_compilation_) return false;
@@ skipping 79 matching lines @@
 Scope* Scope::GetOuterScopeWithContext() {
   Scope* scope = outer_scope_;
   while (scope && !scope->NeedsContext()) {
     scope = scope->outer_scope();
   }
   return scope;
 }
 
 Handle<StringSet> DeclarationScope::CollectNonLocals(
     ParseInfo* info, Handle<StringSet> non_locals) {
-  VariableProxy* free_variables = FetchFreeVariables(this, info);
+  VariableProxy* free_variables = FetchFreeVariables(this, true, info);
   for (VariableProxy* proxy = free_variables; proxy != nullptr;
        proxy = proxy->next_unresolved()) {
     non_locals = StringSet::Add(non_locals, proxy->name());
   }
   return non_locals;
 }
 
 void DeclarationScope::AnalyzePartially(DeclarationScope* migrate_to,
                                         AstNodeFactory* ast_node_factory) {
   // Try to resolve unresolved variables for this Scope and migrate those which
   // cannot be resolved inside. It doesn't make sense to try to resolve them in
   // the outer Scopes here, because they are incomplete.
-  for (VariableProxy* proxy = FetchFreeVariables(this); proxy != nullptr;
-       proxy = proxy->next_unresolved()) {
+  for (VariableProxy* proxy =
+           FetchFreeVariables(this, !FLAG_lazy_inner_functions);
+       proxy != nullptr; proxy = proxy->next_unresolved()) {
     DCHECK(!proxy->is_resolved());
     VariableProxy* copy = ast_node_factory->CopyVariableProxy(proxy);
     migrate_to->AddUnresolved(copy);
   }
 
   // Push scope data up to migrate_to. Note that migrate_to and this Scope
   // describe the same Scope, just in different Zones.
   PropagateUsageFlagsToScope(migrate_to);
   if (scope_uses_super_property_) migrate_to->scope_uses_super_property_ = true;
   if (inner_scope_calls_eval_) migrate_to->inner_scope_calls_eval_ = true;
@@ skipping 303 matching lines @@
   }
 
   return var;
 }
 
 void Scope::ResolveVariable(ParseInfo* info, VariableProxy* proxy) {
   DCHECK(info->script_scope()->is_script_scope());
   DCHECK(!proxy->is_resolved());
   Variable* var = LookupRecursive(proxy, nullptr);
   ResolveTo(info, proxy, var);
+
+  if (FLAG_lazy_inner_functions) {
+    if (info != nullptr && info->is_native()) return;
+    // Pessimistically force context allocation for all variables to which inner
+    // scope variables could potentially resolve to.
+    Scope* scope = GetClosureScope()->outer_scope_;
+    while (scope != nullptr && scope->scope_info_.is_null()) {
+      var = scope->LookupLocal(proxy->raw_name());
+      if (var != nullptr) {
+        if (!var->is_dynamic() && !var->is_this() &&
+            !var->has_forced_context_allocation() && !var->is_arguments()) {

[adamk, 2016/09/23 18:21:30]

I think is_arguments() is now dead.

What's the is_this() about?

[marja, 2016/09/26 07:14:41]

Here I just wanted to be less pessimistic and whitelist some identifier-y things
which are not references to the outer function for sure.

Removed is_arguments and added a comment here: since we don't lazy parse inner
arrow functions, inner functions cannot refer to the outer "this".

+          var->ForceContextAllocation();
+          var->set_is_used();
+          // We don't know what the (potentially lazy parsed) inner function
+          // does with the variable; pessimistically assume that it's assigned.
+          var->set_maybe_assigned();
+        }
+      }
+      scope = scope->outer_scope_;
+    }
+  }
 }
 
 void Scope::ResolveTo(ParseInfo* info, VariableProxy* proxy, Variable* var) {
 #ifdef DEBUG
   if (info->script_is_native()) {
     // To avoid polluting the global object in native scripts
     //  - Variables must not be allocated to the global scope.
     CHECK_NOT_NULL(outer_scope());
     //  - Variables must be bound locally or unallocated.
     if (var->IsGlobalObjectProperty()) {
@@ skipping 25 matching lines @@
     ResolveVariable(info, proxy);
   }
 
   // Resolve unresolved variables for inner scopes.
   for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
     scope->ResolveVariablesRecursively(info);
   }
 }
 
 VariableProxy* Scope::FetchFreeVariables(DeclarationScope* max_outer_scope,
-                                         ParseInfo* info,
+                                         bool try_to_resolve, ParseInfo* info,
                                          VariableProxy* stack) {
   for (VariableProxy *proxy = unresolved_, *next = nullptr; proxy != nullptr;
        proxy = next) {
     next = proxy->next_unresolved();
     DCHECK(!proxy->is_resolved());
-    Variable* var = LookupRecursive(proxy, max_outer_scope->outer_scope());
+    Variable* var = nullptr;
+    if (try_to_resolve) {

[Toon Verwaest, 2016/09/23 11:51:18]

Can't you just move !FLAG_lazy_inner_functions here? CollectNonLocals should
behave properly with that selection too; they are context allocated. You may
have to adapt tests, because the tests make assumptions about our context
allocation which have changed in this CL. If you adapt tests, add a TODO to
update it again later once we do full context analysis.

[marja, 2016/09/23 14:26:52]

Offline discussion:

If we don't resolve here, CollectNonLocals needs to be rewritten so that it also
resolves variables. (An important side effect of CollectNonLocals is that it
causes context allocation on the variables which need to be context allocated;
this is what LookupRecursive does here.)

However, because the unresolved variables are a linked list, the rewrite is
nontrivial, and also orthogonal to this CL.

+      var = LookupRecursive(proxy, max_outer_scope->outer_scope());
+    }
     if (var == nullptr) {
       proxy->set_next_unresolved(stack);
       stack = proxy;
     } else if (info != nullptr) {
       ResolveTo(info, proxy, var);
     }
   }
 
   // Clear unresolved_ as it's in an inconsistent state.
   unresolved_ = nullptr;
 
   for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
-    stack = scope->FetchFreeVariables(max_outer_scope, info, stack);
+    stack =
+        scope->FetchFreeVariables(max_outer_scope, try_to_resolve, info, stack);
   }
 
   return stack;
 }
 
 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.
@@ skipping 235 matching lines @@
   Variable* function =
       is_function_scope() ? AsDeclarationScope()->function_var() : nullptr;
   bool is_function_var_in_context =
       function != nullptr && function->IsContextSlot();
   return num_heap_slots() - Context::MIN_CONTEXT_SLOTS -
          (is_function_var_in_context ? 1 : 0);
 }
 
 }  // namespace internal
 }  // namespace v8