Cleanups needed for this-scoping in arrow functions

Remove Variable::IsValidReference(), and the Variable::is_valid_ref_
member: This was "false" only for "this", and for internal variables.
For the first, VariableProxy::is_this() can be used for the check
instead; and for internal variables, it is guaranteed they they will
not be written to (because the V8 code does not do it, and they are
not accessible from JavaScript).

The "bool is_this" parameter of VariableProxy() constructor is
changed to use Variable::Kind. This will allow to later on adding
a parameter to create unresolved variables of any kind, which in
turn will be used to make references to "this" initially unresolved,
and use the existing variable resolution mechanics for "this".

BUG=v8:2700
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#27404}

src/ast.cc

@@ -72,10 +72,12 @@ VariableProxy::VariableProxy(Zone* zone, Variable* var, int start_position,
 }
 
 
-VariableProxy::VariableProxy(Zone* zone, const AstRawString* name, bool is_this,
-                             int start_position, int end_position)
+VariableProxy::VariableProxy(Zone* zone, const AstRawString* name,
+                             Variable::Kind variable_kind, int start_position,
+                             int end_position)
     : Expression(zone, start_position),
-      bit_field_(IsThisField::encode(is_this) | IsAssignedField::encode(false) |
+      bit_field_(IsThisField::encode(variable_kind == Variable::THIS) |
+                 IsAssignedField::encode(false) |
                  IsResolvedField::encode(false)),
       variable_feedback_slot_(FeedbackVectorICSlot::Invalid()),
       raw_name_(name),

src/ast.h

@@ -1623,9 +1623,7 @@ class VariableProxy FINAL : public Expression {
  public:
   DECLARE_NODE_TYPE(VariableProxy)
 
-  bool IsValidReferenceExpression() const OVERRIDE {
-    return !is_resolved() || var()->IsValidReference();
-  }
+  bool IsValidReferenceExpression() const OVERRIDE { return !is_this(); }
 
   bool IsArguments() const { return is_resolved() && var()->is_arguments(); }
 
@@ -1680,8 +1678,9 @@ class VariableProxy FINAL : public Expression {
   VariableProxy(Zone* zone, Variable* var, int start_position,
                 int end_position);
 
-  VariableProxy(Zone* zone, const AstRawString* name, bool is_this,
-                int start_position, int end_position);
+  VariableProxy(Zone* zone, const AstRawString* name,
+                Variable::Kind variable_kind, int start_position,
+                int end_position);
 
   class IsThisField : public BitField8<bool, 0, 1> {};
   class IsAssignedField : public BitField8<bool, 1, 1> {};
@@ -3402,11 +3401,12 @@ class AstNodeFactory FINAL BASE_EMBEDDED {
     return new (zone_) VariableProxy(zone_, var, start_position, end_position);
   }
 
-  VariableProxy* NewVariableProxy(const AstRawString* name, bool is_this,
+  VariableProxy* NewVariableProxy(const AstRawString* name,
+                                  Variable::Kind variable_kind,
                                   int start_position = RelocInfo::kNoPosition,
                                   int end_position = RelocInfo::kNoPosition) {
     return new (zone_)
-        VariableProxy(zone_, name, is_this, start_position, end_position);
+        VariableProxy(zone_, name, variable_kind, start_position, end_position);
   }
 
   Property* NewProperty(Expression* obj, Expression* key, int pos) {

src/parser.cc

@@ -790,7 +790,7 @@ Expression* ParserTraits::ExpressionFromIdentifier(const AstRawString* name,
   // for Traits::DeclareArrowParametersFromExpression() to be able to
   // pick the names of the parameters.
   return parser_->parsing_lazy_arrow_parameters_
-             ? factory->NewVariableProxy(name, false, start_position,
+             ? factory->NewVariableProxy(name, Variable::NORMAL, start_position,
                                          end_position)
              : scope->NewUnresolved(factory, name, start_position,
                                     end_position);
@@ -1975,7 +1975,7 @@ Variable* Parser::Declare(Declaration* declaration, bool resolve, bool* ok) {
     // For global const variables we bind the proxy to a variable.
     DCHECK(resolve);  // should be set by all callers
     Variable::Kind kind = Variable::NORMAL;
-    var = new (zone()) Variable(declaration_scope, name, mode, true, kind,
+    var = new (zone()) Variable(declaration_scope, name, mode, kind,
                                 kNeedsInitialization, kNotAssigned);
   } else if (declaration_scope->is_eval_scope() &&
              is_sloppy(declaration_scope->language_mode())) {
@@ -1984,7 +1984,7 @@ Variable* Parser::Declare(Declaration* declaration, bool resolve, bool* ok) {
     // DeclareLookupSlot runtime function.
     Variable::Kind kind = Variable::NORMAL;
     // TODO(sigurds) figure out if kNotAssigned is OK here
-    var = new (zone()) Variable(declaration_scope, name, mode, true, kind,
+    var = new (zone()) Variable(declaration_scope, name, mode, kind,
                                 declaration->initialization(), kNotAssigned);
     var->AllocateTo(Variable::LOOKUP, -1);
     resolve = true;
@@ -3883,8 +3883,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
           is_strict(language_mode()) ? CONST : CONST_LEGACY;
       DCHECK(function_name != NULL);
       fvar = new (zone())
-          Variable(scope_, function_name, fvar_mode, true /* is valid LHS */,
-                   Variable::NORMAL, kCreatedInitialized, kNotAssigned);
+          Variable(scope_, function_name, fvar_mode, Variable::NORMAL,
+                   kCreatedInitialized, kNotAssigned);
       VariableProxy* proxy = factory()->NewVariableProxy(fvar);
       VariableDeclaration* fvar_declaration = factory()->NewVariableDeclaration(
           proxy, fvar_mode, scope_, RelocInfo::kNoPosition);

src/scopes.cc

@@ -30,8 +30,7 @@ VariableMap::~VariableMap() {}
 
 
 Variable* VariableMap::Declare(Scope* scope, const AstRawString* name,
-                               VariableMode mode, bool is_valid_lhs,
-                               Variable::Kind kind,
+                               VariableMode mode, Variable::Kind kind,
                                InitializationFlag initialization_flag,
                                MaybeAssignedFlag maybe_assigned_flag) {
   // AstRawStrings are unambiguous, i.e., the same string is always represented
@@ -42,7 +41,7 @@ Variable* VariableMap::Declare(Scope* scope, const AstRawString* name,
   if (p->value == NULL) {
     // The variable has not been declared yet -> insert it.
     DCHECK(p->key == name);
-    p->value = new (zone()) Variable(scope, name, mode, is_valid_lhs, kind,
+    p->value = new (zone()) Variable(scope, name, mode, kind,
                                      initialization_flag, maybe_assigned_flag);
   }
   return reinterpret_cast<Variable*>(p->value);
@@ -131,7 +130,6 @@ Scope::Scope(Zone* zone, Scope* inner_scope,
   Variable* variable = variables_.Declare(this,
                                           catch_variable_name,
                                           VAR,
-                                          true,  // Valid left-hand side.
                                           Variable::NORMAL,
                                           kCreatedInitialized);
   AllocateHeapSlot(variable);
@@ -313,15 +311,15 @@ void Scope::Initialize() {
     DCHECK(!subclass_constructor || is_function_scope());
     Variable* var = variables_.Declare(
         this, ast_value_factory_->this_string(),
-        subclass_constructor ? CONST : VAR, false, Variable::THIS,
+        subclass_constructor ? CONST : VAR, Variable::THIS,
         subclass_constructor ? kNeedsInitialization : kCreatedInitialized);
     var->AllocateTo(Variable::PARAMETER, -1);
     receiver_ = var;
 
     if (subclass_constructor) {
-      new_target_ = variables_.Declare(
-          this, ast_value_factory_->new_target_string(), CONST, false,
-          Variable::NEW_TARGET, kCreatedInitialized);
+      new_target_ =
+          variables_.Declare(this, ast_value_factory_->new_target_string(),
+                             CONST, Variable::NEW_TARGET, kCreatedInitialized);
       new_target_->AllocateTo(Variable::PARAMETER, -2);
       new_target_->set_is_used();
     }
@@ -337,7 +335,6 @@ void Scope::Initialize() {
     variables_.Declare(this,
                        ast_value_factory_->arguments_string(),
                        VAR,
-                       true,
                        Variable::ARGUMENTS,
                        kCreatedInitialized);
   }
@@ -413,7 +410,7 @@ Variable* Scope::LookupLocal(const AstRawString* name) {
     maybe_assigned_flag = kMaybeAssigned;
   }
 
-  Variable* var = variables_.Declare(this, name, mode, true, Variable::NORMAL,
+  Variable* var = variables_.Declare(this, name, mode, Variable::NORMAL,
                                      init_flag, maybe_assigned_flag);
   var->AllocateTo(location, index);
   return var;
@@ -429,9 +426,8 @@ Variable* Scope::LookupFunctionVar(const AstRawString* name,
     VariableMode mode;
     int index = scope_info_->FunctionContextSlotIndex(*(name->string()), &mode);
     if (index < 0) return NULL;
-    Variable* var = new(zone()) Variable(
-        this, name, mode, true /* is valid LHS */,
-        Variable::NORMAL, kCreatedInitialized);
+    Variable* var = new (zone())
+        Variable(this, name, mode, Variable::NORMAL, kCreatedInitialized);
     VariableProxy* proxy = factory->NewVariableProxy(var);
     VariableDeclaration* declaration = factory->NewVariableDeclaration(
         proxy, mode, this, RelocInfo::kNoPosition);
@@ -459,7 +455,7 @@ Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode mode,
                                   bool is_rest) {
   DCHECK(!already_resolved());
   DCHECK(is_function_scope());
-  Variable* var = variables_.Declare(this, name, mode, true, Variable::NORMAL,
+  Variable* var = variables_.Declare(this, name, mode, Variable::NORMAL,
                                      kCreatedInitialized);
   if (is_rest) {
     DCHECK_NULL(rest_parameter_);
@@ -480,7 +476,7 @@ Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
   // explicitly, and TEMPORARY variables are allocated via NewTemporary().
   DCHECK(IsDeclaredVariableMode(mode));
   ++num_var_or_const_;
-  return variables_.Declare(this, name, mode, true, kind, init_flag,
+  return variables_.Declare(this, name, mode, kind, init_flag,
                             maybe_assigned_flag);
 }
 
@@ -490,7 +486,6 @@ Variable* Scope::DeclareDynamicGlobal(const AstRawString* name) {
   return variables_.Declare(this,
                             name,
                             DYNAMIC_GLOBAL,
-                            true,
                             Variable::NORMAL,
                             kCreatedInitialized);
 }
@@ -513,7 +508,6 @@ Variable* Scope::NewInternal(const AstRawString* name) {
   Variable* var = new(zone()) Variable(this,
                                        name,
                                        INTERNAL,
-                                       false,
                                        Variable::NORMAL,
                                        kCreatedInitialized);
   internals_.Add(var, zone());
@@ -526,7 +520,6 @@ Variable* Scope::NewTemporary(const AstRawString* name) {
   Variable* var = new(zone()) Variable(this,
                                        name,
                                        TEMPORARY,
-                                       true,
                                        Variable::NORMAL,
                                        kCreatedInitialized);
   temps_.Add(var, zone());
@@ -987,7 +980,6 @@ Variable* Scope::NonLocal(const AstRawString* name, VariableMode mode) {
     var = map->Declare(NULL,
                        name,
                        mode,
-                       true,
                        Variable::NORMAL,
                        init_flag);
     // Allocate it by giving it a dynamic lookup.

src/scopes.h

@@ -22,8 +22,7 @@ class VariableMap: public ZoneHashMap {
   virtual ~VariableMap();
 
   Variable* Declare(Scope* scope, const AstRawString* name, VariableMode mode,
-                    bool is_valid_lhs, Variable::Kind kind,
-                    InitializationFlag initialization_flag,
+                    Variable::Kind kind, InitializationFlag initialization_flag,
                     MaybeAssignedFlag maybe_assigned_flag = kNotAssigned);
 
   Variable* Lookup(const AstRawString* name);
@@ -149,8 +148,8 @@ class Scope: public ZoneObject {
     // the same name because they may be removed selectively via
     // RemoveUnresolved().
     DCHECK(!already_resolved());
-    VariableProxy* proxy =
-        factory->NewVariableProxy(name, false, start_position, end_position);
+    VariableProxy* proxy = factory->NewVariableProxy(
+        name, Variable::NORMAL, start_position, end_position);
     unresolved_.Add(proxy, zone_);
     return proxy;
   }

src/variables.cc

@@ -33,8 +33,7 @@ const char* Variable::Mode2String(VariableMode mode) {
 
 
 Variable::Variable(Scope* scope, const AstRawString* name, VariableMode mode,
-                   bool is_valid_ref, Kind kind,
-                   InitializationFlag initialization_flag,
+                   Kind kind, InitializationFlag initialization_flag,
                    MaybeAssignedFlag maybe_assigned_flag)
     : scope_(scope),
       name_(name),
@@ -44,7 +43,6 @@ Variable::Variable(Scope* scope, const AstRawString* name, VariableMode mode,
       index_(-1),
       initializer_position_(RelocInfo::kNoPosition),
       local_if_not_shadowed_(NULL),
-      is_valid_ref_(is_valid_ref),
       force_context_allocation_(false),
       is_used_(false),
       initialization_flag_(initialization_flag),

src/variables.h

@@ -47,15 +47,13 @@ class Variable: public ZoneObject {
     LOOKUP
   };
 
-  Variable(Scope* scope, const AstRawString* name, VariableMode mode,
-           bool is_valid_ref, Kind kind, InitializationFlag initialization_flag,
+  Variable(Scope* scope, const AstRawString* name, VariableMode mode, Kind kind,
+           InitializationFlag initialization_flag,
            MaybeAssignedFlag maybe_assigned_flag = kNotAssigned);
 
   // Printing support
   static const char* Mode2String(VariableMode mode);
 
-  bool IsValidReference() { return is_valid_ref_; }
-
   // The source code for an eval() call may refer to a variable that is
   // in an outer scope about which we don't know anything (it may not
   // be the script scope). scope() is NULL in that case. Currently the
@@ -145,9 +143,6 @@ class Variable: public ZoneObject {
   // binding scope (exclusive).
   Variable* local_if_not_shadowed_;
 
-  // Valid as a reference? (const and this are not valid, for example)
-  bool is_valid_ref_;
-
   // Usage info.
   bool force_context_allocation_;  // set by variable resolver
   bool is_used_;