[parser] Refactor bookmark in SkipLazyFunctionBody

src/parsing/preparser.cc

 // Copyright 2011 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 <cmath>
 
 #include "src/allocation.h"
 #include "src/base/logging.h"
 #include "src/conversions-inl.h"
 #include "src/conversions.h"
@@ skipping 57 matching lines @@
       if (scanner()->LiteralMatches("prototype", 9))
         return PreParserIdentifier::Prototype();
       if (scanner()->LiteralMatches("constructor", 11))
         return PreParserIdentifier::Constructor();
       return PreParserIdentifier::Default();
   }
 }
 
 PreParser::PreParseResult PreParser::PreParseLazyFunction(
     LanguageMode language_mode, FunctionKind kind, bool has_simple_parameters,
-    bool parsing_module, ParserRecorder* log, Scanner::BookmarkScope* bookmark,
-    int* use_counts) {
+    bool parsing_module, ParserRecorder* log, bool may_abort, int* use_counts) {
   parsing_module_ = parsing_module;
   log_ = log;
   use_counts_ = use_counts;
   // Lazy functions always have trivial outer scopes (no with/catch scopes).
   DCHECK_NULL(scope_state_);
   DeclarationScope* top_scope = NewScriptScope();
   FunctionState top_state(&function_state_, &scope_state_, top_scope,
                           kNormalFunction);
   scope()->SetLanguageMode(language_mode);
   DeclarationScope* function_scope = NewFunctionScope(kind);
   if (!has_simple_parameters) function_scope->SetHasNonSimpleParameters();
   FunctionState function_state(&function_state_, &scope_state_, function_scope,
                                kind);
   DCHECK_EQ(Token::LBRACE, scanner()->current_token());
   bool ok = true;
   int start_position = peek_position();
-  ParseLazyFunctionLiteralBody(&ok, bookmark);
+  bool aborted = ParseLazyFunctionLiteralBody(may_abort, &ok);
   use_counts_ = nullptr;
-  if (bookmark && bookmark->HasBeenReset()) {
-    // Do nothing, as we've just aborted scanning this function.
+  if (aborted) {
+    return kPreParseAbort;
   } else if (stack_overflow()) {
     return kPreParseStackOverflow;
   } else if (!ok) {
     ReportUnexpectedToken(scanner()->current_token());
   } else {
     DCHECK_EQ(Token::RBRACE, scanner()->peek());
     if (is_strict(scope()->language_mode())) {
       int end_pos = scanner()->location().end_pos;
       CheckStrictOctalLiteral(start_position, end_pos, &ok);
       CheckDecimalLiteralWithLeadingZero(use_counts, start_position, end_pos);
@@ skipping 54 matching lines @@
         Consume(Token::ASYNC);
         return ParseAsyncFunctionDeclaration(ok);
       }
     /* falls through */
     default:
       break;
   }
   return ParseStatement(kAllowLabelledFunctionStatement, ok);
 }
 
-
-void PreParser::ParseStatementList(int end_token, bool* ok,
-                                   Scanner::BookmarkScope* bookmark) {
+bool PreParser::ParseStatementList(int end_token, bool may_abort, bool* ok) {
   // SourceElements ::
   //   (Statement)* <end_token>
 
-  // Bookkeeping for trial parse if bookmark is set:
-  DCHECK_IMPLIES(bookmark, bookmark->HasBeenSet());
-  bool maybe_reset = bookmark != nullptr;
   int count_statements = 0;
 
   bool directive_prologue = true;
   while (peek() != end_token) {
     if (directive_prologue && peek() != Token::STRING) {
       directive_prologue = false;
     }
     bool starts_with_identifier = peek() == Token::IDENTIFIER;
     Scanner::Location token_loc = scanner()->peek_location();
-    Statement statement = ParseStatementListItem(CHECK_OK_CUSTOM(Void));
+    Statement statement = ParseStatementListItem(ok);
+    if (!*ok) return false;
 
     if (directive_prologue) {
       bool use_strict_found = statement.IsUseStrictLiteral();
 
       if (use_strict_found) {
         scope()->SetLanguageMode(
             static_cast<LanguageMode>(scope()->language_mode() | STRICT));
       } else if (!statement.IsStringLiteral()) {
         directive_prologue = false;
       }
 
       if (use_strict_found && !scope()->HasSimpleParameters()) {
         // TC39 deemed "use strict" directives to be an error when occurring
         // in the body of a function with non-simple parameter list, on
         // 29/7/2015. https://goo.gl/ueA7Ln
         ReportMessageAt(token_loc,
                         MessageTemplate::kIllegalLanguageModeDirective,
                         "use strict");
         *ok = false;
-        return;
+        return false;
       }
     }
 
     // If we're allowed to reset to a bookmark, we will do so when we see a long
     // and trivial function.
     // Our current definition of 'long and trivial' is:
     // - over 200 statements
     // - all starting with an identifier (i.e., no if, for, while, etc.)
-    if (maybe_reset && (!starts_with_identifier ||
-                        ++count_statements > kLazyParseTrialLimit)) {
-      if (count_statements > kLazyParseTrialLimit) {
-        bookmark->Reset();
-        return;
-      }
-      maybe_reset = false;
+    if (may_abort) {
+      if (!starts_with_identifier)

[adamk, 2016/08/30 22:13:14]

Please add curly braces around the if/else statements, which is the usual style
for multi-line if statements in V8.

[nickie, 2016/08/31 13:23:36]

Done.

+        may_abort = false;
+      else if (++count_statements > kLazyParseTrialLimit)
+        return true;
     }
   }
+  return false;
 }
 
 
 PreParser::Statement PreParser::ParseStatement(
     AllowLabelledFunctionStatement allow_function, bool* ok) {
   // Statement ::
   //   EmptyStatement
   //   ...
 
   if (peek() == Token::SEMICOLON) {
@@ skipping 815 matching lines @@
   CheckArityRestrictions(formals.arity, kind, formals.has_rest, start_position,
                          formals_end_position, CHECK_OK);
 
   // See Parser::ParseFunctionLiteral for more information about lazy parsing
   // and lazy compilation.
   bool is_lazily_parsed = (outer_is_script_scope && allow_lazy() &&
                            !function_state_->this_function_is_parenthesized());
 
   Expect(Token::LBRACE, CHECK_OK);
   if (is_lazily_parsed) {
-    ParseLazyFunctionLiteralBody(CHECK_OK);
+    ParseLazyFunctionLiteralBody(false, CHECK_OK);
   } else {
     ParseStatementList(Token::RBRACE, CHECK_OK);
   }
   Expect(Token::RBRACE, CHECK_OK);
 
   // Parsing the body may change the language mode in our scope.
   language_mode = function_scope->language_mode();
 
   // Validate name and parameter names. We can do this only after parsing the
   // function, since the function can declare itself strict.
@@ skipping 33 matching lines @@
   }
 
   ParseFunctionLiteral(name, scanner()->location(),
                        is_strict_reserved ? kFunctionNameIsStrictReserved
                                           : kFunctionNameValidityUnknown,
                        FunctionKind::kAsyncFunction, pos, type, language_mode(),
                        CHECK_OK);
   return Expression::Default();
 }
 
-void PreParser::ParseLazyFunctionLiteralBody(bool* ok,
-                                             Scanner::BookmarkScope* bookmark) {
+bool PreParser::ParseLazyFunctionLiteralBody(bool may_abort, bool* ok) {
   int body_start = position();
-  ParseStatementList(Token::RBRACE, ok, bookmark);
-  if (!*ok) return;
-  if (bookmark && bookmark->HasBeenReset()) return;
+  bool aborted = ParseStatementList(Token::RBRACE, may_abort, ok);
+  if (!*ok) return false;
+  if (aborted) return true;

[adamk, 2016/08/30 22:13:14]

And having gotten this far I also think it would be more readable to have two
enums, all these bools are really hard to keep in my head.

[nickie, 2016/08/31 13:23:36]

Done.

 
   // Position right after terminal '}'.
   DCHECK_EQ(Token::RBRACE, scanner()->peek());
   int body_end = scanner()->peek_location().end_pos;
   DeclarationScope* scope = this->scope()->AsDeclarationScope();
   DCHECK(scope->is_function_scope());
   log_->LogFunction(body_start, body_end,
                     function_state_->materialized_literal_count(),
                     function_state_->expected_property_count(), language_mode(),
                     scope->uses_super_property(), scope->calls_eval());
+  return false;
 }
 
 PreParserExpression PreParser::ParseClassLiteral(
     PreParserIdentifier name, Scanner::Location class_name_location,
     bool name_is_strict_reserved, int pos, bool* ok) {
   // All parts of a ClassDeclaration and ClassExpression are strict code.
   if (name_is_strict_reserved) {
     ReportMessageAt(class_name_location,
                     MessageTemplate::kUnexpectedStrictReserved);
     *ok = false;
@@ skipping 88 matching lines @@
 
   body->Add(PreParserStatement::ExpressionStatement(return_value), zone());
 }
 
 #undef CHECK_OK
 #undef CHECK_OK_CUSTOM
 
 
 }  // namespace internal
 }  // namespace v8