Preparse inner functions.

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 2801 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, !allow_natives());
+  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();

[vogelheim, 2016/09/16 08:13:04]

Does this eliminate the force-eager-compile-for-parenthesized-functions things?

If so, we need to update the comments. If not, I don't understand the code. :)

[marja, 2016/09/16 08:38:15]

As discussed offline, the logic is intact (eager_compile_hint above).

 
-  // Determine whether the function body can be discarded after parsing.
+  // Determine whether we can still do the inner function lazy parsing.
   // 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 && FLAG_lazy && !allow_natives() &&
+
+  // Inner functions will be parsed by using a temporary Zone. After parsing, we
+  // 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 && FLAG_lazy && !allow_natives() &&
       extension_ == NULL && 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) {

[vogelheim, 2016/09/16 08:13:04]

For my understanding..

this: DCHECK(is_lazy_top_level_function != is_lazy_inner_function);
or this: DCHECK(!(is_lazy_top_level_function && is_lazy_inner_function));
?

[marja, 2016/09/16 08:38:15]

The latter will work. It's also possible both are false (e.g., when we eagerly
compile). As discussed offline, I didn't add the DCHECK. In addition, I'll
refactor these once the flag is gone; now I wanted to be extra sure that when
the flag is off, the same-thing-than-before happens, so I couldn't modify these
conditions.

       Scanner::BookmarkScope bookmark(scanner());
       bool may_abort = bookmark.Set();
-      LazyParsingResult result =
-          SkipLazyFunctionBody(&materialized_literal_count,
-                               &expected_property_count, may_abort, CHECK_OK);
+      LazyParsingResult result = SkipLazyFunctionBody(
+          &materialized_literal_count, &expected_property_count, false,
+          may_abort, CHECK_OK);
 
       materialized_literal_count += formals.materialized_literals_count +
                                     function_state.materialized_literal_count();
 
       if (result == kLazyParsingAborted) {
         bookmark.Reset();
         // 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) {

[vogelheim, 2016/09/16 08:13:04]

style nitpick: This code was awkward before, since there's several blocks of
almost doing the same thin. Maybe we can merge this and the previous if-block,
since the only difference is the assignment to body. Something like:

f (is_lazy_inner_function && FLAG_laze_...) {
  lazy parse.
} else {
  eager parser. Throw away body if is_lazy_inner_function
}

[marja, 2016/09/16 08:38:15]

Will be changed after the flag is gone.

       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) {
       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());
-  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) {
     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 281 matching lines @@
 }
 
 VariableProxy* Parser::BuildDotDebugIsActive() {
   return NewUnresolved(ast_value_factory()->dot_debug_is_active_string(), VAR);
 }
 
 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,
     // so we reserve a spot and create the actual const assignment later.
     DCHECK_EQ(kFunctionNameAssignmentIndex, result->length());
     result->Add(NULL, zone());
@@ skipping 135 matching lines @@
                                              factory()->NewThisFunction(pos),
                                              kNoSourcePosition),
                     kNoSourcePosition));
   }
 
   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);
 #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
+    // 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::ParseClassLiteral(const AstRawString* name,
                                       Scanner::Location class_name_location,
                                       bool name_is_strict_reserved, int pos,
                                       bool* ok) {
@@ skipping 1873 matching lines @@
   node->Print(Isolate::Current());
 }
 #endif  // DEBUG
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8