[parser|ast] Simplify AstNodeFactory Zone usage.

AstNodeFactory used to get the Zone directly from AstValueFactory. But that's
generally the wrong Zone (the main Zone, instead of the temp Zone), and the
creator of AstNodeFactory had to call set_zone right after. By adding a Zone
param, we can pass the correct Zone right away.

Also made PreParserFactory have an AstNodeFactory, so that we don't need to
create temporary AstNodeFactories all the time.

Also removed AstNodeFactory::BodyScope since DiscardableZoneScope essentially
did the same thing already.

BUG=v8:5516,v8:6092

Change-Id: I189d2e6afe91c91e49d8ed7e3496a0d9c405e1c5
Reviewed-on: https://chromium-review.googlesource.com/507129
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45370}

src/ast/ast.h

@@ -3239,17 +3239,12 @@ class AstVisitor BASE_EMBEDDED {
 
 class AstNodeFactory final BASE_EMBEDDED {
  public:
-  explicit AstNodeFactory(AstValueFactory* ast_value_factory)
-      : zone_(nullptr), ast_value_factory_(ast_value_factory) {
-    if (ast_value_factory != nullptr) {
-      zone_ = ast_value_factory->zone();
-    }
-  }
+  AstNodeFactory(AstValueFactory* ast_value_factory, Zone* zone)
+      : zone_(zone), ast_value_factory_(ast_value_factory) {}
 
   AstValueFactory* ast_value_factory() const { return ast_value_factory_; }
   void set_ast_value_factory(AstValueFactory* ast_value_factory) {
     ast_value_factory_ = ast_value_factory;
-    zone_ = ast_value_factory->zone();
   }
 
   VariableDeclaration* NewVariableDeclaration(VariableProxy* proxy,
@@ -3676,24 +3671,6 @@ class AstNodeFactory final BASE_EMBEDDED {
   Zone* zone() const { return zone_; }
   void set_zone(Zone* zone) { zone_ = zone; }
 
-  // Handles use of temporary zones when parsing inner function bodies.
-  class BodyScope {
-   public:
-    BodyScope(AstNodeFactory* factory, Zone* temp_zone, bool use_temp_zone)
-        : factory_(factory), prev_zone_(factory->zone_) {
-      if (use_temp_zone) {
-        factory->zone_ = temp_zone;
-      }
-    }
-
-    void Reset() { factory_->zone_ = prev_zone_; }
-    ~BodyScope() { Reset(); }
-
-   private:
-    AstNodeFactory* factory_;
-    Zone* prev_zone_;
-  };
-
  private:
   // This zone may be deallocated upon returning from parsing a function body
   // which we can guarantee is not going to be compiled or have its AST

src/ast/scopes.cc

@@ -644,7 +644,7 @@ void DeclarationScope::Analyze(ParseInfo* info, Isolate* isolate,
   }
 
   if (scope->is_eval_scope() && is_sloppy(scope->language_mode())) {
-    AstNodeFactory factory(info->ast_value_factory());
+    AstNodeFactory factory(info->ast_value_factory(), info->zone());
     scope->HoistSloppyBlockFunctions(&factory);
   }
 

src/parsing/parser-base.h

@@ -209,7 +209,7 @@ class ParserBase {
         extension_(extension),
         fni_(nullptr),
         ast_value_factory_(ast_value_factory),
-        ast_node_factory_(ast_value_factory),
+        ast_node_factory_(ast_value_factory, zone),
         runtime_call_stats_(runtime_call_stats),
         parsing_on_main_thread_(parsing_on_main_thread),
         parsing_module_(false),

src/parsing/parser.cc

@@ -111,8 +111,7 @@ int ParseData::FunctionsSize() {
 class DiscardableZoneScope {
  public:
   DiscardableZoneScope(Parser* parser, Zone* temp_zone, bool use_temp_zone)
-      : ast_node_factory_scope_(parser->factory(), temp_zone, use_temp_zone),
-        fni_(parser->ast_value_factory_, temp_zone),
+      : fni_(parser->ast_value_factory_, temp_zone),
         parser_(parser),
         prev_fni_(parser->fni_),
         prev_zone_(parser->zone_),
@@ -124,6 +123,7 @@ class DiscardableZoneScope {
       parser_->temp_zoned_ = true;
       parser_->fni_ = &fni_;
       parser_->zone_ = temp_zone;
+      parser_->factory()->set_zone(temp_zone);
       if (parser_->reusable_preparser_ != nullptr) {
         parser_->reusable_preparser_->zone_ = temp_zone;
         parser_->reusable_preparser_->factory()->set_zone(temp_zone);
@@ -133,18 +133,17 @@ class DiscardableZoneScope {
   void Reset() {
     parser_->fni_ = prev_fni_;
     parser_->zone_ = prev_zone_;
+    parser_->factory()->set_zone(prev_zone_);
     parser_->allow_lazy_ = prev_allow_lazy_;
     parser_->temp_zoned_ = prev_temp_zoned_;
     if (parser_->reusable_preparser_ != nullptr) {
       parser_->reusable_preparser_->zone_ = prev_zone_;
       parser_->reusable_preparser_->factory()->set_zone(prev_zone_);
     }
-    ast_node_factory_scope_.Reset();
   }
   ~DiscardableZoneScope() { Reset(); }
 
  private:
-  AstNodeFactory::BodyScope ast_node_factory_scope_;
   FuncNameInferrer fni_;
   Parser* parser_;
   FuncNameInferrer* prev_fni_;
@@ -2703,8 +2702,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   {
     // Temporary zones can nest. When we migrate free variables (see below), we
     // need to recreate them in the previous Zone.
-    AstNodeFactory previous_zone_ast_node_factory(ast_value_factory());
-    previous_zone_ast_node_factory.set_zone(zone());
+    AstNodeFactory previous_zone_ast_node_factory(ast_value_factory(), zone());
 
     // Open a new zone scope, which sets our AstNodeFactory to allocate in the
     // new temporary zone if the preconditions are satisfied, and ensures that

src/parsing/preparser.cc

@@ -372,13 +372,9 @@ PreParserExpression PreParser::ExpressionFromIdentifier(
     PreParserIdentifier name, int start_position, InferName infer) {
   VariableProxy* proxy = nullptr;
   if (track_unresolved_variables_) {
-    AstNodeFactory factory(ast_value_factory());
-    // Setting the Zone is necessary because zone_ might be the temp Zone, and
-    // AstValueFactory doesn't know about it.
-    factory.set_zone(zone());
     DCHECK_NOT_NULL(name.string_);
-    proxy = scope()->NewUnresolved(&factory, name.string_, start_position,
-                                   NORMAL_VARIABLE);
+    proxy = scope()->NewUnresolved(factory()->ast_node_factory(), name.string_,
+                                   start_position, NORMAL_VARIABLE);
   }
   return PreParserExpression::FromIdentifier(name, proxy, zone());
 }

src/parsing/preparser.h

@@ -544,11 +544,15 @@ class PreParserStatement {
 
 class PreParserFactory {
  public:
-  explicit PreParserFactory(AstValueFactory* ast_value_factory)
-      : ast_value_factory_(ast_value_factory),
-        zone_(ast_value_factory->zone()) {}
+  explicit PreParserFactory(AstValueFactory* ast_value_factory, Zone* zone)
+      : ast_node_factory_(ast_value_factory, zone), zone_(zone) {}
 
-  void set_zone(Zone* zone) { zone_ = zone; }
+  void set_zone(Zone* zone) {
+    ast_node_factory_.set_zone(zone);
+    zone_ = zone;
+  }
+
+  AstNodeFactory* ast_node_factory() { return &ast_node_factory_; }
 
   PreParserExpression NewStringLiteral(PreParserIdentifier identifier,
                                        int pos) {
@@ -557,10 +561,8 @@ class PreParserFactory {
     PreParserExpression expression = PreParserExpression::Default();
     if (identifier.string_ != nullptr) {
       DCHECK(FLAG_lazy_inner_functions);
-      AstNodeFactory factory(ast_value_factory_);
-      factory.set_zone(zone_);
-      VariableProxy* variable =
-          factory.NewVariableProxy(identifier.string_, NORMAL_VARIABLE);
+      VariableProxy* variable = ast_node_factory_.NewVariableProxy(
+          identifier.string_, NORMAL_VARIABLE);
       expression.AddVariable(variable, zone_);
     }
     return expression;
@@ -795,7 +797,9 @@ class PreParserFactory {
   }
 
  private:
-  AstValueFactory* ast_value_factory_;
+  // For creating VariableProxy objects (if
+  // PreParser::track_unresolved_variables_ is used).
+  AstNodeFactory ast_node_factory_;
   Zone* zone_;
 };
 
@@ -1506,12 +1510,9 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE PreParserExpression ThisExpression(int pos = kNoSourcePosition) {
     ZoneList<VariableProxy*>* variables = nullptr;
     if (track_unresolved_variables_) {
-      AstNodeFactory factory(ast_value_factory());
-      // Setting the Zone is necessary because zone_ might be the temp Zone, and
-      // AstValueFactory doesn't know about it.
-      factory.set_zone(zone());
       VariableProxy* proxy = scope()->NewUnresolved(
-          &factory, ast_value_factory()->this_string(), pos, THIS_VARIABLE);
+          factory()->ast_node_factory(), ast_value_factory()->this_string(),
+          pos, THIS_VARIABLE);
 
       variables = new (zone()) ZoneList<VariableProxy*>(1, zone());
       variables->Add(proxy, zone());

src/parsing/rewriter.cc

@@ -24,7 +24,7 @@ class Processor final : public AstVisitor<Processor> {
         breakable_(false),
         zone_(ast_value_factory->zone()),
         closure_scope_(closure_scope),
-        factory_(ast_value_factory) {
+        factory_(ast_value_factory, ast_value_factory->zone()) {
     DCHECK_EQ(closure_scope, closure_scope->GetClosureScope());
     InitializeAstVisitor(stack_limit);
   }
@@ -38,7 +38,7 @@ class Processor final : public AstVisitor<Processor> {
         breakable_(false),
         zone_(ast_value_factory->zone()),
         closure_scope_(closure_scope),
-        factory_(ast_value_factory) {
+        factory_(ast_value_factory, zone_) {
     DCHECK_EQ(closure_scope, closure_scope->GetClosureScope());
     InitializeAstVisitor(parser->stack_limit());
   }

test/cctest/test-ast.cc

@@ -48,7 +48,7 @@ TEST(List) {
   Zone zone(&allocator, ZONE_NAME);
   AstValueFactory value_factory(&zone, isolate->ast_string_constants(),
                                 isolate->heap()->HashSeed());
-  AstNodeFactory factory(&value_factory);
+  AstNodeFactory factory(&value_factory, &zone);
   AstNode* node = factory.NewEmptyStatement(kNoSourcePosition);
   list->Add(node);
   CHECK_EQ(1, list->length());