[parser] Cleaning up adding skippable function preparse data

- Add DataGatheringScope::AddSkippableFunction
- Rename preparsed_scope_data_builder to preparse_data_builder

Change-Id: Ic882de638bed91a6ca4716f88db859410f1450b8
Reviewed-on: https://chromium-review.googlesource.com/c/1400846Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58673}

src/ast/scopes.cc

@@ -299,7 +299,7 @@ void DeclarationScope::SetDefaults() {
   should_eager_compile_ = false;
   was_lazily_parsed_ = false;
   is_skipped_function_ = false;
-  preparsed_scope_data_builder_ = nullptr;
+  preparse_data_builder_ = nullptr;
 #ifdef DEBUG
   DeclarationScope* outer_declaration_scope =
       outer_scope_ ? outer_scope_->GetDeclarationScope() : nullptr;
@@ -1497,8 +1497,8 @@ void Scope::SavePreparseData() {
 }
 
 void DeclarationScope::SavePreparseDataForDeclarationScope() {
-  if (preparsed_scope_data_builder_ == nullptr) return;
-  preparsed_scope_data_builder_->SaveScopeAllocationData(this);
+  if (preparse_data_builder_ == nullptr) return;
+  preparse_data_builder_->SaveScopeAllocationData(this);
 }
 
 void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
@@ -1506,8 +1506,8 @@ void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
   UnresolvedList new_unresolved_list;
   if (!IsArrowFunction(function_kind_) &&
       (!outer_scope_->is_script_scope() ||
-       (preparsed_scope_data_builder_ != nullptr &&
-        preparsed_scope_data_builder_->ContainsInnerFunctions()))) {
+       (preparse_data_builder_ != nullptr &&
+        preparse_data_builder_->ContainsInnerFunctions()))) {
     // Try to resolve unresolved variables for this Scope and migrate those
     // which cannot be resolved inside. It doesn't make sense to try to resolve
     // them in the outer Scopes here, because they are incomplete.

src/ast/scopes.h

@@ -1017,13 +1017,12 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   // saved in produced_preparse_data_.
   void SavePreparseDataForDeclarationScope();
 
-  void set_preparsed_scope_data_builder(
-      PreparseDataBuilder* preparsed_scope_data_builder) {
-    preparsed_scope_data_builder_ = preparsed_scope_data_builder;
+  void set_preparse_data_builder(PreparseDataBuilder* preparse_data_builder) {
+    preparse_data_builder_ = preparse_data_builder;
   }
 
-  PreparseDataBuilder* preparsed_scope_data_builder() const {
-    return preparsed_scope_data_builder_;
+  PreparseDataBuilder* preparse_data_builder() const {
+    return preparse_data_builder_;
   }
 
  private:
@@ -1079,7 +1078,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   Variable* arguments_;
 
   // For producing the scope allocation data during preparsing.
-  PreparseDataBuilder* preparsed_scope_data_builder_;
+  PreparseDataBuilder* preparse_data_builder_;
 
   struct RareData : public ZoneObject {
     // Convenience variable; Subclass constructor only

src/parsing/preparse-data.cc

@@ -160,11 +160,11 @@ PreparseDataBuilder::PreparseDataBuilder(Zone* zone,
 
 void PreparseDataBuilder::DataGatheringScope::Start(
     DeclarationScope* function_scope) {
-  PreparseDataBuilder* parent = preparser_->preparsed_scope_data_builder();
+  PreparseDataBuilder* parent = preparser_->preparse_data_builder();
   Zone* main_zone = preparser_->main_zone();
   builder_ = new (main_zone) PreparseDataBuilder(main_zone, parent);
-  preparser_->set_preparsed_scope_data_builder(builder_);
-  function_scope->set_preparsed_scope_data_builder(builder_);
+  preparser_->set_preparse_data_builder(builder_);
+  function_scope->set_preparse_data_builder(builder_);
 }
 
 PreparseDataBuilder::DataGatheringScope::~DataGatheringScope() {
@@ -174,7 +174,16 @@ PreparseDataBuilder::DataGatheringScope::~DataGatheringScope() {
     parent->data_for_inner_functions_.push_back(builder_->HasData() ? builder_
                                                                     : nullptr);
   }
-  preparser_->set_preparsed_scope_data_builder(parent);
+  preparser_->set_preparse_data_builder(parent);
+}
+
+void PreparseDataBuilder::DataGatheringScope::AddSkippableFunction(
+    DeclarationScope* function_scope, int end_position,
+    int num_inner_functions) {
+  builder_->parent_->AddSkippableFunction(
+      function_scope->start_position(), end_position,
+      function_scope->num_parameters(), num_inner_functions,
+      function_scope->language_mode(), function_scope->NeedsHomeObject());
 }
 
 void PreparseDataBuilder::AddSkippableFunction(int start_position,
@@ -305,7 +314,7 @@ bool PreparseDataBuilder::ScopeIsSkippableFunctionScope(Scope* scope) {
   if (scope->scope_type() != ScopeType::FUNCTION_SCOPE) return false;
   DeclarationScope* declaration_scope = scope->AsDeclarationScope();
   return !declaration_scope->is_arrow_scope() &&
-         declaration_scope->preparsed_scope_data_builder() != nullptr;
+         declaration_scope->preparse_data_builder() != nullptr;
 }
 
 void PreparseDataBuilder::SaveDataForScope(Scope* scope) {
@@ -362,8 +371,7 @@ void PreparseDataBuilder::SaveDataForInnerScopes(Scope* scope) {
     if (ScopeIsSkippableFunctionScope(inner)) {
       // Don't save data about function scopes, since they'll have their own
       // PreparseDataBuilder where their data is saved.
-      DCHECK_NOT_NULL(
-          inner->AsDeclarationScope()->preparsed_scope_data_builder());
+      DCHECK_NOT_NULL(inner->AsDeclarationScope()->preparse_data_builder());
       continue;
     }
     if (!ScopeNeedsData(inner)) continue;

src/parsing/preparse-data.h

@@ -81,6 +81,8 @@ class PreparseDataBuilder : public ZoneObject {
         : preparser_(preparser), builder_(nullptr) {}
 
     void Start(DeclarationScope* function_scope);
+    void AddSkippableFunction(DeclarationScope* function_scope,
+                              int end_position, int num_inner_functions);
     ~DataGatheringScope();
 
    private:

src/parsing/preparser.cc

@@ -335,13 +335,10 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
     if (is_strict(language_mode)) {
       CheckStrictOctalLiteral(start_position, end_position());
     }
-  }
-
-  if (skippable_function) {
-    preparse_data_builder_->AddSkippableFunction(
-        function_scope->start_position(), end_position(),
-        function_scope->num_parameters(), GetLastFunctionLiteralId() - func_id,
-        function_scope->language_mode(), function_scope->NeedsHomeObject());
+    if (skippable_function) {
+      preparse_data_builder_scope.AddSkippableFunction(
+          function_scope, end_position(), GetLastFunctionLiteralId() - func_id);
+    }
   }
 
   if (V8_UNLIKELY(FLAG_log_function_events)) {

src/parsing/preparser.h

@@ -1010,13 +1010,12 @@ class PreParser : public ParserBase<PreParser> {
       DeclarationScope* function_scope, int* use_counts,
       ProducedPreparseData** produced_preparser_scope_data, int script_id);
 
-  PreparseDataBuilder* preparsed_scope_data_builder() const {
+  PreparseDataBuilder* preparse_data_builder() const {
     return preparse_data_builder_;
   }
 
-  void set_preparsed_scope_data_builder(
-      PreparseDataBuilder* preparsed_scope_data_builder) {
-    preparse_data_builder_ = preparsed_scope_data_builder;
+  void set_preparse_data_builder(PreparseDataBuilder* preparse_data_builder) {
+    preparse_data_builder_ = preparse_data_builder;
   }
 
  private:

test/cctest/parsing/test-preparser.cc

@@ -38,7 +38,7 @@ TEST(PreParserScopeAnalysis) {
   i::Factory* factory = isolate->factory();
   LocalContext env;
 
-  struct {
+  struct Outer {
     const char* code;
     bool strict_outer;
     bool strict_test_function;
@@ -688,34 +688,27 @@ TEST(PreParserScopeAnalysis) {
        [] { i::FLAG_harmony_private_fields = false; }},
   };
 
-  for (unsigned outer_ix = 0; outer_ix < arraysize(outers); ++outer_ix) {
-    for (unsigned inner_ix = 0; inner_ix < arraysize(inners); ++inner_ix) {
-      if (outers[outer_ix].strict_outer &&
-          (inners[inner_ix].skip & SKIP_STRICT_OUTER)) {
-        continue;
-      }
-      if (outers[outer_ix].strict_test_function &&
-          (inners[inner_ix].skip & SKIP_STRICT_FUNCTION)) {
-        continue;
-      }
-      if (outers[outer_ix].arrow && (inners[inner_ix].skip & SKIP_ARROW)) {
+  for (unsigned i = 0; i < arraysize(outers); ++i) {
+    struct Outer outer = outers[i];
+    for (unsigned j = 0; j < arraysize(inners); ++j) {
+      struct Inner inner = inners[j];
+      if (outer.strict_outer && (inner.skip & SKIP_STRICT_OUTER)) continue;
+      if (outer.strict_test_function && (inner.skip & SKIP_STRICT_FUNCTION)) {
         continue;
       }
+      if (outer.arrow && (inner.skip & SKIP_ARROW)) continue;
 
-      const char* code = outers[outer_ix].code;
+      const char* code = outer.code;
       int code_len = Utf8LengthHelper(code);
 
-      int params_len = Utf8LengthHelper(inners[inner_ix].params);
-      int source_len = Utf8LengthHelper(inners[inner_ix].source);
+      int params_len = Utf8LengthHelper(inner.params);
+      int source_len = Utf8LengthHelper(inner.source);
       int len = code_len + params_len + source_len;
 
-      if (inners[inner_ix].prologue != nullptr) {
-        inners[inner_ix].prologue();
-      }
+      if (inner.prologue != nullptr) inner.prologue();
 
       i::ScopedVector<char> program(len + 1);
-      i::SNPrintF(program, code, inners[inner_ix].params,
-                  inners[inner_ix].source);
+      i::SNPrintF(program, code, inner.params, inner.source);
 
       i::HandleScope scope(isolate);
 
@@ -730,8 +723,8 @@ TEST(PreParserScopeAnalysis) {
       i::Handle<i::JSFunction> f = i::Handle<i::JSFunction>::cast(o);
       i::Handle<i::SharedFunctionInfo> shared = i::handle(f->shared(), isolate);
 
-      if (inners[inner_ix].bailout == Bailout::BAILOUT_IF_OUTER_SLOPPY &&
-          !outers[outer_ix].strict_outer) {
+      if (inner.bailout == Bailout::BAILOUT_IF_OUTER_SLOPPY &&
+          !outer.strict_outer) {
         CHECK(!shared->HasUncompiledDataWithPreparseData());
         continue;
       }
@@ -776,11 +769,9 @@ TEST(PreParserScopeAnalysis) {
       // scope data (and skipping functions), and when parsing without.
       i::ScopeTestHelper::CompareScopes(
           scope_without_skipped_functions, scope_with_skipped_functions,
-          inners[inner_ix].precise_maybe_assigned == PreciseMaybeAssigned::YES);
+          inner.precise_maybe_assigned == PreciseMaybeAssigned::YES);
 
-      if (inners[inner_ix].epilogue != nullptr) {
-        inners[inner_ix].epilogue();
-      }
+      if (inner.epilogue != nullptr) inner.epilogue();
     }
   }
 }