From 5c56665059891cbacf43f6efe483582029442f0b Mon Sep 17 00:00:00 2001
From: marja <marja@chromium.org>
Date: Fri, 6 Feb 2015 07:58:36 -0800
Subject: [PATCH] PreParser / Parser consistency: Make PreParser aware of Zone
 and AstValueFactory.

Previously it just had hacks to have NULLs instead of them and pretended to know
nothing about Zone. The hacks provide no real benefit (probably historically
based on some weird misconception about the relationship between Zone and
Isolate), and make it harder for the PreParser to start to know more about
variables and scoping.

BUG=

Review URL: https://codereview.chromium.org/909463003

Cr-Commit-Position: refs/heads/master@{#26494}
---
 src/parser.cc               | 11 ++++++-----
 src/parser.h                | 10 ----------
 src/preparser.h             | 45 ++++++++++++++++++++----------------------
 test/cctest/test-parsing.cc | 48 +++++++++++++++++++++++++++++++++++++--------
 4 files changed, 67 insertions(+), 47 deletions(-)

diff --git a/src/parser.cc b/src/parser.cc
index d3fde9e..78039f6 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -671,7 +671,7 @@ const AstRawString* ParserTraits::GetNumberAsSymbol(Scanner* scanner) {
   char array[100];
   const char* string =
       DoubleToCString(double_value, Vector<char>(array, arraysize(array)));
-  return ast_value_factory()->GetOneByteString(string);
+  return parser_->ast_value_factory()->GetOneByteString(string);
 }
 
 
@@ -794,8 +794,8 @@ ClassLiteral* ParserTraits::ParseClassLiteral(
 
 Parser::Parser(CompilationInfo* info, ParseInfo* parse_info)
     : ParserBase<ParserTraits>(info->isolate(), info->zone(), &scanner_,
-                               parse_info->stack_limit, info->extension(), NULL,
-                               this),
+                               parse_info->stack_limit, info->extension(),
+                               info->ast_value_factory(), NULL, this),
       scanner_(parse_info->unicode_cache),
       reusable_preparser_(NULL),
       original_scope_(NULL),
@@ -833,6 +833,7 @@ Parser::Parser(CompilationInfo* info, ParseInfo* parse_info)
     // info takes ownership of AstValueFactory.
     info->SetAstValueFactory(
         new AstValueFactory(zone(), parse_info->hash_seed));
+    ast_value_factory_ = info->ast_value_factory();
   }
 }
 
@@ -4037,8 +4038,8 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
   DCHECK_EQ(Token::LBRACE, scanner()->current_token());
 
   if (reusable_preparser_ == NULL) {
-    reusable_preparser_ =
-        new PreParser(isolate(), &scanner_, NULL, stack_limit_);
+    reusable_preparser_ = new PreParser(
+        isolate(), zone(), &scanner_, ast_value_factory(), NULL, stack_limit_);
     reusable_preparser_->set_allow_lazy(true);
     reusable_preparser_->set_allow_natives(allow_natives());
     reusable_preparser_->set_allow_harmony_scoping(allow_harmony_scoping());
diff --git a/src/parser.h b/src/parser.h
index d1d1d7c..9c2b056 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -364,7 +364,6 @@ class ParserTraits {
     inline static Scope* ptr_to_scope(ScopePtr scope) { return scope; }
 
     typedef Variable GeneratorVariable;
-    typedef v8::internal::Zone Zone;
 
     typedef v8::internal::AstProperties AstProperties;
     typedef Vector<VariableProxy*> ParameterIdentifierVector;
@@ -569,7 +568,6 @@ class ParserTraits {
   int DeclareArrowParametersFromExpression(Expression* expression, Scope* scope,
                                            Scanner::Location* dupe_loc,
                                            bool* ok);
-  V8_INLINE AstValueFactory* ast_value_factory();
 
   // Temporary glue; these functions will move to ParserBase.
   Expression* ParseV8Intrinsic(bool* ok);
@@ -716,9 +714,6 @@ class Parser : public ParserBase<ParserTraits> {
   Isolate* isolate() { return info_->isolate(); }
   CompilationInfo* info() const { return info_; }
   Handle<Script> script() const { return info_->script(); }
-  AstValueFactory* ast_value_factory() const {
-    return info_->ast_value_factory();
-  }
 
   // Called by ParseProgram after setting up the scanner.
   FunctionLiteral* DoParseProgram(CompilationInfo* info, Scope** scope,
@@ -947,11 +942,6 @@ void ParserTraits::CheckConflictingVarDeclarations(v8::internal::Scope* scope,
 }
 
 
-AstValueFactory* ParserTraits::ast_value_factory() {
-  return parser_->ast_value_factory();
-}
-
-
 // Support for handling complex values (array and object literals) that
 // can be fully handled at compile time.
 class CompileTimeValue: public AllStatic {
diff --git a/src/preparser.h b/src/preparser.h
index e6ef4ec..5f1714b 100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -42,7 +42,6 @@ namespace internal {
 //     // Used by FunctionState and BlockState.
 //     typedef Scope;
 //     typedef GeneratorVariable;
-//     typedef Zone;
 //     // Return types for traversing functions.
 //     typedef Identifier;
 //     typedef Expression;
@@ -68,15 +67,17 @@ class ParserBase : public Traits {
   typedef typename Traits::Type::Literal LiteralT;
   typedef typename Traits::Type::ObjectLiteralProperty ObjectLiteralPropertyT;
 
-  ParserBase(Isolate* isolate, typename Traits::Type::Zone* zone,
-             Scanner* scanner, uintptr_t stack_limit, v8::Extension* extension,
-             ParserRecorder* log, typename Traits::Type::Parser this_object)
+  ParserBase(Isolate* isolate, Zone* zone, Scanner* scanner,
+             uintptr_t stack_limit, v8::Extension* extension,
+             AstValueFactory* ast_value_factory, ParserRecorder* log,
+             typename Traits::Type::Parser this_object)
       : Traits(this_object),
         parenthesized_function_(false),
         scope_(NULL),
         function_state_(NULL),
         extension_(extension),
         fni_(NULL),
+        ast_value_factory_(ast_value_factory),
         log_(log),
         mode_(PARSE_EAGERLY),  // Lazy mode must be set explicitly.
         stack_limit_(stack_limit),
@@ -257,7 +258,6 @@ class ParserBase : public Traits {
     FunctionState* outer_function_state_;
     typename Traits::Type::Scope** scope_stack_;
     typename Traits::Type::Scope* outer_scope_;
-    typename Traits::Type::Zone* extra_param_;
     typename Traits::Type::Factory* factory_;
 
     friend class ParserTraits;
@@ -310,12 +310,13 @@ class ParserBase : public Traits {
 
   Isolate* isolate() const { return isolate_; }
   Scanner* scanner() const { return scanner_; }
+  AstValueFactory* ast_value_factory() const { return ast_value_factory_; }
   int position() { return scanner_->location().beg_pos; }
   int peek_position() { return scanner_->peek_location().beg_pos; }
   bool stack_overflow() const { return stack_overflow_; }
   void set_stack_overflow() { stack_overflow_ = true; }
   Mode mode() const { return mode_; }
-  typename Traits::Type::Zone* zone() const { return zone_; }
+  Zone* zone() const { return zone_; }
 
   INLINE(Token::Value peek()) {
     if (stack_overflow_) return Token::ILLEGAL;
@@ -620,13 +621,14 @@ class ParserBase : public Traits {
   FunctionState* function_state_;  // Function state stack.
   v8::Extension* extension_;
   FuncNameInferrer* fni_;
+  AstValueFactory* ast_value_factory_;  // Not owned.
   ParserRecorder* log_;
   Mode mode_;
   uintptr_t stack_limit_;
 
  private:
   Isolate* isolate_;
-  typename Traits::Type::Zone* zone_;  // Only used by Parser.
+  Zone* zone_;
 
   Scanner* scanner_;
   bool stack_overflow_;
@@ -1210,8 +1212,6 @@ class PreParserTraits {
 
     // PreParser doesn't need to store generator variables.
     typedef void GeneratorVariable;
-    // No interaction with Zones.
-    typedef void Zone;
 
     typedef int AstProperties;
     typedef Vector<PreParserIdentifier> ParameterIdentifierVector;
@@ -1432,15 +1432,15 @@ class PreParserTraits {
     return PreParserExpression::Default();
   }
 
-  static PreParserExpressionList NewExpressionList(int size, void* zone) {
+  static PreParserExpressionList NewExpressionList(int size, Zone* zone) {
     return PreParserExpressionList();
   }
 
-  static PreParserStatementList NewStatementList(int size, void* zone) {
+  static PreParserStatementList NewStatementList(int size, Zone* zone) {
     return PreParserStatementList();
   }
 
-  static PreParserExpressionList NewPropertyList(int size, void* zone) {
+  static PreParserExpressionList NewPropertyList(int size, Zone* zone) {
     return PreParserExpressionList();
   }
 
@@ -1488,7 +1488,6 @@ class PreParserTraits {
   static bool IsTaggedTemplate(const PreParserExpression tag) {
     return !tag.IsNoTemplateTag();
   }
-  static AstValueFactory* ast_value_factory() { return NULL; }
 
   void CheckConflictingVarDeclarations(PreParserScope scope, bool* ok) {}
 
@@ -1533,10 +1532,11 @@ class PreParser : public ParserBase<PreParserTraits> {
     kPreParseSuccess
   };
 
-  PreParser(Isolate* isolate, Scanner* scanner, ParserRecorder* log,
+  PreParser(Isolate* isolate, Zone* zone, Scanner* scanner,
+            AstValueFactory* ast_value_factory, ParserRecorder* log,
             uintptr_t stack_limit)
-      : ParserBase<PreParserTraits>(isolate, NULL, scanner, stack_limit, NULL,
-                                    log, this) {}
+      : ParserBase<PreParserTraits>(isolate, zone, scanner, stack_limit, NULL,
+                                    ast_value_factory, log, this) {}
 
   // Pre-parse the program from the character stream; returns true on
   // success (even if parsing failed, the pre-parse data successfully
@@ -1759,8 +1759,7 @@ typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParseIdentifier(
       ReportMessage("strict_eval_arguments");
       *ok = false;
     }
-    if (name->IsArguments(this->ast_value_factory()))
-      scope_->RecordArgumentsUsage();
+    if (name->IsArguments(ast_value_factory())) scope_->RecordArgumentsUsage();
     return name;
   } else if (is_sloppy(language_mode()) &&
              (next == Token::FUTURE_STRICT_RESERVED_WORD ||
@@ -1793,8 +1792,7 @@ typename ParserBase<Traits>::IdentifierT ParserBase<
   }
 
   IdentifierT name = this->GetSymbol(scanner());
-  if (name->IsArguments(this->ast_value_factory()))
-    scope_->RecordArgumentsUsage();
+  if (name->IsArguments(ast_value_factory())) scope_->RecordArgumentsUsage();
   return name;
 }
 
@@ -1812,8 +1810,7 @@ ParserBase<Traits>::ParseIdentifierName(bool* ok) {
   }
 
   IdentifierT name = this->GetSymbol(scanner());
-  if (name->IsArguments(this->ast_value_factory()))
-    scope_->RecordArgumentsUsage();
+  if (name->IsArguments(ast_value_factory())) scope_->RecordArgumentsUsage();
   return name;
 }
 
@@ -2836,7 +2833,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
   int handler_count = 0;
 
   {
-    typename Traits::Type::Factory function_factory(this->ast_value_factory());
+    typename Traits::Type::Factory function_factory(ast_value_factory());
     FunctionState function_state(&function_state_, &scope_,
                                  Traits::Type::ptr_to_scope(scope),
                                  kArrowFunction, &function_factory);
@@ -2914,7 +2911,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
   }
 
   FunctionLiteralT function_literal = factory()->NewFunctionLiteral(
-      this->EmptyIdentifierString(), this->ast_value_factory(), scope, body,
+      this->EmptyIdentifierString(), ast_value_factory(), scope, body,
       materialized_literal_count, expected_property_count, handler_count,
       num_parameters, FunctionLiteral::kNoDuplicateParameters,
       FunctionLiteral::ANONYMOUS_EXPRESSION, FunctionLiteral::kIsFunction,
diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index 41d8dfe..f0df320 100644
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -157,7 +157,11 @@ TEST(ScanHTMLEndComments) {
     i::CompleteParserRecorder log;
     i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
     scanner.Initialize(&stream);
-    i::PreParser preparser(CcTest::i_isolate(), &scanner, &log, stack_limit);
+    i::Zone zone;
+    i::AstValueFactory ast_value_factory(
+        &zone, CcTest::i_isolate()->heap()->HashSeed());
+    i::PreParser preparser(CcTest::i_isolate(), &zone, &scanner,
+                           &ast_value_factory, &log, stack_limit);
     preparser.set_allow_lazy(true);
     i::PreParser::PreParseResult result = preparser.PreParseProgram();
     CHECK_EQ(i::PreParser::kPreParseSuccess, result);
@@ -171,7 +175,11 @@ TEST(ScanHTMLEndComments) {
     i::CompleteParserRecorder log;
     i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
     scanner.Initialize(&stream);
-    i::PreParser preparser(CcTest::i_isolate(), &scanner, &log, stack_limit);
+    i::Zone zone;
+    i::AstValueFactory ast_value_factory(
+        &zone, CcTest::i_isolate()->heap()->HashSeed());
+    i::PreParser preparser(CcTest::i_isolate(), &zone, &scanner,
+                           &ast_value_factory, &log, stack_limit);
     preparser.set_allow_lazy(true);
     i::PreParser::PreParseResult result = preparser.PreParseProgram();
     // Even in the case of a syntax error, kPreParseSuccess is returned.
@@ -317,7 +325,11 @@ TEST(StandAlonePreParser) {
     i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
     scanner.Initialize(&stream);
 
-    i::PreParser preparser(CcTest::i_isolate(), &scanner, &log, stack_limit);
+    i::Zone zone;
+    i::AstValueFactory ast_value_factory(
+        &zone, CcTest::i_isolate()->heap()->HashSeed());
+    i::PreParser preparser(CcTest::i_isolate(), &zone, &scanner,
+                           &ast_value_factory, &log, stack_limit);
     preparser.set_allow_lazy(true);
     preparser.set_allow_natives(true);
     preparser.set_allow_harmony_arrow_functions(true);
@@ -351,7 +363,11 @@ TEST(StandAlonePreParserNoNatives) {
     scanner.Initialize(&stream);
 
     // Preparser defaults to disallowing natives syntax.
-    i::PreParser preparser(CcTest::i_isolate(), &scanner, &log, stack_limit);
+    i::Zone zone;
+    i::AstValueFactory ast_value_factory(
+        &zone, CcTest::i_isolate()->heap()->HashSeed());
+    i::PreParser preparser(CcTest::i_isolate(), &zone, &scanner,
+                           &ast_value_factory, &log, stack_limit);
     preparser.set_allow_lazy(true);
     i::PreParser::PreParseResult result = preparser.PreParseProgram();
     CHECK_EQ(i::PreParser::kPreParseSuccess, result);
@@ -416,7 +432,11 @@ TEST(RegressChromium62639) {
   i::CompleteParserRecorder log;
   i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
   scanner.Initialize(&stream);
-  i::PreParser preparser(CcTest::i_isolate(), &scanner, &log,
+  i::Zone zone;
+  i::AstValueFactory ast_value_factory(&zone,
+                                       CcTest::i_isolate()->heap()->HashSeed());
+  i::PreParser preparser(CcTest::i_isolate(), &zone, &scanner,
+                         &ast_value_factory, &log,
                          CcTest::i_isolate()->stack_guard()->real_climit());
   preparser.set_allow_lazy(true);
   i::PreParser::PreParseResult result = preparser.PreParseProgram();
@@ -448,7 +468,11 @@ TEST(Regress928) {
   i::CompleteParserRecorder log;
   i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
   scanner.Initialize(&stream);
-  i::PreParser preparser(CcTest::i_isolate(), &scanner, &log,
+  i::Zone zone;
+  i::AstValueFactory ast_value_factory(&zone,
+                                       CcTest::i_isolate()->heap()->HashSeed());
+  i::PreParser preparser(CcTest::i_isolate(), &zone, &scanner,
+                         &ast_value_factory, &log,
                          CcTest::i_isolate()->stack_guard()->real_climit());
   preparser.set_allow_lazy(true);
   i::PreParser::PreParseResult result = preparser.PreParseProgram();
@@ -497,7 +521,11 @@ TEST(PreParseOverflow) {
   i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
   scanner.Initialize(&stream);
 
-  i::PreParser preparser(CcTest::i_isolate(), &scanner, &log, stack_limit);
+  i::Zone zone;
+  i::AstValueFactory ast_value_factory(&zone,
+                                       CcTest::i_isolate()->heap()->HashSeed());
+  i::PreParser preparser(CcTest::i_isolate(), &zone, &scanner,
+                         &ast_value_factory, &log, stack_limit);
   preparser.set_allow_lazy(true);
   preparser.set_allow_harmony_arrow_functions(true);
   i::PreParser::PreParseResult result = preparser.PreParseProgram();
@@ -1411,7 +1439,11 @@ void TestParserSyncWithFlags(i::Handle<i::String> source,
   {
     i::Scanner scanner(isolate->unicode_cache());
     i::GenericStringUtf16CharacterStream stream(source, 0, source->length());
-    i::PreParser preparser(CcTest::i_isolate(), &scanner, &log, stack_limit);
+    i::Zone zone;
+    i::AstValueFactory ast_value_factory(
+        &zone, CcTest::i_isolate()->heap()->HashSeed());
+    i::PreParser preparser(CcTest::i_isolate(), &zone, &scanner,
+                           &ast_value_factory, &log, stack_limit);
     SetParserFlags(&preparser, flags);
     scanner.Initialize(&stream);
     i::PreParser::PreParseResult result = preparser.PreParseProgram(
-- 
2.7.4