Implement a 'trial parse' step, that will abort pre-parsing excessively

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index fb52eef56382155d70889fa53efcc74ba8f5a700..f52e6c3b4aef8394a7d7048dfb8bce85e5d5fa69 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -4037,15 +4037,48 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
                              !parenthesized_function_);
     parenthesized_function_ = false;  // The bit was set for this function only.
 
-    if (is_lazily_parsed) {
+    // Trial parse.
+    Scanner::BookmarkScope bookmark(scanner());
+    if (is_lazily_parsed && bookmark.Set()) {
+      // We call SkipLazyFunctionBody with a bookmark, meaning that
+      // SkipLazyFunctionBody may decide to abort the skipping if it
+      // finds out the function should really have been eagerly parsed
+      // afterall.
       SkipLazyFunctionBody(function_name, &materialized_literal_count,
-                           &expected_property_count, CHECK_OK);
-    } else {
+                           &expected_property_count, ok /*CHECK_OK*/,
+                           &bookmark);
+    }
+
+    // Following the trial parse we can be in one of these states now:
+    // - we wanted to eagerly parse all along
+    //   condition: !is_lazily_parsed
+    //   action: eager parse.
+    // - we wanted to lazy parse and have not tried trial parsing
+    //   condition: is_lazily_parsed (but none of the conditions below)

[marja, 2015/04/22 16:40:55]

... is this the same as "couldn't trial parse because cannot set bookmark" or
are there some othere reasons why we "have not tried trial parsing"?

[vogelheim, 2015/04/22 17:08:34]

Yes. Cannot bookmark, or !is_lazily_parsed

+    //   action: lazy parse
+    // - we trial parsed, and trial parsing finished
+    //   condition: is_lazily_parsed && bookmark.HasBeenSet()
+    //   action: we're done.
+    // - we trial parsed, and trial parsing aborted
+    //   condition: is_lazily_parsed && bookmark.HasBeenReset()
+    //   action: eager parse.

[marja, 2015/04/22 16:40:55]

The logic sounds correct, but... isn't there a simpler way to express it? I'm
assuming we can know early that we cannot trial parse (and that the only reason
for that is not being able to set the bookmark).

So after we have trial parsed (and we know we can successfully do it),
HasBeenReset and HasBeenSet are complements, right, the first means "trial
aborted" and the latter "trial finished"?

How about:

if (is_lazily_parsed) {
  if (bookmark.Set()) { // can set bookmarks
    Skip(..); // trial
    if (trial parsing aborted) {
      is_lazily_parsed = false;
    } // else: trial parsing finished, we're done
  } else {
    Skip(...); // normal lazy parsing without bookmark
  }
}
if (!is_lazily_parsed) {
  ParseEager(..);
}

That puts the trial parsing logic into the same control flow, so we don't need
to try to figure out in retrospect whether we even tried trial parsing or not.

[vogelheim, 2015/04/22 17:08:34]

That'd work as well. I'll change it.

+    if (!is_lazily_parsed || (is_lazily_parsed && bookmark.HasBeenReset())) {
+      if (bookmark.HasBeenReset()) {
+        // TODO(vogelheim): We're lying, the function is not parenthesized.
+        //     But this will cause the compiler to actually compile it.

[marja, 2015/04/22 16:40:55]

... we might want to use the non-hacky way of saying "please compile this"
already in this CL...

+        parenthesized = FunctionLiteral::kIsParenthesized;
+      }
+
       body = ParseEagerFunctionBody(function_name, pos, fvar, fvar_init_op,
                                     kind, CHECK_OK);
       materialized_literal_count = function_state.materialized_literal_count();
       expected_property_count = function_state.expected_property_count();
       handler_count = function_state.handler_count();
+    } else if (is_lazily_parsed && bookmark.HasBeenSet()) {
+      ;  // Work was already done in trial parse, above.
+    } else {
+      SkipLazyFunctionBody(function_name, &materialized_literal_count,
+                           &expected_property_count, /*CHECK_OK*/ ok);
     }
 
     // Validate name and parameter names. We can do this only after parsing the
@@ -4094,8 +4127,9 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
 
 void Parser::SkipLazyFunctionBody(const AstRawString* function_name,
                                   int* materialized_literal_count,
-                                  int* expected_property_count,
-                                  bool* ok) {
+                                  int* expected_property_count, bool* ok,
+                                  Scanner::BookmarkScope* bookmark) {
+  DCHECK_IMPLIES(bookmark, bookmark->HasBeenSet());
   if (produce_cached_parse_data()) CHECK(log_);
 
   int function_block_pos = position();
@@ -4128,7 +4162,10 @@ void Parser::SkipLazyFunctionBody(const AstRawString* function_name,
   // AST. This gathers the data needed to build a lazy function.
   SingletonLogger logger;
   PreParser::PreParseResult result =
-      ParseLazyFunctionBodyWithPreParser(&logger);
+      ParseLazyFunctionBodyWithPreParser(&logger, bookmark);
+  if (bookmark && bookmark->HasBeenReset()) {
+    return;  // Return immediately if pre-parser devided to abort parsing.
+  }
   if (result == PreParser::kPreParseStackOverflow) {
     // Propagate stack overflow.
     set_stack_overflow();
@@ -4257,7 +4294,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
 
 
 PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
-    SingletonLogger* logger) {
+    SingletonLogger* logger, Scanner::BookmarkScope* bookmark) {
   // This function may be called on a background thread too; record only the
   // main thread preparse times.
   if (pre_parse_timer_ != NULL) {
@@ -4287,7 +4324,7 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
     reusable_preparser_->set_allow_strong_mode(allow_strong_mode());
   }
   PreParser::PreParseResult result = reusable_preparser_->PreParseLazyFunction(
-      language_mode(), function_state_->kind(), logger);
+      language_mode(), function_state_->kind(), logger, bookmark);
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }