Redirect most NewUnresolved calls over Parser

src/parsing/parser.cc

Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index c920c9c87fe78ed50c0ada65bf85ddd35630a486..3f4305bd8149f11ee9acb524f92c14e27ad23b78 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -275,9 +275,8 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
 
       ZoneList<Expression*>* args =
           new (zone()) ZoneList<Expression*>(2, zone());
-      VariableProxy* this_function_proxy = this->scope()->NewUnresolved(
-          factory(), ast_value_factory()->this_function_string(),
-          Variable::NORMAL, pos);
+      VariableProxy* this_function_proxy = NewUnresolved(
+          ast_value_factory()->this_function_string(), Variable::NORMAL, pos);

[adamk, 2016/08/09 20:11:45]

I don't think we want callers like this to have to specify Variable::NORMAL. The
only use of the Variable::Kind argument is to be able to specify Variable::THIS,
so I'd prefer if the weirdness was kept to that function alone.

       ZoneList<Expression*>* tmp =
           new (zone()) ZoneList<Expression*>(1, zone());
       tmp->Add(this_function_proxy, zone());
@@ -290,9 +289,8 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
           new (zone()) ZoneList<Expression*>(1, zone());
       spread_args_expr->Add(spread_args, zone());
       args->AddAll(*PrepareSpreadArguments(spread_args_expr), zone());
-      VariableProxy* new_target_proxy = this->scope()->NewUnresolved(
-          factory(), ast_value_factory()->new_target_string(), Variable::NORMAL,
-          pos);
+      VariableProxy* new_target_proxy = NewUnresolved(
+          ast_value_factory()->new_target_string(), Variable::NORMAL, pos);
       args->Add(new_target_proxy, zone());
       CallRuntime* call = factory()->NewCallRuntime(
           Context::REFLECT_CONSTRUCT_INDEX, args, pos);
@@ -690,45 +688,42 @@ const AstRawString* ParserTraits::GetNextSymbol(Scanner* scanner) {
   return parser_->scanner()->NextSymbol(parser_->ast_value_factory());
 }
 
-Expression* ParserTraits::ThisExpression(AstNodeFactory* factory, int pos) {
-  return parser_->scope()->NewUnresolved(
-      factory, parser_->ast_value_factory()->this_string(), Variable::THIS, pos,
-      pos + 4);
+Expression* ParserTraits::ThisExpression(int pos) {
+  return parser_->NewUnresolved(parser_->ast_value_factory()->this_string(),

[adamk, 2016/08/09 20:11:45]

I think this is the only caller that should have to specify a Variable::Kind.

+                                Variable::THIS, pos, pos + 4);
 }
 
 Expression* ParserTraits::NewSuperPropertyReference(AstNodeFactory* factory,
                                                     int pos) {
   // this_function[home_object_symbol]
-  VariableProxy* this_function_proxy = parser_->scope()->NewUnresolved(
-      factory, parser_->ast_value_factory()->this_function_string(),
-      Variable::NORMAL, pos);
+  VariableProxy* this_function_proxy = parser_->NewUnresolved(
+      parser_->ast_value_factory()->this_function_string(), Variable::NORMAL,
+      pos);
   Expression* home_object_symbol_literal =
       factory->NewSymbolLiteral("home_object_symbol", kNoSourcePosition);
   Expression* home_object = factory->NewProperty(
       this_function_proxy, home_object_symbol_literal, pos);
   return factory->NewSuperPropertyReference(
-      ThisExpression(factory, pos)->AsVariableProxy(), home_object, pos);
+      ThisExpression(pos)->AsVariableProxy(), home_object, pos);
 }
 
 Expression* ParserTraits::NewSuperCallReference(AstNodeFactory* factory,
                                                 int pos) {
-  VariableProxy* new_target_proxy = parser_->scope()->NewUnresolved(
-      factory, parser_->ast_value_factory()->new_target_string(),
-      Variable::NORMAL, pos);
-  VariableProxy* this_function_proxy = parser_->scope()->NewUnresolved(
-      factory, parser_->ast_value_factory()->this_function_string(),
-      Variable::NORMAL, pos);
-  return factory->NewSuperCallReference(
-      ThisExpression(factory, pos)->AsVariableProxy(), new_target_proxy,
-      this_function_proxy, pos);
-}
-
-Expression* ParserTraits::NewTargetExpression(AstNodeFactory* factory,
-                                              int pos) {
+  VariableProxy* new_target_proxy = parser_->NewUnresolved(
+      parser_->ast_value_factory()->new_target_string(), Variable::NORMAL, pos);
+  VariableProxy* this_function_proxy = parser_->NewUnresolved(
+      parser_->ast_value_factory()->this_function_string(), Variable::NORMAL,
+      pos);
+  return factory->NewSuperCallReference(ThisExpression(pos)->AsVariableProxy(),
+                                        new_target_proxy, this_function_proxy,
+                                        pos);
+}
+
+Expression* ParserTraits::NewTargetExpression(int pos) {
   static const int kNewTargetStringLength = 10;
-  auto proxy = parser_->scope()->NewUnresolved(
-      factory, parser_->ast_value_factory()->new_target_string(),
-      Variable::NORMAL, pos, pos + kNewTargetStringLength);
+  auto proxy = parser_->NewUnresolved(
+      parser_->ast_value_factory()->new_target_string(), Variable::NORMAL, pos,
+      pos + kNewTargetStringLength);
   proxy->set_is_new_target();
   return proxy;
 }
@@ -771,14 +766,12 @@ Literal* ParserTraits::ExpressionFromLiteral(Token::Value token, int pos,
   return NULL;
 }
 
-
 Expression* ParserTraits::ExpressionFromIdentifier(const AstRawString* name,
                                                    int start_position,
-                                                   int end_position,
-                                                   AstNodeFactory* factory) {
+                                                   int end_position) {
   if (parser_->fni_ != NULL) parser_->fni_->PushVariableName(name);
-  return parser_->scope()->NewUnresolved(factory, name, Variable::NORMAL,
-                                         start_position, end_position);
+  return parser_->NewUnresolved(name, Variable::NORMAL, start_position,
+                                end_position);
 }
 
 
@@ -1947,18 +1940,22 @@ Statement* Parser::ParseStatementAsUnlabelled(
   }
 }
 
-
 VariableProxy* Parser::NewUnresolved(const AstRawString* name,
-                                     VariableMode mode) {
+                                     Variable::Kind kind, int begin_pos,
+                                     int end_pos, VariableMode mode) {
   // If we are inside a function, a declaration of a 'var' variable is a
   // truly local variable, and the scope of the variable is always the function
   // scope.
   // Let/const variables are always added to the immediately enclosing scope.
   Scope* scope =
       IsLexicalVariableMode(mode) ? this->scope() : GetDeclarationScope();
-  return scope->NewUnresolved(factory(), name, Variable::NORMAL,
-                              scanner()->location().beg_pos,
-                              scanner()->location().end_pos);
+  return scope->NewUnresolved(factory(), name, kind, begin_pos, end_pos);
+}
+
+VariableProxy* Parser::NewUnresolved(const AstRawString* name,
+                                     VariableMode mode) {
+  return NewUnresolved(name, Variable::NORMAL, scanner()->location().beg_pos,

[Toon Verwaest, 2016/08/09 19:20:03]

Not exactly sure about redirecting this through the method above, moving
VariableMode there.

+                       scanner()->location().end_pos, mode);
 }
 
 InitializationFlag Parser::DefaultInitializationFlag(VariableMode mode) {
@@ -2716,7 +2713,7 @@ Statement* Parser::ParseReturnStatement(bool* ok) {
       tok == Token::RBRACE ||
       tok == Token::EOS) {
     if (IsSubclassConstructor(function_state_->kind())) {
-      return_value = ThisExpression(factory(), loc.beg_pos);
+      return_value = ThisExpression(loc.beg_pos);
     } else {
       return_value = GetLiteralUndefined(position());
     }
@@ -2763,9 +2760,8 @@ Statement* Parser::ParseReturnStatement(bool* ok) {
           factory()->NewUndefinedLiteral(kNoSourcePosition), pos);
 
       // is_undefined ? this : is_object_conditional
-      return_value = factory()->NewConditional(is_undefined,
-                                               ThisExpression(factory(), pos),
-                                               is_object_conditional, pos);
+      return_value = factory()->NewConditional(
+          is_undefined, ThisExpression(pos), is_object_conditional, pos);
     } else {
       ReturnExprScope maybe_allow_tail_calls(
           function_state_, ReturnExprContext::kInsideValidReturnStatement);
@@ -4758,9 +4754,8 @@ Expression* Parser::BuildCreateJSGeneratorObject(int pos, FunctionKind kind) {
   DCHECK_NOT_NULL(function_state_->generator_object_variable());
   ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
   args->Add(factory()->NewThisFunction(pos), zone());
-  args->Add(IsArrowFunction(kind)
-                ? GetLiteralUndefined(pos)
-                : ThisExpression(factory(), kNoSourcePosition),
+  args->Add(IsArrowFunction(kind) ? GetLiteralUndefined(pos)
+                                  : ThisExpression(kNoSourcePosition),
             zone());
   return factory()->NewCallRuntime(Runtime::kCreateJSGeneratorObject, args,
                                    pos);
@@ -4881,8 +4876,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
 
     if (IsSubclassConstructor(kind)) {
       body->Add(factory()->NewReturnStatement(
-                    this->ThisExpression(factory(), kNoSourcePosition),
-                    kNoSourcePosition),
+                    this->ThisExpression(kNoSourcePosition), kNoSourcePosition),
                 zone());
     }
   }
@@ -5700,7 +5694,7 @@ Expression* Parser::SpreadCall(Expression* function,
     if (function->IsProperty()) {
       // Method calls
       if (function->AsProperty()->IsSuperAccess()) {
-        Expression* home = ThisExpression(factory(), kNoSourcePosition);
+        Expression* home = ThisExpression(kNoSourcePosition);
         args->InsertAt(0, function, zone());
         args->InsertAt(1, home, zone());
       } else {
@@ -5945,7 +5939,7 @@ Expression* Parser::RewriteAssignExponentiation(Expression* left,
     Expression* result;
     DCHECK_NOT_NULL(lhs->raw_name());
     result = this->ExpressionFromIdentifier(lhs->raw_name(), lhs->position(),
-                                            lhs->end_position(), factory());
+                                            lhs->end_position());
     args->Add(left, zone());
     args->Add(right, zone());
     Expression* call =