[parser] Refactor bookmark in SkipLazyFunctionBody

src/parsing/parser-base.h

Index: src/parsing/parser-base.h
diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
index 7580c93f36eda958a1517643ad0ee230832ffdc0..2bc4429fbe036085bc7170347f2bab057daed685 100644
--- a/src/parsing/parser-base.h
+++ b/src/parsing/parser-base.h
@@ -3362,12 +3362,15 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
     return impl()->EmptyExpression();
   }
 
-  typename Types::StatementList body;
+  typename Types::StatementList body = impl()->NullStatementList();

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

FunctionLiteral-consuming code is OK with null body?

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

Yes, this is how it works already, in the case of non-arrow function literals.

   int num_parameters = formal_parameters.scope->num_parameters();
   int materialized_literal_count = -1;
   int expected_property_count = -1;
 
   FunctionKind arrow_kind = is_async ? kAsyncArrowFunction : kArrowFunction;
+  FunctionLiteral::EagerCompileHint eager_compile_hint =
+      FunctionLiteral::kShouldLazyCompile;
+  bool should_be_used_once_hint = false;
   {
     FunctionState function_state(&function_state_, &scope_state_,
                                  formal_parameters.scope, arrow_kind);
@@ -3386,14 +3389,29 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
       bool is_lazily_parsed = (mode() == PARSE_LAZILY &&
                                formal_parameters.scope->AllowsLazyParsing());
       if (is_lazily_parsed) {
-        body = impl()->NewStatementList(0);
-        impl()->SkipLazyFunctionBody(&materialized_literal_count,
-                                     &expected_property_count, CHECK_OK);
-        if (formal_parameters.materialized_literals_count > 0) {
-          materialized_literal_count +=
-              formal_parameters.materialized_literals_count;
+        Scanner::BookmarkScope bookmark(scanner());
+        bool may_abort = bookmark.Set();
+        bool aborted = impl()->SkipLazyFunctionBody(&materialized_literal_count,
+                                                    &expected_property_count,
+                                                    may_abort, CHECK_OK);
+
+        materialized_literal_count +=
+            formal_parameters.materialized_literals_count +
+            function_state.materialized_literal_count();

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

Was this wrong before? Why did it change?

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

I changed because it was different from the case of non-arrow function literals
(parser.cc:4071).  I'm changing it back, as I'm not sure I understand the logic
of materialized literals.

+
+        if (aborted) {
+          bookmark.Reset();
+          // Trigger eager (re-)parsing, just below this block.
+          is_lazily_parsed = 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;
         }
-      } else {
+      }
+      if (!is_lazily_parsed) {
         body = impl()->ParseEagerFunctionBody(
             impl()->EmptyIdentifier(), kNoSourcePosition, formal_parameters,
             arrow_kind, FunctionLiteral::kAnonymousExpression, CHECK_OK);
@@ -3454,12 +3472,13 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
       impl()->EmptyIdentifierString(), formal_parameters.scope, body,
       materialized_literal_count, expected_property_count, num_parameters,
       FunctionLiteral::kNoDuplicateParameters,
-      FunctionLiteral::kAnonymousExpression,
-      FunctionLiteral::kShouldLazyCompile, arrow_kind,
+      FunctionLiteral::kAnonymousExpression, eager_compile_hint, arrow_kind,
       formal_parameters.scope->start_position());
 
   function_literal->set_function_token_position(
       formal_parameters.scope->start_position());
+  if (should_be_used_once_hint)
+    function_literal->set_should_be_used_once_hint();

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

Same style nit, please add { }

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

Done.  But look at the very next line...  :-)

[adamk, 2016/08/31 18:08:53]

The difference is whether the entire if-statement fits on a single line. Sadly I
don't think this is written down anywhere, but it's the generally-followed
practice in v8 from what I can tell (and I'm scared of non-curly-wrapped
if-statements :)

 
   if (fni_ != NULL) impl()->InferFunctionName(fni_, function_literal);