Remove ast_value_factory_ and usages from scope

This frees up a field in Scope and untangles scope a little from the parser.

BUG=v8:5209

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

src/ast/scopes.cc

@@ -83,7 +83,7 @@ void SloppyBlockFunctionMap::Declare(const AstRawString* name,
 // Implementation of Scope
 
 Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
-             AstValueFactory* ast_value_factory, FunctionKind function_kind)
+             FunctionKind function_kind)
     : inner_scopes_(4, zone),
       variables_(zone),
       temps_(4, zone),
@@ -95,7 +95,6 @@ Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
                                                     : NULL),
       sloppy_block_function_map_(zone),
       already_resolved_(false),
-      ast_value_factory_(ast_value_factory),
       zone_(zone) {
   SetDefaults(scope_type, outer_scope, Handle<ScopeInfo>::null(),
               function_kind);
@@ -104,7 +103,7 @@ Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
 }
 
 Scope::Scope(Zone* zone, Scope* inner_scope, ScopeType scope_type,
-             Handle<ScopeInfo> scope_info, AstValueFactory* value_factory)
+             Handle<ScopeInfo> scope_info)
     : inner_scopes_(4, zone),
       variables_(zone),
       temps_(4, zone),
@@ -114,7 +113,6 @@ Scope::Scope(Zone* zone, Scope* inner_scope, ScopeType scope_type,
       module_descriptor_(NULL),
       sloppy_block_function_map_(zone),
       already_resolved_(true),
-      ast_value_factory_(value_factory),
       zone_(zone) {
   SetDefaults(scope_type, NULL, scope_info);
   if (!scope_info.is_null()) {
@@ -127,8 +125,7 @@ Scope::Scope(Zone* zone, Scope* inner_scope, ScopeType scope_type,
 }
 
 Scope::Scope(Zone* zone, Scope* inner_scope,
-             const AstRawString* catch_variable_name,
-             AstValueFactory* value_factory)
+             const AstRawString* catch_variable_name)
     : inner_scopes_(1, zone),
       variables_(zone),
       temps_(0, zone),
@@ -138,7 +135,6 @@ Scope::Scope(Zone* zone, Scope* inner_scope,
       module_descriptor_(NULL),
       sloppy_block_function_map_(zone),
       already_resolved_(true),
-      ast_value_factory_(value_factory),
       zone_(zone) {
   SetDefaults(CATCH_SCOPE, NULL, Handle<ScopeInfo>::null());
   AddInnerScope(inner_scope);
@@ -162,7 +158,7 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
       is_eval_scope() || is_function_scope() ||
       is_module_scope() || is_script_scope();
   function_kind_ = function_kind;
-  scope_name_ = ast_value_factory_->empty_string();
+  scope_name_ = nullptr;
   dynamics_ = nullptr;
   receiver_ = nullptr;
   new_target_ = nullptr;
@@ -206,9 +202,9 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
   }
 }
 
-
 Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
-                                    Context* context, Scope* script_scope) {
+                                    Context* context, Scope* script_scope,
+                                    AstValueFactory* ast_value_factory) {
   // Reconstruct the outer scope chain from a closure's context chain.
   Scope* current_scope = NULL;
   Scope* innermost_scope = NULL;
@@ -216,8 +212,7 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
     if (context->IsWithContext() || context->IsDebugEvaluateContext()) {
       // For scope analysis, debug-evaluate is equivalent to a with scope.
       Scope* with_scope = new (zone)
-          Scope(zone, current_scope, WITH_SCOPE, Handle<ScopeInfo>::null(),
-                script_scope->ast_value_factory_);
+          Scope(zone, current_scope, WITH_SCOPE, Handle<ScopeInfo>::null());
       current_scope = with_scope;
       // All the inner scopes are inside a with.
       for (Scope* s = innermost_scope; s != NULL; s = s->outer_scope()) {
@@ -226,27 +221,23 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
     } else if (context->IsScriptContext()) {
       ScopeInfo* scope_info = context->scope_info();
       current_scope = new (zone) Scope(zone, current_scope, SCRIPT_SCOPE,
-                                       Handle<ScopeInfo>(scope_info),
-                                       script_scope->ast_value_factory_);
+                                       Handle<ScopeInfo>(scope_info));
     } else if (context->IsFunctionContext()) {
       ScopeInfo* scope_info = context->closure()->shared()->scope_info();
       current_scope = new (zone) Scope(zone, current_scope, FUNCTION_SCOPE,
-                                       Handle<ScopeInfo>(scope_info),
-                                       script_scope->ast_value_factory_);
+                                       Handle<ScopeInfo>(scope_info));
       if (scope_info->IsAsmFunction()) current_scope->asm_function_ = true;
       if (scope_info->IsAsmModule()) current_scope->asm_module_ = true;
     } else if (context->IsBlockContext()) {
       ScopeInfo* scope_info = context->scope_info();
-      current_scope = new (zone)
-          Scope(zone, current_scope, BLOCK_SCOPE, Handle<ScopeInfo>(scope_info),
-                script_scope->ast_value_factory_);
+      current_scope = new (zone) Scope(zone, current_scope, BLOCK_SCOPE,
+                                       Handle<ScopeInfo>(scope_info));
     } else {
       DCHECK(context->IsCatchContext());
       String* name = context->catch_name();
-      current_scope = new (zone) Scope(
-          zone, current_scope,
-          script_scope->ast_value_factory_->GetString(Handle<String>(name)),
-          script_scope->ast_value_factory_);
+      current_scope =
+          new (zone) Scope(zone, current_scope,
+                           ast_value_factory->GetString(handle(name, isolate)));
     }
     if (innermost_scope == NULL) innermost_scope = current_scope;
     context = context->previous();
@@ -297,43 +288,47 @@ bool Scope::Analyze(ParseInfo* info) {
 
 void Scope::Initialize() {
   DCHECK(!already_resolved());
-
-  // Add this scope as a new inner scope of the outer scope.
-  if (outer_scope_ != NULL) {
+  if (outer_scope_ == nullptr) {
+    scope_inside_with_ = is_with_scope();
+  } else {
     outer_scope_->inner_scopes_.Add(this, zone());
     scope_inside_with_ = outer_scope_->scope_inside_with_ || is_with_scope();
-  } else {
-    scope_inside_with_ = is_with_scope();
   }
+}
 
-  // Declare convenience variables and the receiver.
-  if (is_declaration_scope() && has_this_declaration()) {
-    bool subclass_constructor = IsSubclassConstructor(function_kind_);
-    Variable* var = variables_.Declare(
-        this, ast_value_factory_->this_string(),
-        subclass_constructor ? CONST : VAR, Variable::THIS,
-        subclass_constructor ? kNeedsInitialization : kCreatedInitialized);
-    receiver_ = var;
-  }
-
-  if (is_function_scope() && !is_arrow_scope()) {
-    // Declare 'arguments' variable which exists in all non arrow functions.
-    // Note that it might never be accessed, in which case it won't be
-    // allocated during variable allocation.
-    arguments_ =
-        variables_.Declare(this, ast_value_factory_->arguments_string(), VAR,
-                           Variable::ARGUMENTS, kCreatedInitialized);
-
-    new_target_ =
-        variables_.Declare(this, ast_value_factory_->new_target_string(), CONST,
-                           Variable::NORMAL, kCreatedInitialized);
-
-    if (IsConciseMethod(function_kind_) || IsClassConstructor(function_kind_) ||
-        IsAccessorFunction(function_kind_)) {
-      this_function_ =
-          variables_.Declare(this, ast_value_factory_->this_function_string(),
-                             CONST, Variable::NORMAL, kCreatedInitialized);
-    }
+void Scope::DeclareThis(AstValueFactory* ast_value_factory) {
+  DCHECK(!already_resolved());
+  DCHECK(is_declaration_scope());
+  DCHECK(has_this_declaration());
+
+  bool subclass_constructor = IsSubclassConstructor(function_kind_);
+  Variable* var = variables_.Declare(
+      this, ast_value_factory->this_string(),
+      subclass_constructor ? CONST : VAR, Variable::THIS,
+      subclass_constructor ? kNeedsInitialization : kCreatedInitialized);
+  receiver_ = var;
+}
+
+void Scope::DeclareDefaultFunctionVariables(
+    AstValueFactory* ast_value_factory) {
+  DCHECK(is_function_scope());
+  DCHECK(!is_arrow_scope());
+  // Declare 'arguments' variable which exists in all non arrow functions.
+  // Note that it might never be accessed, in which case it won't be
+  // allocated during variable allocation.
+  arguments_ =
+      variables_.Declare(this, ast_value_factory->arguments_string(), VAR,
+                         Variable::ARGUMENTS, kCreatedInitialized);
+
+  new_target_ =
+      variables_.Declare(this, ast_value_factory->new_target_string(), CONST,
+                         Variable::NORMAL, kCreatedInitialized);
+
+  if (IsConciseMethod(function_kind_) || IsClassConstructor(function_kind_) ||
+      IsAccessorFunction(function_kind_)) {
+    this_function_ =
+        variables_.Declare(this, ast_value_factory->this_function_string(),
+                           CONST, Variable::NORMAL, kCreatedInitialized);
   }
 }
 

src/ast/scopes.h

@@ -90,7 +90,6 @@ class Scope: public ZoneObject {
   // Construction
 
   Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
-        AstValueFactory* value_factory,
         FunctionKind function_kind = kNormalFunction);
 
   // Compute top scope and allocate variables. For lazy compilation the top
@@ -99,7 +98,8 @@ class Scope: public ZoneObject {
   static bool Analyze(ParseInfo* info);
 
   static Scope* DeserializeScopeChain(Isolate* isolate, Zone* zone,
-                                      Context* context, Scope* script_scope);
+                                      Context* context, Scope* script_scope,
+                                      AstValueFactory* ast_value_factory);
 
   // The scope name is only used for printing/debugging.
   void SetScopeName(const AstRawString* scope_name) {
@@ -107,6 +107,8 @@ class Scope: public ZoneObject {
   }
 
   void Initialize();
+  void DeclareThis(AstValueFactory* ast_value_factory);
+  void DeclareDefaultFunctionVariables(AstValueFactory* ast_value_factory);
 
   // Checks if the block scope is redundant, i.e. it does not contain any
   // block scoped declarations. In that case it is removed from the scope
@@ -801,11 +803,11 @@ class Scope: public ZoneObject {
 
   // Construct a scope based on the scope info.
   Scope(Zone* zone, Scope* inner_scope, ScopeType type,
-        Handle<ScopeInfo> scope_info, AstValueFactory* value_factory);
+        Handle<ScopeInfo> scope_info);
 
   // Construct a catch scope with a binding for the name.
-  Scope(Zone* zone, Scope* inner_scope, const AstRawString* catch_variable_name,
-        AstValueFactory* value_factory);
+  Scope(Zone* zone, Scope* inner_scope,
+        const AstRawString* catch_variable_name);
 
   void AddInnerScope(Scope* inner_scope) {
     if (inner_scope != NULL) {
@@ -828,7 +830,6 @@ class Scope: public ZoneObject {
                    Handle<ScopeInfo> scope_info,
                    FunctionKind function_kind = kNormalFunction);
 
-  AstValueFactory* ast_value_factory_;
   Zone* zone_;
 
   PendingCompilationErrorHandler pending_error_handler_;

src/parsing/parser-base.h

@@ -618,14 +618,21 @@ class ParserBase : public Traits {
   Scope* NewScope(Scope* parent, ScopeType scope_type) {
     // Must always pass the function kind for FUNCTION_SCOPE.
     DCHECK(scope_type != FUNCTION_SCOPE);
-    return NewScope(parent, scope_type, kNormalFunction);
+    Scope* result =
+        new (zone()) Scope(zone(), parent, scope_type, kNormalFunction);
+    result->Initialize();
+    if (scope_type == MODULE_SCOPE) result->DeclareThis(ast_value_factory());
+    return result;
   }
 
-  Scope* NewScope(Scope* parent, ScopeType scope_type, FunctionKind kind) {
+  Scope* NewFunctionScope(Scope* parent, FunctionKind kind) {
     DCHECK(ast_value_factory());
-    Scope* result = new (zone())
-        Scope(zone(), parent, scope_type, ast_value_factory(), kind);
+    Scope* result = new (zone()) Scope(zone(), parent, FUNCTION_SCOPE, kind);
     result->Initialize();
+    if (!IsArrowFunction(kind)) {
+      result->DeclareThis(ast_value_factory());
+      result->DeclareDefaultFunctionVariables(ast_value_factory());
+    }
     return result;
   }
 
@@ -2307,9 +2314,9 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN,
     ValidateFormalParameterInitializer(&arrow_formals_classifier, ok);
 
     Scanner::Location loc(lhs_beg_pos, scanner()->location().end_pos);
-    Scope* scope = this->NewScope(this->scope(), FUNCTION_SCOPE,
-                                  is_async ? FunctionKind::kAsyncArrowFunction
-                                           : FunctionKind::kArrowFunction);
+    Scope* scope = this->NewFunctionScope(
+        this->scope(), is_async ? FunctionKind::kAsyncArrowFunction
+                                : FunctionKind::kArrowFunction);
     // Because the arrow's parameters were parsed in the outer scope, any
     // usage flags that might have been triggered there need to be copied
     // to the arrow scope.

src/parsing/parser.cc

@@ -210,7 +210,7 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
 
   FunctionKind kind = call_super ? FunctionKind::kDefaultSubclassConstructor
                                  : FunctionKind::kDefaultBaseConstructor;
-  Scope* function_scope = NewScope(scope, FUNCTION_SCOPE, kind);
+  Scope* function_scope = NewFunctionScope(scope, kind);
   SetLanguageMode(function_scope,
                   static_cast<LanguageMode>(language_mode | STRICT));
   // Set start and end position to the same value
@@ -943,7 +943,8 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
     info->set_script_scope(scope);
     if (!info->context().is_null() && !info->context()->IsNativeContext()) {
       scope = Scope::DeserializeScopeChain(info->isolate(), zone(),
-                                           *info->context(), scope);
+                                           *info->context(), scope,
+                                           ast_value_factory());
       // The Scope is backed up by ScopeInfo (which is in the V8 heap); this
       // means the Parser cannot operate independent of the V8 heap. Tell the
       // string table to internalize strings and values right after they're
@@ -1109,7 +1110,7 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info,
       // main thread.
       DCHECK(parsing_on_main_thread_);
       scope = Scope::DeserializeScopeChain(isolate, zone(), *info->context(),
-                                           scope);
+                                           scope, ast_value_factory());
     }
     original_scope_ = scope;
     AstNodeFactory function_factory(ast_value_factory());
@@ -1138,7 +1139,7 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info,
 
       // TODO(adamk): We should construct this scope from the ScopeInfo.
       Scope* scope =
-          NewScope(this->scope(), FUNCTION_SCOPE, FunctionKind::kArrowFunction);
+          NewFunctionScope(this->scope(), FunctionKind::kArrowFunction);
 
       // These two bits only need to be explicitly set because we're
       // not passing the ScopeInfo to the Scope constructor.
@@ -4293,7 +4294,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
     function_name = ast_value_factory()->empty_string();
   }
 
-  Scope* scope = NewScope(this->scope(), FUNCTION_SCOPE, kind);
+  Scope* scope = NewFunctionScope(this->scope(), kind);
   SetLanguageMode(scope, language_mode);
   ZoneList<Statement*>* body = NULL;
   int arity = -1;

src/parsing/parser.h

@@ -542,8 +542,8 @@ class ParserTraits {
       ZoneList<Statement*>* body, bool accept_IN,
       Type::ExpressionClassifier* classifier, int pos, bool* ok);
 
-  V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type,
-                            FunctionKind kind = kNormalFunction);
+  V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type);
+  V8_INLINE Scope* NewFunctionScope(Scope* parent_scope, FunctionKind kind);
 
   V8_INLINE void AddFormalParameter(ParserFormalParameters* parameters,
                                     Expression* pattern,
@@ -1143,12 +1143,13 @@ bool ParserTraits::IsFutureStrictReserved(
   return parser_->scanner()->IdentifierIsFutureStrictReserved(identifier);
 }
 
-
-Scope* ParserTraits::NewScope(Scope* parent_scope, ScopeType scope_type,
-                              FunctionKind kind) {
-  return parser_->NewScope(parent_scope, scope_type, kind);
+Scope* ParserTraits::NewScope(Scope* parent_scope, ScopeType scope_type) {
+  return parser_->NewScope(parent_scope, scope_type);
 }
 
+Scope* ParserTraits::NewFunctionScope(Scope* parent_scope, FunctionKind kind) {
+  return parser_->NewFunctionScope(parent_scope, kind);
+}
 
 const AstRawString* ParserTraits::EmptyIdentifierString() {
   return parser_->ast_value_factory()->empty_string();

src/parsing/preparser.cc

@@ -136,7 +136,7 @@ PreParser::PreParseResult PreParser::PreParseLazyFunction(
   FunctionState top_state(&function_state_, &scope_state_, top_scope,
                           kNormalFunction, &top_factory);
   scope()->SetLanguageMode(language_mode);
-  Scope* function_scope = NewScope(scope(), FUNCTION_SCOPE, kind);
+  Scope* function_scope = NewFunctionScope(scope(), kind);
   if (!has_simple_parameters) function_scope->SetHasNonSimpleParameters();
   PreParserFactory function_factory(nullptr);
   FunctionState function_state(&function_state_, &scope_state_, function_scope,
@@ -1111,7 +1111,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
 
   // Parse function body.
   bool outer_is_script_scope = scope()->is_script_scope();
-  Scope* function_scope = NewScope(scope(), FUNCTION_SCOPE, kind);
+  Scope* function_scope = NewFunctionScope(scope(), kind);
   function_scope->SetLanguageMode(language_mode);
   PreParserFactory factory(NULL);
   FunctionState function_state(&function_state_, &scope_state_, function_scope,

test/cctest/test-parsing.cc

@@ -3361,10 +3361,10 @@ TEST(SerializationOfMaybeAssignmentFlag) {
   i::Handle<i::String> str = name->string();
   CHECK(str->IsInternalizedString());
   i::Scope* script_scope =
-      new (&zone) i::Scope(&zone, NULL, i::SCRIPT_SCOPE, &avf);
+      new (&zone) i::Scope(&zone, nullptr, i::SCRIPT_SCOPE);
   script_scope->Initialize();
-  i::Scope* s =
-      i::Scope::DeserializeScopeChain(isolate, &zone, context, script_scope);
+  i::Scope* s = i::Scope::DeserializeScopeChain(isolate, &zone, context,
+                                                script_scope, &avf);
   CHECK(s != script_scope);
   CHECK(name != NULL);
 
@@ -3409,10 +3409,10 @@ TEST(IfArgumentsArrayAccessedThenParametersMaybeAssigned) {
   avf.Internalize(isolate);
 
   i::Scope* script_scope =
-      new (&zone) i::Scope(&zone, NULL, i::SCRIPT_SCOPE, &avf);
+      new (&zone) i::Scope(&zone, nullptr, i::SCRIPT_SCOPE);
   script_scope->Initialize();
-  i::Scope* s =
-      i::Scope::DeserializeScopeChain(isolate, &zone, context, script_scope);
+  i::Scope* s = i::Scope::DeserializeScopeChain(isolate, &zone, context,
+                                                script_scope, &avf);
   CHECK(s != script_scope);
   const i::AstRawString* name_x = avf.GetOneByteString("x");