[parser] Refactor bookmark in SkipLazyFunctionBody

src/parsing/parser-base.h

 // 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.
 
 #ifndef V8_PARSING_PARSER_BASE_H
 #define V8_PARSING_PARSER_BASE_H
 
 #include "src/ast/scopes.h"
 #include "src/bailout-reason.h"
 #include "src/base/hashmap.h"
@@ skipping 3344 matching lines @@
     bool* ok) {
   if (peek() == Token::ARROW && scanner_->HasAnyLineTerminatorBeforeNext()) {
     // ASI inserts `;` after arrow parameters if a line terminator is found.
     // `=> ...` is never a valid expression, so report as syntax error.
     // If next token is not `=>`, it's a syntax error anyways.
     ReportUnexpectedTokenAt(scanner_->peek_location(), Token::ARROW);
     *ok = false;
     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);
 
     function_state.SkipMaterializedLiterals(
         formal_parameters.materialized_literals_count);
 
     impl()->ReindexLiterals(formal_parameters);
 
     Expect(Token::ARROW, CHECK_OK);
 
     if (peek() == Token::LBRACE) {
       // Multiple statement body
       Consume(Token::LBRACE);
       DCHECK_EQ(scope(), formal_parameters.scope);
       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);
         materialized_literal_count =
             function_state.materialized_literal_count();
         expected_property_count = function_state.expected_property_count();
       }
     } else {
       // Single-expression body
       int pos = position();
@@ skipping 40 matching lines @@
     }
     impl()->CheckConflictingVarDeclarations(formal_parameters.scope, CHECK_OK);
 
     impl()->RewriteDestructuringAssignments();
   }
 
   FunctionLiteralT function_literal = factory()->NewFunctionLiteral(
       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);
 
   return function_literal;
 }
 
 template <typename Impl>
 typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseTemplateLiteral(
     ExpressionT tag, int start, bool* ok) {
   // A TemplateLiteral is made up of 0 or more TEMPLATE_SPAN tokens (literal
@@ skipping 195 matching lines @@
     has_seen_constructor_ = true;
     return;
   }
 }
 
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_PARSING_PARSER_BASE_H