Preparse inner functions (new try)

src/parsing/parser.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/parsing/parser.h"
 
 #include <memory>
 
 #include "src/api.h"
 #include "src/ast/ast-expression-rewriter.h"
@@ skipping 104 matching lines @@
  public:
   DiscardableZoneScope(Parser* parser, Zone* temp_zone, bool use_temp_zone)
       : ast_node_factory_scope_(parser->factory(), temp_zone, use_temp_zone),
         fni_(parser->ast_value_factory_, temp_zone),
         parser_(parser),
         prev_fni_(parser->fni_),
         prev_zone_(parser->zone_) {
     if (use_temp_zone) {
       parser_->fni_ = &fni_;
       parser_->zone_ = temp_zone;
+      if (parser_->reusable_preparser_ != nullptr) {
+        parser_->reusable_preparser_->zone_ = temp_zone;
+      }
     }
   }
   ~DiscardableZoneScope() {
     parser_->fni_ = prev_fni_;
     parser_->zone_ = prev_zone_;
+    if (parser_->reusable_preparser_ != nullptr) {
+      parser_->reusable_preparser_->zone_ = prev_zone_;
+    }
   }
 
  private:
   AstNodeFactory::BodyScope ast_node_factory_scope_;
   FuncNameInferrer fni_;
   Parser* parser_;
   FuncNameInferrer* prev_fni_;
   Zone* prev_zone_;
 
   DISALLOW_COPY_AND_ASSIGN(DiscardableZoneScope);
@@ skipping 2535 matching lines @@
   // - It must not have been prohibited by the caller to Parse (some callers
   //   need a full AST).
   // - The outer scope must allow lazy compilation of inner functions.
   // - The function mustn't be a function expression with an open parenthesis
   //   before; we consider that a hint that the function will be called
   //   immediately, and it would be a waste of time to make it lazily
   //   compiled.
   // These are all things we can know at this point, without looking at the
   // function itself.
 
-  // In addition, we need to distinguish between these cases:
+  // We separate between lazy parsing top level functions and lazy parsing inner
+  // functions, because the latter needs to do more work. In particular, we need
+  // to track unresolved variables to distinguish between these cases:
   // (function foo() {
   //   bar = function() { return 1; }
   //  })();
   // and
   // (function foo() {
   //   var a = 1;
   //   bar = function() { return a; }
   //  })();
 
   // Now foo will be parsed eagerly and compiled eagerly (optimization: assume
   // parenthesis before the function means that it will be called
-  // immediately). The inner function *must* be parsed eagerly to resolve the
-  // possible reference to the variable in foo's scope. However, it's possible
-  // that it will be compiled lazily.
+  // immediately). bar can be parsed lazily, but we need to parse it in a mode
+  // that tracks unresolved variables.
+  DCHECK_IMPLIES(mode() == PARSE_LAZILY, FLAG_lazy);
+  DCHECK_IMPLIES(mode() == PARSE_LAZILY, allow_lazy());
+  DCHECK_IMPLIES(mode() == PARSE_LAZILY, extension_ == nullptr);
 
-  // To make this additional case work, both Parser and PreParser implement a
-  // logic where only top-level functions will be parsed lazily.
-  bool is_lazily_parsed = mode() == PARSE_LAZILY &&
-                          scope()->AllowsLazyParsing() &&
-                          !function_state_->next_function_is_parenthesized();
+  bool is_lazy_top_level_function =
+      mode() == PARSE_LAZILY &&
+      eager_compile_hint == FunctionLiteral::kShouldLazyCompile &&
+      scope()->AllowsLazyParsingWithoutUnresolvedVariables();
 
-  // Determine whether the function body can be discarded after parsing.
+  // Determine whether we can still do the inner function lazy parsing.

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

Determine whether we can still lazy parse the inner function ?

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

Done.

   // The preconditions are:
   // - Lazy compilation has to be enabled.
   // - Neither V8 natives nor native function declarations can be allowed,
   //   since parsing one would retroactively force the function to be
   //   eagerly compiled.
   // - The invoker of this parser can't depend on the AST being eagerly
   //   built (either because the function is about to be compiled, or
   //   because the AST is going to be inspected for some reason).
   // - Because of the above, we can't be attempting to parse a
   //   FunctionExpression; even without enclosing parentheses it might be
   //   immediately invoked.
   // - The function literal shouldn't be hinted to eagerly compile.
   // - For asm.js functions the body needs to be available when module
   //   validation is active, because we examine the entire module at once.
-  bool use_temp_zone =
-      !is_lazily_parsed && allow_lazy() &&
+
+  // Inner functions will be parsed by using a temporary Zone. After parsing, we

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

parsed using

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

Done.

+  // will migrate unresolved variable into a Scope in the main Zone.
+  // TODO(marja): Refactor parsing modes: simplify this.
+  bool is_lazy_inner_function =
+      !is_lazy_top_level_function && allow_lazy() &&
       function_type == FunctionLiteral::kDeclaration &&
       eager_compile_hint != FunctionLiteral::kShouldEagerCompile &&
       !(FLAG_validate_asm && scope()->IsAsmModule());
 
   DeclarationScope* main_scope = nullptr;
-  if (use_temp_zone) {
+  if (is_lazy_inner_function) {
     // This Scope lives in the main Zone; we'll migrate data into it later.
     main_scope = NewFunctionScope(kind);
   }
 
   ZoneList<Statement*>* body = nullptr;
   int arity = -1;
   int materialized_literal_count = -1;
   int expected_property_count = -1;
   DuplicateFinder duplicate_finder(scanner()->unicode_cache());
   bool should_be_used_once_hint = false;
   bool has_duplicate_parameters;
 
   {
     // Temporary zones can nest. When we migrate free variables (see below), we
     // need to recreate them in the previous Zone.
     AstNodeFactory previous_zone_ast_node_factory(ast_value_factory());
     previous_zone_ast_node_factory.set_zone(zone());
 
     // Open a new zone scope, which sets our AstNodeFactory to allocate in the
     // new temporary zone if the preconditions are satisfied, and ensures that
     // the previous zone is always restored after parsing the body. To be able
     // to do scope analysis correctly after full parsing, we migrate needed
     // information from scope into main_scope when the function has been parsed.
     Zone temp_zone(zone()->allocator());
-    DiscardableZoneScope zone_scope(this, &temp_zone, use_temp_zone);
+    DiscardableZoneScope zone_scope(this, &temp_zone, is_lazy_inner_function);
 
     DeclarationScope* scope = NewFunctionScope(kind);
     SetLanguageMode(scope, language_mode);
-    if (!use_temp_zone) {
+    if (!is_lazy_inner_function) {
       main_scope = scope;
     } else {
       DCHECK(main_scope->zone() != scope->zone());
     }
 
     FunctionState function_state(&function_state_, &scope_state_, scope, kind);
 #ifdef DEBUG
     scope->SetScopeName(function_name);
 #endif
     ExpressionClassifier formals_classifier(this, &duplicate_finder);
@@ skipping 23 matching lines @@
     int formals_end_position = scanner()->location().end_pos;
 
     CheckArityRestrictions(arity, kind, formals.has_rest, start_position,
                            formals_end_position, CHECK_OK);
     Expect(Token::LBRACE, CHECK_OK);
     // Don't include the rest parameter into the function's formal parameter
     // count (esp. the SharedFunctionInfo::internal_formal_parameter_count,
     // which says whether we need to create an arguments adaptor frame).
     if (formals.has_rest) arity--;
 
-    // Eager or lazy parse?
-    // If is_lazily_parsed, we'll parse lazily. We'll call SkipLazyFunctionBody,
-    // which may decide to abort lazy parsing if it suspects that wasn't a good
-    // idea. If so (in which case the parser is expected to have backtracked),
-    // or if we didn't try to lazy parse in the first place, we'll have to parse
-    // eagerly.
-    if (is_lazily_parsed) {
+    // Eager or lazy parse? If is_lazy_top_level_function, we'll parse
+    // lazily. We'll call SkipLazyFunctionBody, which may decide to abort lazy
+    // parsing if it suspects that wasn't a good idea. If so (in which case the
+    // parser is expected to have backtracked), or if we didn't try to lazy
+    // parse in the first place, we'll have to parse eagerly.
+    if (is_lazy_top_level_function) {

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

What about just merging this with the is_lazy_inner_function below, only
allowing aborting if is_lazy_top_level_function? Basically:

if (is_lazy_function) {
  bookmark scope...
   result = SkipLazy...( , is_lazy_top_level_function, ...) ;
   ...
   if (result == kLazyParsingAborted) {
     DCHECK(is_lazy_top_level_function);
     is_lazy_function = false;
     ...
    } else if (is_lazy_inner_function) {
      // By now this can be moved here since we just need to fetch all
references. Even if we'd do scope resolution it can be done here since I've
already modified DeclareArguments to happen within function parsing rather than
down below.
      scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory); 
    }
}
if (!is_lazy_function) {
  DCHECK_EQ(scope, main_scope);
  ...eager parse...
}

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

Done - but in the new version is much more difficult to see that exactly on
branch gets executed.

(This is because of the lazy parsing aborting, so, the structure cannot be just
if - if else - if else - else, but it needs to be more complicated.)

       Scanner::BookmarkScope bookmark(scanner());
       bookmark.Set();
       LazyParsingResult result =
           SkipLazyFunctionBody(&materialized_literal_count,
-                               &expected_property_count, true, CHECK_OK);
+                               &expected_property_count, false, true, CHECK_OK);
 
       materialized_literal_count += formals.materialized_literals_count +
                                     function_state.materialized_literal_count();
 
       if (result == kLazyParsingAborted) {
         bookmark.Apply();
         // Trigger eager (re-)parsing, just below this block.
-        is_lazily_parsed = false;
+        is_lazy_top_level_function = false;
 
         // This is probably an initialization function. Inform the compiler it
         // should also eager-compile this function, and that we expect it to be
         // used once.
         eager_compile_hint = FunctionLiteral::kShouldEagerCompile;
         should_be_used_once_hint = true;
       }
     }
-    if (!is_lazily_parsed) {
+    if (is_lazy_inner_function) {
+      if (FLAG_lazy_inner_functions) {
+        LazyParsingResult result = SkipLazyFunctionBody(
+            &materialized_literal_count, &expected_property_count, true, false,
+            CHECK_OK);
+        materialized_literal_count +=
+            formals.materialized_literals_count +
+            function_state.materialized_literal_count();
+        DCHECK(result != kLazyParsingAborted);
+        USE(result);
+      } else {
+        ParseEagerFunctionBody(function_name, pos, formals, kind, function_type,
+                               CHECK_OK);
+        materialized_literal_count =
+            function_state.materialized_literal_count();
+        expected_property_count = function_state.expected_property_count();
+      }
+    } else if (!is_lazy_top_level_function) {
       body = ParseEagerFunctionBody(function_name, pos, formals, kind,
                                     function_type, CHECK_OK);
 
       materialized_literal_count = function_state.materialized_literal_count();
       expected_property_count = function_state.expected_property_count();
-      if (use_temp_zone) {
-        // If the preconditions are correct the function body should never be
-        // accessed, but do this anyway for better behaviour if they're wrong.
-        body = nullptr;
-      }
     }
 
     // Parsing the body may change the language mode in our scope.
     language_mode = scope->language_mode();
     scope->DeclareArguments(ast_value_factory());
     if (main_scope != scope) {
       main_scope->DeclareArguments(ast_value_factory());
     }
 
     // Validate name and parameter names. We can do this only after parsing the
@@ skipping 13 matching lines @@
     }
     CheckConflictingVarDeclarations(scope, CHECK_OK);
 
     if (body) {
       // If body can be inspected, rewrite queued destructuring assignments
       RewriteDestructuringAssignments();
     }
     has_duplicate_parameters =
         !classifier()->is_valid_formal_parameter_list_without_duplicates();
 
-    if (use_temp_zone) {
+    if (is_lazy_inner_function) {

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

This can be dropped if you handle it as described above.

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

Done.

       DCHECK(main_scope != scope);
       scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
     }
   }  // DiscardableZoneScope goes out of scope.
 
   FunctionLiteral::ParameterFlag duplicate_parameters =
       has_duplicate_parameters ? FunctionLiteral::kHasDuplicateParameters
                                : FunctionLiteral::kNoDuplicateParameters;
 
   // Note that the FunctionLiteral needs to be created in the main Zone again.
@@ skipping 33 matching lines @@
   }
   return ParseFunctionLiteral(name, scanner()->location(),
                               is_strict_reserved ? kFunctionNameIsStrictReserved
                                                  : kFunctionNameValidityUnknown,
                               FunctionKind::kAsyncFunction, pos, type,
                               language_mode(), CHECK_OK);
 }
 
 Parser::LazyParsingResult Parser::SkipLazyFunctionBody(
     int* materialized_literal_count, int* expected_property_count,
-    bool may_abort, bool* ok) {
+    bool is_inner_function, bool may_abort, bool* ok) {
   if (produce_cached_parse_data()) CHECK(log_);
 
   int function_block_pos = position();
   DeclarationScope* scope = this->scope()->AsDeclarationScope();
   DCHECK(scope->is_function_scope());
   scope->set_is_lazily_parsed(true);
-  if (consume_cached_parse_data() && !cached_parse_data_->rejected()) {
+  // Inner functions are not part of the cached data.
+  if (!is_inner_function && consume_cached_parse_data() &&
+      !cached_parse_data_->rejected()) {
     // If we have cached data, we use it to skip parsing the function body. The
     // data contains the information we need to construct the lazy function.
     FunctionEntry entry =
         cached_parse_data_->GetFunctionEntry(function_block_pos);
     // Check that cached data is valid. If not, mark it as invalid (the embedder
     // handles it). Note that end position greater than end of stream is safe,
     // and hard to check.
     if (entry.is_valid() && entry.end_pos() > function_block_pos) {
       scanner()->SeekForward(entry.end_pos() - 1);
 
       scope->set_end_position(entry.end_pos());
       Expect(Token::RBRACE, CHECK_OK_VALUE(kLazyParsingComplete));
       total_preparse_skipped_ += scope->end_position() - function_block_pos;
       *materialized_literal_count = entry.literal_count();
       *expected_property_count = entry.property_count();
       SetLanguageMode(scope, entry.language_mode());
       if (entry.uses_super_property()) scope->RecordSuperPropertyUsage();
       if (entry.calls_eval()) scope->RecordEvalCall();
       return kLazyParsingComplete;
     }
     cached_parse_data_->Reject();
   }
   // With no cached data, we partially parse the function, without building an
   // AST. This gathers the data needed to build a lazy function.
   SingletonLogger logger;
   PreParser::PreParseResult result =
-      ParseLazyFunctionBodyWithPreParser(&logger, may_abort);
+      ParseLazyFunctionBodyWithPreParser(&logger, is_inner_function, may_abort);
   // Return immediately if pre-parser decided to abort parsing.
   if (result == PreParser::kPreParseAbort) {
     scope->set_is_lazily_parsed(false);
     return kLazyParsingAborted;
   }
   if (result == PreParser::kPreParseStackOverflow) {
     // Propagate stack overflow.
     set_stack_overflow();
     *ok = false;
     return kLazyParsingComplete;
   }
   if (logger.has_error()) {
     ReportMessageAt(Scanner::Location(logger.start(), logger.end()),
                     logger.message(), logger.argument_opt(),
                     logger.error_type());
     *ok = false;
     return kLazyParsingComplete;
   }
   scope->set_end_position(logger.end());
   Expect(Token::RBRACE, CHECK_OK_VALUE(kLazyParsingComplete));
   total_preparse_skipped_ += scope->end_position() - function_block_pos;
   *materialized_literal_count = logger.literals();
   *expected_property_count = logger.properties();
   SetLanguageMode(scope, logger.language_mode());
   if (logger.uses_super_property()) scope->RecordSuperPropertyUsage();
   if (logger.calls_eval()) scope->RecordEvalCall();
-  if (produce_cached_parse_data()) {
+  if (!is_inner_function && produce_cached_parse_data()) {
     DCHECK(log_);
     // Position right after terminal '}'.
     int body_end = scanner()->location().end_pos;
     log_->LogFunction(function_block_pos, body_end, *materialized_literal_count,
                       *expected_property_count, language_mode(),
                       scope->uses_super_property(), scope->calls_eval());
   }
   return kLazyParsingComplete;
 }
 
@@ skipping 286 matching lines @@
     promise = scope()->NewTemporary(ast_value_factory()->empty_string());
     function_state_->set_promise_variable(promise);
   }
   return promise;
 }
 
 ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     const AstRawString* function_name, int pos,
     const ParserFormalParameters& parameters, FunctionKind kind,
     FunctionLiteral::FunctionType function_type, bool* ok) {
-  // Everything inside an eagerly parsed function will be parsed eagerly
-  // (see comment above).
+  // Everything inside an eagerly parsed function will be parsed eagerly (see
+  // comment above). Lazy inner functions are handled separately and they won't
+  // require the mode to be PARSE_LAZILY (see ParseFunctionLiteral).
+  // TODO(marja): Refactor parsing modes: remove this.
   ParsingModeScope parsing_mode(this, PARSE_EAGERLY);
   ZoneList<Statement*>* result = new(zone()) ZoneList<Statement*>(8, zone());
 
   static const int kFunctionNameAssignmentIndex = 0;
   if (function_type == FunctionLiteral::kNamedExpression) {
     DCHECK(function_name != NULL);
     // If we have a named function expression, we add a local variable
     // declaration to the body of the function with the name of the
     // function and let it refer to the function itself (closure).
     // Not having parsed the function body, the language mode may still change,
@@ skipping 143 matching lines @@
       statement = factory()->NewEmptyStatement(kNoSourcePosition);
     }
     result->Set(kFunctionNameAssignmentIndex, statement);
   }
 
   MarkCollectedTailCallExpressions();
   return result;
 }
 
 PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
-    SingletonLogger* logger, bool may_abort) {
+    SingletonLogger* logger, bool is_inner_function, bool may_abort) {
   // This function may be called on a background thread too; record only the
   // main thread preparse times.
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Start();
   }
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.PreParse");
 
   DCHECK_EQ(Token::LBRACE, scanner()->current_token());
 
   if (reusable_preparser_ == NULL) {
     reusable_preparser_ = new PreParser(zone(), &scanner_, ast_value_factory(),
                                         NULL, stack_limit_);
     reusable_preparser_->set_allow_lazy(true);
 #define SET_ALLOW(name) reusable_preparser_->set_allow_##name(allow_##name());
     SET_ALLOW(natives);
     SET_ALLOW(harmony_do_expressions);
     SET_ALLOW(harmony_for_in);
     SET_ALLOW(harmony_function_sent);
     SET_ALLOW(harmony_restrictive_declarations);
     SET_ALLOW(harmony_async_await);
     SET_ALLOW(harmony_trailing_commas);
     SET_ALLOW(harmony_class_fields);
 #undef SET_ALLOW
   }
-  PreParser::PreParseResult result = reusable_preparser_->PreParseLazyFunction(
-      language_mode(), function_state_->kind(),
-      scope()->AsDeclarationScope()->has_simple_parameters(), parsing_module_,
-      logger, may_abort, use_counts_);
+  // Aborting inner function preparsing would leave scopes in an inconsistent
+  // state; we don't parse inner functions in the abortable mode anyway.
+  DCHECK(!is_inner_function || !may_abort);
+
+  FunctionKind kind = function_state_->kind();
+  PreParser::PreParseResult result;
+  if (!is_inner_function) {
+    // If we don't need to look at the scope, construct a dummy scope chain

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

I don't see why this is necessary now but wasn't before? Nothing changed for
top-level parsing?

If this distinction isn't necessary, it seems like we can drop is_inner_function
as a parameter entirely and keep this function untouched.

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

This code was moved from PreParser::PreParseLazyFunction and I'd like to keep
the code w/o the flag as close to master as possible...

+    // which is not connected to the real scope chain.
+    LanguageMode mode = language_mode();
+    bool has_simple_parameters =
+        scope()->AsDeclarationScope()->has_simple_parameters();
+    DeclarationScope* top_scope = NewScriptScope();
+    top_scope->SetLanguageMode(mode);
+    FunctionState top_state(&function_state_, &scope_state_, top_scope,
+                            kNormalFunction);
+    DeclarationScope* function_scope = NewFunctionScope(kind);
+    if (!has_simple_parameters) {
+      function_scope->SetHasNonSimpleParameters();
+    }
+    result = reusable_preparser_->PreParseLazyFunction(
+        kind, function_scope, parsing_module_, logger, is_inner_function,
+        may_abort, use_counts_);
+  } else {
+    // Detaching the scopes created by PreParser from the Scope chain must be
+    // done above (see ParseFunctionLiteral & AnalyzePartially).
+    result = reusable_preparser_->PreParseLazyFunction(
+        kind, scope()->AsDeclarationScope(), parsing_module_, logger,
+        is_inner_function, may_abort, use_counts_);
+  }
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }
   return result;
 }
 
 Expression* Parser::InstallHomeObject(Expression* function_literal,
                                       Expression* home_object) {
   Block* do_block = factory()->NewBlock(nullptr, 1, false, kNoSourcePosition);
   Variable* result_var =
@@ skipping 2103 matching lines @@
 
   return final_loop;
 }
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8