Preparse inner functions (new try)

src/ast/scopes.cc

Index: src/ast/scopes.cc
diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
index 170ec49783b0f003ea342d037adb79dd41bad942..c273e67c4886ba64bb96534346458ae3d4d117f3 100644
--- a/src/ast/scopes.cc
+++ b/src/ast/scopes.cc
@@ -1073,12 +1073,14 @@ void DeclarationScope::AllocateVariables(ParseInfo* info, AnalyzeMode mode) {
   }
 }
 
-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;
 }
@@ -1178,7 +1180,7 @@ Scope* Scope::GetOuterScopeWithContext() {
 
 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());
@@ -1191,8 +1193,9 @@ void DeclarationScope::AnalyzePartially(DeclarationScope* migrate_to,
   // 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);
@@ -1516,6 +1519,27 @@ void Scope::ResolveVariable(ParseInfo* info, VariableProxy* proxy) {
   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) {
@@ -1561,13 +1585,16 @@ void Scope::ResolveVariablesRecursively(ParseInfo* 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;
@@ -1580,7 +1607,8 @@ VariableProxy* Scope::FetchFreeVariables(DeclarationScope* max_outer_scope,
   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;