[parser] Skipping inner funcs: make more functions skippable.

- Enable aggressive lazy inner funcs (make non-declaration funcs lazy, ie let f =
  function() { ... } when --experimental-preparser-scope-analysis is on.
- Turn on variable tracking for lazy top level functions: this makes their inner
  functions skippable.
- Test fix for an testing bug uncovered by this work: when restoring the data
  for the relevant scope, don't assume it's the outermost scope for which we
  have data.
- Fix: if we abort lazy parsing a function, we shouldn't produce any data for
  it.

BUG=v8:5516

Change-Id: I0606fbabb5886dc57dbb53ab5f3fb894ff5d032e
Reviewed-on: https://chromium-review.googlesource.com/518165
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45615}

src/ast/scopes.cc

@@ -1530,8 +1530,10 @@ void DeclarationScope::AnalyzePartially(
     PreParsedScopeData* preparsed_scope_data) {
   DCHECK(!force_eager_compilation_);
   VariableProxy* unresolved = nullptr;
+  bool need_preparsed_scope_data = FLAG_experimental_preparser_scope_analysis &&
+                                   preparsed_scope_data->Producing();
 
-  if (!outer_scope_->is_script_scope()) {
+  if (!outer_scope_->is_script_scope() || need_preparsed_scope_data) {
     // 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.
@@ -1549,8 +1551,7 @@ void DeclarationScope::AnalyzePartially(
       arguments_ = nullptr;
     }
 
-    if (FLAG_experimental_preparser_scope_analysis &&
+    if (need_preparsed_scope_data) {
-        preparsed_scope_data->Producing()) {
       // Store the information needed for allocating the locals of this scope
       // and its inner scopes.
       preparsed_scope_data->SaveData(this);

src/flag-definitions.h

@@ -965,7 +965,8 @@ DEFINE_BOOL(aggressive_lazy_inner_functions, false,
 DEFINE_IMPLICATION(aggressive_lazy_inner_functions, lazy_inner_functions)
 DEFINE_BOOL(experimental_preparser_scope_analysis, false,
             "perform scope analysis for preparsed inner functions")
-DEFINE_IMPLICATION(experimental_preparser_scope_analysis, lazy_inner_functions)
+DEFINE_IMPLICATION(experimental_preparser_scope_analysis,
+                   aggressive_lazy_inner_functions)
 
 // simulator-arm.cc, simulator-arm64.cc and simulator-mips.cc
 DEFINE_BOOL(trace_sim, false, "Trace simulator execution")

src/parsing/preparsed-scope-data.cc

@@ -142,7 +142,7 @@ void PreParsedScopeData::RestoreData(Scope* scope, uint32_t* index_ptr) const {
     DCHECK_EQ(data.uses_super_property,
               scope->AsDeclarationScope()->uses_super_property());
     uint32_t index_from_data = 0;
-    FindFunctionData(scope->start_position(), &index_from_data);
+    DCHECK(FindFunctionData(scope->start_position(), &index_from_data));
     DCHECK_EQ(index_from_data, index);
   }
 #endif

src/parsing/preparser.cc

@@ -137,7 +137,8 @@ PreParser::PreParseResult PreParser::PreParseFunction(
   parsing_module_ = parsing_module;
   use_counts_ = use_counts;
   DCHECK(!track_unresolved_variables_);
-  track_unresolved_variables_ = is_inner_function;
+  track_unresolved_variables_ =
+      is_inner_function || FLAG_experimental_preparser_scope_analysis;
 #ifdef DEBUG
   function_scope->set_is_being_lazily_parsed(true);
 #endif
@@ -213,7 +214,7 @@ PreParser::PreParseResult PreParser::PreParseFunction(
     function_scope->DeclareArguments(ast_value_factory());
 
     if (FLAG_experimental_preparser_scope_analysis &&
-        preparsed_scope_data_ != nullptr) {
+        preparsed_scope_data_ != nullptr && result != kLazyParsingAborted) {
       // We're not going to skip this function, but it might contain skippable
       // functions inside it.
       preparsed_scope_data_->AddFunction(

test/cctest/parsing/test-preparser.cc

@@ -770,8 +770,8 @@ TEST(PreParserScopeAnalysis) {
       CHECK_NULL(unallocated_scope->sibling());
       CHECK(unallocated_scope->is_function_scope());
 
-      uint32_t index = 0;
+      lazy_info.preparsed_scope_data()->RestoreData(
-      lazy_info.preparsed_scope_data()->RestoreData(unallocated_scope, &index);
+          unallocated_scope->AsDeclarationScope());
       i::ScopeTestHelper::AllocateWithoutVariableResolution(unallocated_scope);
 
       i::ScopeTestHelper::CompareScopes(

test/mjsunit/skipping-inner-functions.js

@@ -35,3 +35,61 @@
   lazy(9)();
   assertEquals(19, result);
 })();
+
+(function TestCtxtAllocatingNonSimpleParams1() {
+  var result = 0;
+
+  function lazy([other_param1, ctxt_alloc_param, other_param2]) {
+    function skip_me() {
+      result = ctxt_alloc_param;
+    }
+    return skip_me;
+  }
+  // Test that parameters and variables of the outer function get context
+  // allocated even if we skip the inner function.
+  lazy([30, 29, 28])();
+  assertEquals(29, result);
+})();
+
+(function TestCtxtAllocatingNonSimpleParams2() {
+  var result = 0;
+
+  function lazy({a: other_param1, b: ctxt_alloc_param, c: other_param2}) {
+    function skip_me() {
+      result = ctxt_alloc_param;
+    }
+    return skip_me;
+  }
+  // Test that parameters and variables of the outer function get context
+  // allocated even if we skip the inner function.
+  lazy({a: 31, b: 32, c: 33})();
+  assertEquals(32, result);
+})();
+
+(function TestCtxtAllocatingNonSimpleParams3() {
+  var result = 0;
+
+  function lazy(...ctxt_alloc_param) {
+    function skip_me() {
+      result = ctxt_alloc_param;
+    }
+    return skip_me;
+  }
+  // Test that parameters and variables of the outer function get context
+  // allocated even if we skip the inner function.
+  lazy(34, 35)();
+  assertEquals([34, 35], result);
+})();
+
+// Skippable top level functions.
+let result = 0;
+function lazy_top_level(ctxt_alloc_param) {
+  let ctxt_alloc_var = 24;
+  function skip_me() {
+    result = ctxt_alloc_param + ctxt_alloc_var;
+  }
+  skip_me();
+}
+
+lazy_top_level(10);
+assertEquals(34, result);