Preparse lazy function parameters

Parameters of a lazily parsed function used to be parsed eagerly, and parameter
handling was split between Parser::ParseFunctionLiteral and
ParseEagerFunctionBody, leading to inconsistencies.

After this CL, we preparse (lazy parse) the parameters of lazily parsed
functions.

(For arrow functions, we cannot do that ofc.)

This is needed for later features (PreParser with scope analysis).

-- CL adapted from marja's https://codereview.chromium.org/2411793003/

BUG=

Review-Url: https://codereview.chromium.org/2472063002
Cr-Commit-Position: refs/heads/master@{#40771}

src/ast/scopes.cc

@@ -1261,6 +1261,7 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   DCHECK(is_function_scope());
 
   // Reset all non-trivial members.
+  params_.Clear();
   decls_.Clear();
   locals_.Clear();
   sloppy_block_function_map_.Clear();
@@ -1269,28 +1270,8 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   inner_scope_ = nullptr;
   unresolved_ = nullptr;
 
-  // TODO(verwaest): We should properly preparse the parameters (no declarations
-  // should be created), and reparse on abort.
-  if (aborted) {
-    if (!IsArrowFunction(function_kind_)) {
-      DeclareDefaultFunctionVariables(ast_value_factory);
-    }
-    // Recreate declarations for parameters.
-    for (int i = 0; i < params_.length(); i++) {
-      Variable* var = params_[i];
-      if (var->mode() == TEMPORARY) {
-        // TODO(verwaest): Remove and unfriend DeclarationScope from Variable.
-        *var->next() = nullptr;
-        locals_.Add(var);
-      } else if (variables_.Lookup(var->raw_name()) == nullptr) {
-        // TODO(verwaest): Remove and unfriend DeclarationScope from Variable.
-        *var->next() = nullptr;
-        variables_.Add(zone(), var);
-        locals_.Add(var);
-      }
-    }
-  } else {
-    params_.Rewind(0);
+  if (aborted && !IsArrowFunction(function_kind_)) {
+    DeclareDefaultFunctionVariables(ast_value_factory);
   }
 
 #ifdef DEBUG

src/ast/variables.h

@@ -152,9 +152,6 @@ class Variable final : public ZoneObject {
                           2> {};
   Variable** next() { return &next_; }
   friend List;
-  // To reset next to nullptr upon resetting after preparsing.
-  // TODO(verwaest): Remove once we properly preparse parameters.
-  friend class DeclarationScope;
 };
 }  // namespace internal
 }  // namespace v8

src/parsing/parser-base.h

@@ -3953,12 +3953,22 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
 
     if (peek() == Token::LBRACE) {
       // Multiple statement body
-      Consume(Token::LBRACE);
       DCHECK_EQ(scope(), formal_parameters.scope);
       if (is_lazy_top_level_function) {
+        // FIXME(marja): Arrow function parameters will be parsed even if the
+        // body is preparsed; move relevant parts of parameter handling to
+        // simulate consistent parameter handling.
         Scanner::BookmarkScope bookmark(scanner());
         bookmark.Set();
-        LazyParsingResult result = impl()->SkipLazyFunctionBody(
+        // For arrow functions, we don't need to retrieve data about function
+        // parameters.
+        int dummy_num_parameters = -1;
+        int dummy_function_length = -1;
+        bool dummy_has_duplicate_parameters = false;
+        DCHECK((kind & FunctionKind::kArrowFunction) != 0);
+        LazyParsingResult result = impl()->SkipFunction(
+            kind, formal_parameters.scope, &dummy_num_parameters,
+            &dummy_function_length, &dummy_has_duplicate_parameters,
             &materialized_literal_count, &expected_property_count, false, true,
             CHECK_OK);
         formal_parameters.scope->ResetAfterPreparsing(
@@ -3982,6 +3992,7 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
         }
       }
       if (!is_lazy_top_level_function) {
+        Consume(Token::LBRACE);
         body = impl()->ParseEagerFunctionBody(
             impl()->EmptyIdentifier(), kNoSourcePosition, formal_parameters,
             kind, FunctionLiteral::kAnonymousExpression, CHECK_OK);

src/parsing/parser.cc

@@ -2523,8 +2523,6 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   int pos = function_token_pos == kNoSourcePosition ? peek_position()
                                                     : function_token_pos;
 
-  bool is_generator = IsGeneratorFunction(kind);
-
   // Anonymous functions were passed either the empty symbol or a null
   // handle as the function name.  Remember if we were passed a non-empty
   // handle to decide whether to invoke function name inference.
@@ -2614,34 +2612,20 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   // This Scope lives in the main zone. We'll migrate data into that zone later.
   DeclarationScope* scope = NewFunctionScope(kind);
   SetLanguageMode(scope, language_mode);
+#ifdef DEBUG
+  scope->SetScopeName(function_name);
+#endif
 
   ZoneList<Statement*>* body = nullptr;
   int materialized_literal_count = -1;
   int expected_property_count = -1;
-  DuplicateFinder duplicate_finder(scanner()->unicode_cache());
   bool should_be_used_once_hint = false;
-  bool has_duplicate_parameters;
-
-  FunctionState function_state(&function_state_, &scope_state_, scope);
-#ifdef DEBUG
-  scope->SetScopeName(function_name);
-#endif
-
-  ExpressionClassifier formals_classifier(this, &duplicate_finder);
-
-  if (is_generator) PrepareGeneratorVariables(&function_state);
+  int num_parameters = -1;
+  int function_length = -1;
+  bool has_duplicate_parameters = false;
 
   Expect(Token::LPAREN, CHECK_OK);
-  int start_position = scanner()->location().beg_pos;
-  this->scope()->set_start_position(start_position);
-  ParserFormalParameters formals(scope);
-  ParseFormalParameterList(&formals, CHECK_OK);
-  Expect(Token::RPAREN, CHECK_OK);
-  int formals_end_position = scanner()->location().end_pos;
-
-  CheckArityRestrictions(formals.arity, kind, formals.has_rest, start_position,
-                         formals_end_position, CHECK_OK);
-  Expect(Token::LBRACE, CHECK_OK);
+  scope->set_start_position(scanner()->location().beg_pos);
 
   {
     // Temporary zones can nest. When we migrate free variables (see below), we
@@ -2661,19 +2645,18 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
 #endif
 
     // Eager or lazy parse? If is_lazy_top_level_function, we'll parse
-    // lazily. We'll call SkipLazyFunctionBody, which may decide to abort lazy
-    // parsing if it suspects that wasn't a good idea. If so (in which case the
-    // parser is expected to have backtracked), or if we didn't try to lazy
-    // parse in the first place, we'll have to parse eagerly.
+    // lazily. We'll call SkipFunction, which may decide to
+    // abort lazy parsing if it suspects that wasn't a good idea. If so (in
+    // which case the parser is expected to have backtracked), or if we didn't
+    // try to lazy parse in the first place, we'll have to parse eagerly.
     if (is_lazy_top_level_function || is_lazy_inner_function) {
       Scanner::BookmarkScope bookmark(scanner());
       bookmark.Set();
-      LazyParsingResult result = SkipLazyFunctionBody(
-          &materialized_literal_count, &expected_property_count,
-          is_lazy_inner_function, is_lazy_top_level_function, CHECK_OK);
-
-      materialized_literal_count += formals.materialized_literals_count +
-                                    function_state.materialized_literal_count();
+      LazyParsingResult result =
+          SkipFunction(kind, scope, &num_parameters, &function_length,
+                       &has_duplicate_parameters, &materialized_literal_count,
+                       &expected_property_count, is_lazy_inner_function,
+                       is_lazy_top_level_function, CHECK_OK);
 
       if (result == kLazyParsingAborted) {
         DCHECK(is_lazy_top_level_function);
@@ -2693,11 +2676,10 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
     }
 
     if (!is_lazy_top_level_function && !is_lazy_inner_function) {
-      body = ParseEagerFunctionBody(function_name, pos, formals, kind,
-                                    function_type, CHECK_OK);
-
-      materialized_literal_count = function_state.materialized_literal_count();
-      expected_property_count = function_state.expected_property_count();
+      body = ParseFunction(
+          function_name, pos, kind, function_type, scope, &num_parameters,
+          &function_length, &has_duplicate_parameters,
+          &materialized_literal_count, &expected_property_count, CHECK_OK);
     }
 
     DCHECK(use_temp_zone || !is_lazy_top_level_function);
@@ -2717,17 +2699,11 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
              function_name->byte_length(), function_name->raw_data());
     }
 
-    // Parsing the body may change the language mode in our scope.
+    // Validate function name. We can do this only after parsing the function,
+    // since the function can declare itself strict.
     language_mode = scope->language_mode();
-
-    // Validate name and parameter names. We can do this only after parsing the
-    // function, since the function can declare itself strict.
     CheckFunctionName(language_mode, function_name, function_name_validity,
                       function_name_location, CHECK_OK);
-    const bool allow_duplicate_parameters =
-        is_sloppy(language_mode) && formals.is_simple && !IsConciseMethod(kind);
-    ValidateFormalParameters(language_mode, allow_duplicate_parameters,
-                             CHECK_OK);
 
     if (is_strict(language_mode)) {
       CheckStrictOctalLiteral(scope->start_position(), scope->end_position(),
@@ -2736,13 +2712,6 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
                                          scope->end_position());
     }
     CheckConflictingVarDeclarations(scope, CHECK_OK);
-
-    if (body) {
-      // If body can be inspected, rewrite queued destructuring assignments
-      RewriteDestructuringAssignments();
-    }
-    has_duplicate_parameters =
-        !classifier()->is_valid_formal_parameter_list_without_duplicates();
   }  // DiscardableZoneScope goes out of scope.
 
   FunctionLiteral::ParameterFlag duplicate_parameters =
@@ -2752,9 +2721,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   // Note that the FunctionLiteral needs to be created in the main Zone again.
   FunctionLiteral* function_literal = factory()->NewFunctionLiteral(
       function_name, scope, body, materialized_literal_count,
-      expected_property_count, formals.num_parameters(),
-      formals.function_length, duplicate_parameters, function_type,
-      eager_compile_hint, pos);
+      expected_property_count, num_parameters, function_length,
+      duplicate_parameters, function_type, eager_compile_hint, pos);
   function_literal->set_function_token_position(function_token_pos);
   if (should_be_used_once_hint)
     function_literal->set_should_be_used_once_hint();
@@ -2766,35 +2734,42 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   return function_literal;
 }
 
-Parser::LazyParsingResult Parser::SkipLazyFunctionBody(
+Parser::LazyParsingResult Parser::SkipFunction(
+    FunctionKind kind, DeclarationScope* function_scope, int* num_parameters,
+    int* function_length, bool* has_duplicate_parameters,
     int* materialized_literal_count, int* expected_property_count,
     bool is_inner_function, bool may_abort, bool* ok) {
+  DCHECK_NE(kNoSourcePosition, function_scope->start_position());
   if (produce_cached_parse_data()) CHECK(log_);
 
-  int function_block_pos = position();
-  DeclarationScope* scope = function_state_->scope();
-  DCHECK(scope->is_function_scope());
+  DCHECK_IMPLIES(IsArrowFunction(kind),
+                 scanner()->current_token() == Token::ARROW);
+
   // Inner functions are not part of the cached data.
   if (!is_inner_function && consume_cached_parse_data() &&
       !cached_parse_data_->rejected()) {
-    // If we have cached data, we use it to skip parsing the function body. The
-    // data contains the information we need to construct the lazy function.
+    // If we have cached data, we use it to skip parsing the function. The data
+    // contains the information we need to construct the lazy function.
     FunctionEntry entry =
-        cached_parse_data_->GetFunctionEntry(function_block_pos);
+        cached_parse_data_->GetFunctionEntry(function_scope->start_position());
     // Check that cached data is valid. If not, mark it as invalid (the embedder
     // handles it). Note that end position greater than end of stream is safe,
     // and hard to check.
-    if (entry.is_valid() && entry.end_pos() > function_block_pos) {
+    if (entry.is_valid() &&
+        entry.end_pos() > function_scope->start_position()) {
+      total_preparse_skipped_ += entry.end_pos() - position();
+      function_scope->set_end_position(entry.end_pos());
       scanner()->SeekForward(entry.end_pos() - 1);
-
-      scope->set_end_position(entry.end_pos());
       Expect(Token::RBRACE, CHECK_OK_VALUE(kLazyParsingComplete));
-      total_preparse_skipped_ += scope->end_position() - function_block_pos;
+      *num_parameters = entry.num_parameters();
+      *function_length = entry.function_length();
+      *has_duplicate_parameters = entry.has_duplicate_parameters();
       *materialized_literal_count = entry.literal_count();
       *expected_property_count = entry.property_count();
-      SetLanguageMode(scope, entry.language_mode());
-      if (entry.uses_super_property()) scope->RecordSuperPropertyUsage();
-      if (entry.calls_eval()) scope->RecordEvalCall();
+      SetLanguageMode(function_scope, entry.language_mode());
+      if (entry.uses_super_property())
+        function_scope->RecordSuperPropertyUsage();
+      if (entry.calls_eval()) function_scope->RecordEvalCall();
       return kLazyParsingComplete;
     }
     cached_parse_data_->Reject();
@@ -2802,8 +2777,8 @@ Parser::LazyParsingResult Parser::SkipLazyFunctionBody(
   // With no cached data, we partially parse the function, without building an
   // AST. This gathers the data needed to build a lazy function.
   SingletonLogger logger;
-  PreParser::PreParseResult result =
-      ParseFunctionBodyWithPreParser(&logger, is_inner_function, may_abort);
+  PreParser::PreParseResult result = ParseFunctionWithPreParser(
+      kind, function_scope, &logger, is_inner_function, may_abort);
 
   // Return immediately if pre-parser decided to abort parsing.
   if (result == PreParser::kPreParseAbort) return kLazyParsingAborted;
@@ -2820,21 +2795,25 @@ Parser::LazyParsingResult Parser::SkipLazyFunctionBody(
     *ok = false;
     return kLazyParsingComplete;
   }
-  scope->set_end_position(logger.end());
+  function_scope->set_end_position(logger.end());
   Expect(Token::RBRACE, CHECK_OK_VALUE(kLazyParsingComplete));
-  total_preparse_skipped_ += scope->end_position() - function_block_pos;
+  total_preparse_skipped_ +=
+      function_scope->end_position() - function_scope->start_position();
+  *num_parameters = logger.num_parameters();
+  *function_length = logger.function_length();
+  *has_duplicate_parameters = logger.has_duplicate_parameters();
   *materialized_literal_count = logger.literals();
   *expected_property_count = logger.properties();
-  SetLanguageMode(scope, logger.language_mode());
-  if (logger.uses_super_property()) scope->RecordSuperPropertyUsage();
-  if (logger.calls_eval()) scope->RecordEvalCall();
+  SetLanguageMode(function_scope, logger.language_mode());
+  if (logger.uses_super_property()) function_scope->RecordSuperPropertyUsage();
+  if (logger.calls_eval()) function_scope->RecordEvalCall();
   if (!is_inner_function && produce_cached_parse_data()) {
     DCHECK(log_);
-    // Position right after terminal '}'.
-    int body_end = scanner()->location().end_pos;
-    log_->LogFunction(function_block_pos, body_end, *materialized_literal_count,
-                      *expected_property_count, language_mode(),
-                      scope->uses_super_property(), scope->calls_eval());
+    log_->LogFunction(
+        function_scope->start_position(), function_scope->end_position(),
+        *num_parameters, *function_length, *has_duplicate_parameters,
+        *materialized_literal_count, *expected_property_count, language_mode(),
+        function_scope->uses_super_property(), function_scope->calls_eval());
   }
   return kLazyParsingComplete;
 }
@@ -3107,6 +3086,52 @@ Expression* Parser::BuildInitialYield(int pos, FunctionKind kind) {
                              Yield::kOnExceptionThrow);
 }
 
+ZoneList<Statement*>* Parser::ParseFunction(
+    const AstRawString* function_name, int pos, FunctionKind kind,
+    FunctionLiteral::FunctionType function_type,
+    DeclarationScope* function_scope, int* num_parameters, int* function_length,
+    bool* has_duplicate_parameters, int* materialized_literal_count,
+    int* expected_property_count, bool* ok) {
+  FunctionState function_state(&function_state_, &scope_state_, function_scope);
+
+  DuplicateFinder duplicate_finder(scanner()->unicode_cache());
+  ExpressionClassifier formals_classifier(this, &duplicate_finder);
+
+  if (IsGeneratorFunction(kind)) PrepareGeneratorVariables(&function_state);
+
+  ParserFormalParameters formals(function_scope);
+  ParseFormalParameterList(&formals, CHECK_OK);
+  Expect(Token::RPAREN, CHECK_OK);
+  int formals_end_position = scanner()->location().end_pos;
+  *num_parameters = formals.num_parameters();
+  *function_length = formals.function_length;
+
+  CheckArityRestrictions(formals.arity, kind, formals.has_rest,
+                         function_scope->start_position(), formals_end_position,
+                         CHECK_OK);
+  Expect(Token::LBRACE, CHECK_OK);
+
+  ZoneList<Statement*>* body = ParseEagerFunctionBody(
+      function_name, pos, formals, kind, function_type, ok);
+
+  // Validate parameter names. We can do this only after parsing the function,
+  // since the function can declare itself strict.
+  const bool allow_duplicate_parameters =
+      is_sloppy(function_scope->language_mode()) && formals.is_simple &&
+      !IsConciseMethod(kind);
+  ValidateFormalParameters(function_scope->language_mode(),
+                           allow_duplicate_parameters, CHECK_OK);
+
+  RewriteDestructuringAssignments();
+
+  *has_duplicate_parameters =
+      !classifier()->is_valid_formal_parameter_list_without_duplicates();
+
+  *materialized_literal_count = function_state.materialized_literal_count();
+  *expected_property_count = function_state.expected_property_count();
+  return body;
+}
+
 ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     const AstRawString* function_name, int pos,
     const ParserFormalParameters& parameters, FunctionKind kind,
@@ -3263,12 +3288,11 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
   return result;
 }
 
-PreParser::PreParseResult Parser::ParseFunctionBodyWithPreParser(
+PreParser::PreParseResult Parser::ParseFunctionWithPreParser(
+    FunctionKind kind, DeclarationScope* function_scope,
     SingletonLogger* logger, bool is_inner_function, bool may_abort) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.PreParse");
 
-  DCHECK_EQ(Token::LBRACE, scanner()->current_token());
-
   if (reusable_preparser_ == NULL) {
     reusable_preparser_ = new PreParser(zone(), &scanner_, ast_value_factory(),
                                         NULL, stack_limit_);
@@ -3287,10 +3311,9 @@ PreParser::PreParseResult Parser::ParseFunctionBodyWithPreParser(
   // state; we don't parse inner functions in the abortable mode anyway.
   DCHECK(!is_inner_function || !may_abort);
 
-  DeclarationScope* function_scope = function_state_->scope();
   PreParser::PreParseResult result = reusable_preparser_->PreParseFunction(
-      function_scope, parsing_module_, logger, is_inner_function, may_abort,
-      use_counts_);
+      kind, function_scope, parsing_module_, logger, is_inner_function,
+      may_abort, use_counts_);
   return result;
 }
 

src/parsing/parser.h

@@ -31,11 +31,11 @@ class FunctionEntry BASE_EMBEDDED {
   enum {
     kStartPositionIndex,
     kEndPositionIndex,
+    kNumParametersIndex,
+    kFunctionLengthIndex,
     kLiteralCountIndex,
     kPropertyCountIndex,
-    kLanguageModeIndex,
-    kUsesSuperPropertyIndex,
-    kCallsEvalIndex,
+    kFlagsIndex,
     kSize
   };
 
@@ -44,18 +44,43 @@ class FunctionEntry BASE_EMBEDDED {
 
   FunctionEntry() : backing_() { }
 
-  int start_pos() { return backing_[kStartPositionIndex]; }
-  int end_pos() { return backing_[kEndPositionIndex]; }
-  int literal_count() { return backing_[kLiteralCountIndex]; }
-  int property_count() { return backing_[kPropertyCountIndex]; }
-  LanguageMode language_mode() {
-    DCHECK(is_valid_language_mode(backing_[kLanguageModeIndex]));
-    return static_cast<LanguageMode>(backing_[kLanguageModeIndex]);
+  class LanguageModeField : public BitField<LanguageMode, 0, 1> {};
+  class UsesSuperPropertyField
+      : public BitField<bool, LanguageModeField::kNext, 1> {};
+  class CallsEvalField
+      : public BitField<bool, UsesSuperPropertyField::kNext, 1> {};
+  class HasDuplicateParametersField
+      : public BitField<bool, CallsEvalField::kNext, 1> {};
+
+  static uint32_t EncodeFlags(LanguageMode language_mode,
+                              bool uses_super_property, bool calls_eval,
+                              bool has_duplicate_parameters) {
+    return LanguageModeField::encode(language_mode) |
+           UsesSuperPropertyField::encode(uses_super_property) |
+           CallsEvalField::encode(calls_eval) |
+           HasDuplicateParametersField::encode(has_duplicate_parameters);
+  }
+
+  int start_pos() const { return backing_[kStartPositionIndex]; }
+  int end_pos() const { return backing_[kEndPositionIndex]; }
+  int num_parameters() const { return backing_[kNumParametersIndex]; }
+  int function_length() const { return backing_[kFunctionLengthIndex]; }
+  int literal_count() const { return backing_[kLiteralCountIndex]; }
+  int property_count() const { return backing_[kPropertyCountIndex]; }
+  LanguageMode language_mode() const {
+    return LanguageModeField::decode(backing_[kFlagsIndex]);
+  }
+  bool uses_super_property() const {
+    return UsesSuperPropertyField::decode(backing_[kFlagsIndex]);
+  }
+  bool calls_eval() const {
+    return CallsEvalField::decode(backing_[kFlagsIndex]);
+  }
+  bool has_duplicate_parameters() const {
+    return HasDuplicateParametersField::decode(backing_[kFlagsIndex]);
   }
-  bool uses_super_property() { return backing_[kUsesSuperPropertyIndex]; }
-  bool calls_eval() { return backing_[kCallsEvalIndex]; }
 
-  bool is_valid() { return !backing_.is_empty(); }
+  bool is_valid() const { return !backing_.is_empty(); }
 
  private:
   Vector<unsigned> backing_;
@@ -490,13 +515,17 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   // by parsing the function with PreParser. Consumes the ending }.
   // If may_abort == true, the (pre-)parser may decide to abort skipping
   // in order to force the function to be eagerly parsed, after all.
-  LazyParsingResult SkipLazyFunctionBody(int* materialized_literal_count,
-                                         int* expected_property_count,
-                                         bool is_inner_function, bool may_abort,
-                                         bool* ok);
-
-  PreParser::PreParseResult ParseFunctionBodyWithPreParser(
-      SingletonLogger* logger, bool is_inner_function, bool may_abort);
+  LazyParsingResult SkipFunction(
+      FunctionKind kind, DeclarationScope* function_scope, int* num_parameters,
+      int* function_length, bool* has_duplicate_parameters,
+      int* materialized_literal_count, int* expected_property_count,
+      bool is_inner_function, bool may_abort, bool* ok);
+
+  PreParser::PreParseResult ParseFunctionWithPreParser(FunctionKind kind,
+                                                       DeclarationScope* scope,
+                                                       SingletonLogger* logger,
+                                                       bool is_inner_function,
+                                                       bool may_abort);
 
   Block* BuildParameterInitializationBlock(
       const ParserFormalParameters& parameters, bool* ok);
@@ -508,6 +537,13 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       const ParserFormalParameters& parameters, FunctionKind kind,
       FunctionLiteral::FunctionType function_type, bool* ok);
 
+  ZoneList<Statement*>* ParseFunction(
+      const AstRawString* function_name, int pos, FunctionKind kind,
+      FunctionLiteral::FunctionType function_type,
+      DeclarationScope* function_scope, int* num_parameters,
+      int* function_length, bool* has_duplicate_parameters,
+      int* materialized_literal_count, int* expected_property_count, bool* ok);
+
   void ThrowPendingError(Isolate* isolate, Handle<Script> script);
 
   class TemplateLiteral : public ZoneObject {

src/parsing/preparse-data-format.h

@@ -14,7 +14,7 @@ struct PreparseDataConstants {
  public:
   // Layout and constants of the preparse data exchange format.
   static const unsigned kMagicNumber = 0xBadDead;
-  static const unsigned kCurrentVersion = 11;
+  static const unsigned kCurrentVersion = 12;
 
   static const int kMagicOffset = 0;
   static const int kVersionOffset = 1;

src/parsing/preparse-data.cc

@@ -12,6 +12,20 @@
 namespace v8 {
 namespace internal {
 
+void CompleteParserRecorder::LogFunction(
+    int start, int end, int num_parameters, int function_length,
+    bool has_duplicate_parameters, int literals, int properties,
+    LanguageMode language_mode, bool uses_super_property, bool calls_eval) {
+  function_store_.Add(start);
+  function_store_.Add(end);
+  function_store_.Add(num_parameters);
+  function_store_.Add(function_length);
+  function_store_.Add(literals);
+  function_store_.Add(properties);
+  function_store_.Add(
+      FunctionEntry::EncodeFlags(language_mode, uses_super_property, calls_eval,
+                                 has_duplicate_parameters));
+}
 
 CompleteParserRecorder::CompleteParserRecorder() {
   preamble_[PreparseDataConstants::kMagicOffset] =

src/parsing/preparse-data.h

@@ -53,7 +53,9 @@ class ParserRecorder {
   virtual ~ParserRecorder() { }
 
   // Logs the scope and some details of a function literal in the source.
-  virtual void LogFunction(int start, int end, int literals, int properties,
+  virtual void LogFunction(int start, int end, int num_parameters,
+                           int function_length, bool has_duplicate_parameters,
+                           int literals, int properties,
                            LanguageMode language_mode, bool uses_super_property,
                            bool calls_eval) = 0;
 
@@ -72,12 +74,20 @@ class ParserRecorder {
 class SingletonLogger : public ParserRecorder {
  public:
   SingletonLogger()
-      : has_error_(false), start_(-1), end_(-1), error_type_(kSyntaxError) {}
+      : has_error_(false),
+        start_(-1),
+        end_(-1),
+        num_parameters_(-1),
+        function_length_(-1),
+        has_duplicate_parameters_(false),
+        error_type_(kSyntaxError) {}
   virtual ~SingletonLogger() {}
 
   void Reset() { has_error_ = false; }
 
-  virtual void LogFunction(int start, int end, int literals, int properties,
+  virtual void LogFunction(int start, int end, int num_parameters,
+                           int function_length, bool has_duplicate_parameters,
+                           int literals, int properties,
                            LanguageMode language_mode, bool uses_super_property,
                            bool calls_eval) {
     DCHECK(!has_error_);
@@ -85,6 +95,9 @@ class SingletonLogger : public ParserRecorder {
     DCHECK(start_ == -1 && end_ == -1);
     start_ = start;
     end_ = end;
+    num_parameters_ = num_parameters;
+    function_length_ = function_length;
+    has_duplicate_parameters_ = has_duplicate_parameters;
     literals_ = literals;
     properties_ = properties;
     language_mode_ = language_mode;
@@ -110,6 +123,18 @@ class SingletonLogger : public ParserRecorder {
 
   int start() const { return start_; }
   int end() const { return end_; }
+  int num_parameters() const {
+    DCHECK(!has_error_);
+    return num_parameters_;
+  }
+  int function_length() const {
+    DCHECK(!has_error_);
+    return function_length_;
+  }
+  bool has_duplicate_parameters() const {
+    DCHECK(!has_error_);
+    return has_duplicate_parameters_;
+  }
   int literals() const {
     DCHECK(!has_error_);
     return literals_;
@@ -148,6 +173,9 @@ class SingletonLogger : public ParserRecorder {
   int start_;
   int end_;
   // For function entries.
+  int num_parameters_;
+  int function_length_;
+  bool has_duplicate_parameters_;
   int literals_;
   int properties_;
   LanguageMode language_mode_;
@@ -170,17 +198,11 @@ class CompleteParserRecorder : public ParserRecorder {
   CompleteParserRecorder();
   virtual ~CompleteParserRecorder() {}
 
-  virtual void LogFunction(int start, int end, int literals, int properties,
+  virtual void LogFunction(int start, int end, int num_parameters,
+                           int function_length, bool has_duplicate_parameters,
+                           int literals, int properties,
                            LanguageMode language_mode, bool uses_super_property,
-                           bool calls_eval) {
-    function_store_.Add(start);
-    function_store_.Add(end);
-    function_store_.Add(literals);
-    function_store_.Add(properties);
-    function_store_.Add(language_mode);
-    function_store_.Add(uses_super_property);
-    function_store_.Add(calls_eval);
-  }
+                           bool calls_eval);
 
   // Logs an error message and marks the log as containing an error.
   // Further logging will be ignored, and ExtractData will return a vector

src/parsing/preparser.cc

@@ -84,8 +84,9 @@ PreParserIdentifier PreParser::GetSymbol() const {
 }
 
 PreParser::PreParseResult PreParser::PreParseFunction(
-    DeclarationScope* function_scope, bool parsing_module, SingletonLogger* log,
-    bool is_inner_function, bool may_abort, int* use_counts) {
+    FunctionKind kind, DeclarationScope* function_scope, bool parsing_module,
+    SingletonLogger* log, bool is_inner_function, bool may_abort,
+    int* use_counts) {
   DCHECK_EQ(FUNCTION_SCOPE, function_scope->scope_type());
   parsing_module_ = parsing_module;
   log_ = log;
@@ -98,24 +99,63 @@ PreParser::PreParseResult PreParser::PreParseFunction(
   // PreParser.
   DCHECK_NULL(scope_state_);
   FunctionState function_state(&function_state_, &scope_state_, function_scope);
-  DCHECK_EQ(Token::LBRACE, scanner()->current_token());
-  bool ok = true;
-  int start_position = peek_position();
-  LazyParsingResult result = ParseStatementListAndLogFunction(may_abort, &ok);
+  // This indirection is needed so that we can use the CHECK_OK macros.
+  bool ok_holder = true;
+  bool* ok = &ok_holder;
+
+  PreParserFormalParameters formals(function_scope);
+  bool has_duplicate_parameters = false;
+  DuplicateFinder duplicate_finder(scanner()->unicode_cache());
+  std::unique_ptr<ExpressionClassifier> formals_classifier;
+
+  // Parse non-arrow function parameters. For arrow functions, the parameters
+  // have already been parsed.
+  if (!IsArrowFunction(kind)) {
+    formals_classifier.reset(new ExpressionClassifier(this, &duplicate_finder));
+    // We return kPreParseSuccess in failure cases too - errors are retrieved
+    // separately by Parser::SkipLazyFunctionBody.
+    ParseFormalParameterList(&formals, CHECK_OK_VALUE(kPreParseSuccess));
+    Expect(Token::RPAREN, CHECK_OK_VALUE(kPreParseSuccess));
+    int formals_end_position = scanner()->location().end_pos;
+
+    CheckArityRestrictions(
+        formals.arity, kind, formals.has_rest, function_scope->start_position(),
+        formals_end_position, CHECK_OK_VALUE(kPreParseSuccess));
+    has_duplicate_parameters =
+        !classifier()->is_valid_formal_parameter_list_without_duplicates();
+  }
+
+  Expect(Token::LBRACE, CHECK_OK_VALUE(kPreParseSuccess));
+  LazyParsingResult result = ParseStatementListAndLogFunction(
+      function_scope->start_position(), &formals, has_duplicate_parameters,
+      may_abort, ok);
   use_counts_ = nullptr;
   track_unresolved_variables_ = false;
   if (result == kLazyParsingAborted) {
     return kPreParseAbort;
   } else if (stack_overflow()) {
     return kPreParseStackOverflow;
-  } else if (!ok) {
+  } else if (!*ok) {
     DCHECK(log->has_error());
   } else {
     DCHECK_EQ(Token::RBRACE, scanner()->peek());
+
+    if (!IsArrowFunction(kind)) {
+      // Validate parameter names. We can do this only after parsing the
+      // function, since the function can declare itself strict.
+      const bool allow_duplicate_parameters =
+          is_sloppy(function_scope->language_mode()) && formals.is_simple &&
+          !IsConciseMethod(kind);
+      ValidateFormalParameters(function_scope->language_mode(),
+                               allow_duplicate_parameters,
+                               CHECK_OK_VALUE(kPreParseSuccess));
+    }
+
     if (is_strict(function_scope->language_mode())) {
       int end_pos = scanner()->location().end_pos;
-      CheckStrictOctalLiteral(start_position, end_pos, &ok);
-      CheckDecimalLiteralWithLeadingZero(start_position, end_pos);
+      CheckStrictOctalLiteral(function_scope->start_position(), end_pos, ok);
+      CheckDecimalLiteralWithLeadingZero(function_scope->start_position(),
+                                         end_pos);
     }
   }
   return kPreParseSuccess;
@@ -195,8 +235,8 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
 }
 
 PreParser::LazyParsingResult PreParser::ParseStatementListAndLogFunction(
-    bool may_abort, bool* ok) {
-  int body_start = position();
+    int start_position, PreParserFormalParameters* formals,
+    bool has_duplicate_parameters, bool may_abort, bool* ok) {
   PreParserStatementList body;
   LazyParsingResult result = ParseStatementList(
       body, Token::RBRACE, may_abort, CHECK_OK_VALUE(kLazyParsingComplete));
@@ -207,7 +247,8 @@ PreParser::LazyParsingResult PreParser::ParseStatementListAndLogFunction(
   int body_end = scanner()->peek_location().end_pos;
   DeclarationScope* scope = this->scope()->AsDeclarationScope();
   DCHECK(scope->is_function_scope());
-  log_->LogFunction(body_start, body_end,
+  log_->LogFunction(start_position, body_end, formals->num_parameters(),
+                    formals->function_length, has_duplicate_parameters,
                     function_state_->materialized_literal_count(),
                     function_state_->expected_property_count(), language_mode(),
                     scope->uses_super_property(), scope->calls_eval());

src/parsing/preparser.h

@@ -887,7 +887,8 @@ class PreParser : public ParserBase<PreParser> {
   // keyword and parameters, and have consumed the initial '{'.
   // At return, unless an error occurred, the scanner is positioned before the
   // the final '}'.
-  PreParseResult PreParseFunction(DeclarationScope* function_scope,
+  PreParseResult PreParseFunction(FunctionKind kind,
+                                  DeclarationScope* function_scope,
                                   bool parsing_module, SingletonLogger* log,
                                   bool track_unresolved_variables,
                                   bool may_abort, int* use_counts);
@@ -910,9 +911,11 @@ class PreParser : public ParserBase<PreParser> {
 
   bool AllowsLazyParsingWithoutUnresolvedVariables() const { return false; }
 
-  V8_INLINE LazyParsingResult SkipLazyFunctionBody(
+  V8_INLINE LazyParsingResult SkipFunction(
+      FunctionKind kind, DeclarationScope* function_scope, int* num_parameters,
+      int* function_length, bool* has_duplicate_parameters,
       int* materialized_literal_count, int* expected_property_count,
-      bool track_unresolved_variables, bool may_abort, bool* ok) {
+      bool is_inner_function, bool may_abort, bool* ok) {
     UNREACHABLE();
     return kLazyParsingComplete;
   }
@@ -921,7 +924,9 @@ class PreParser : public ParserBase<PreParser> {
       FunctionNameValidity function_name_validity, FunctionKind kind,
       int function_token_pos, FunctionLiteral::FunctionType function_type,
       LanguageMode language_mode, bool* ok);
-  LazyParsingResult ParseStatementListAndLogFunction(bool may_abort, bool* ok);
+  LazyParsingResult ParseStatementListAndLogFunction(
+      int position_before_formals, PreParserFormalParameters* formals,
+      bool has_duplicate_parameters, bool maybe_abort, bool* ok);
 
   struct TemplateLiteralState {};
 

test/cctest/test-parsing.cc

@@ -287,14 +287,30 @@ TEST(PreparseFunctionDataIsUsed) {
       i::GetCurrentStackPosition() - 128 * 1024);
 
   const char* good_code[] = {
-      "function this_is_lazy() { var a; } function foo() { return 25; } foo();",
-      "var this_is_lazy = () => { var a; }; var foo = () => 25; foo();",
+      "function z() { var a; } function f() { return 25; } f();",
+      "var z = function () { var a; }; function f() { return 25; } f();",
+      "function *z() { var a; } function f() { return 25; } f();",
+      "var z = function *() { var a; }; function f() { return 25; } f();",
+      "function z(p1, p2) { var a; } function f() { return 25; } f();",
+      "var z = function (p1, p2) { var a; }; function f() { return 25; } f();",
+      "function *z(p1, p2) { var a; } function f() { return 25; } f();",
+      "var z = function *(p1, p2) { var a; }; function f() { return 25; } f();",
+      "var z = () => { var a; }; function f() { return 25; } f();",
+      "var z = (p1, p2) => { var a; }; function f() { return 25; } f();",
   };
 
   // Insert a syntax error inside the lazy function.
   const char* bad_code[] = {
-      "function this_is_lazy() { if (   } function foo() { return 25; } foo();",
-      "var this_is_lazy = () => { if (   }; var foo = () => 25; foo();",
+      "function z() { if (   } function f() { return 25; } f();",
+      "var z = function () { if (   }; function f() { return 25; } f();",
+      "function *z() { if (   } function f() { return 25; } f();",
+      "var z = function *() { if (   }; function f() { return 25; } f();",
+      "function z(p1, p2) { if (   } function f() { return 25; } f();",
+      "var z = function (p1, p2) { if (   }; function f() { return 25; } f();",
+      "function *z(p1, p2) { if (   } function f() { return 25; } f();",
+      "var z = function *(p1, p2) { if (   }; function f() { return 25; } f();",
+      "var z = () => { if (   }; function f() { return 25; } f();",
+      "var z = (p1, p2) => { if (   }; function f() { return 25; } f();",
   };
 
   for (unsigned i = 0; i < arraysize(good_code); i++) {
@@ -488,17 +504,16 @@ TEST(Regress928) {
 
   int first_function =
       static_cast<int>(strstr(program, "function") - program);
-  int first_lbrace = first_function + i::StrLength("function () ");
-  CHECK_EQ('{', program[first_lbrace]);
-  i::FunctionEntry entry1 = pd->GetFunctionEntry(first_lbrace);
+  int first_lparen = first_function + i::StrLength("function ");
+  CHECK_EQ('(', program[first_lparen]);
+  i::FunctionEntry entry1 = pd->GetFunctionEntry(first_lparen);
   CHECK(!entry1.is_valid());
 
   int second_function =
-      static_cast<int>(strstr(program + first_lbrace, "function") - program);
-  int second_lbrace =
-      second_function + i::StrLength("function () ");
-  CHECK_EQ('{', program[second_lbrace]);
-  i::FunctionEntry entry2 = pd->GetFunctionEntry(second_lbrace);
+      static_cast<int>(strstr(program + first_lparen, "function") - program);
+  int second_lparen = second_function + i::StrLength("function ");
+  CHECK_EQ('(', program[second_lparen]);
+  i::FunctionEntry entry2 = pd->GetFunctionEntry(second_lparen);
   CHECK(entry2.is_valid());
   CHECK_EQ('}', program[entry2.end_pos() - 1]);
 }