[parser] Refactor bookmark in SkipLazyFunctionBody

src/parsing/preparser.cc

Index: src/parsing/preparser.cc
diff --git a/src/parsing/preparser.cc b/src/parsing/preparser.cc
index 0ca7e3dd66df8241e98e1fb2e87929190cb6f92c..1037a95bcc5c526d97da8fec0b5044978ed917e3 100644
--- a/src/parsing/preparser.cc
+++ b/src/parsing/preparser.cc
@@ -75,8 +75,7 @@ PreParserIdentifier PreParser::GetSymbol() const {
 
 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;
@@ -93,10 +92,10 @@ PreParser::PreParseResult PreParser::PreParseLazyFunction(
   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) {
@@ -171,15 +170,10 @@ PreParser::Statement PreParser::ParseStatementListItem(bool* ok) {
   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;
@@ -189,7 +183,8 @@ void PreParser::ParseStatementList(int end_token, bool* ok,
     }
     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();
@@ -209,7 +204,7 @@ void PreParser::ParseStatementList(int end_token, bool* ok,
                         MessageTemplate::kIllegalLanguageModeDirective,
                         "use strict");
         *ok = false;
-        return;
+        return false;
       }
     }
 
@@ -218,15 +213,14 @@ void PreParser::ParseStatementList(int end_token, bool* ok,
     // 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;
 }
 
 
@@ -1062,7 +1056,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
 
   Expect(Token::LBRACE, CHECK_OK);
   if (is_lazily_parsed) {
-    ParseLazyFunctionLiteralBody(CHECK_OK);
+    ParseLazyFunctionLiteralBody(false, CHECK_OK);
   } else {
     ParseStatementList(Token::RBRACE, CHECK_OK);
   }
@@ -1116,12 +1110,11 @@ PreParser::Expression PreParser::ParseAsyncFunctionExpression(bool* 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());
@@ -1132,6 +1125,7 @@ void PreParser::ParseLazyFunctionLiteralBody(bool* ok,
                     function_state_->materialized_literal_count(),
                     function_state_->expected_property_count(), language_mode(),
                     scope->uses_super_property(), scope->calls_eval());
+  return false;
 }
 
 PreParserExpression PreParser::ParseClassLiteral(