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}

diff --git a/src/ast.cc b/src/ast.cc
index c907df19b45a1b3641ae16d215cd2d262c4db0a6..f8289311c47812f56e4249678339e0b9f6b80ca0 100644 (file)
--- a/src/ast.cc
+++ b/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),

diff --git a/src/ast.h b/src/ast.h
index 60ba21d986c29243e7ec0d8dd4b5ad6b5136b120..676ec262310307173264f057b6d7a4d498600d81 100644 (file)
--- a/src/ast.h
+++ b/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) {

diff --git a/src/parser.cc b/src/parser.cc
index ccfefb8a70933117ece73bda6c6b692e05ddaf90..fddd81b2fcecefdfed97a3827b7390f980d2cd9e 100644 (file)
--- a/src/parser.cc
+++ b/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);

diff --git a/src/scopes.cc b/src/scopes.cc
index c744dd418bbaaca2b74567048f30ecf208274da3..97fbc2144f796872edd47c1bc6419369d67718cd 100644 (file)
--- a/src/scopes.cc
+++ b/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.

diff --git a/src/scopes.h b/src/scopes.h
index 44434fa3622091d19d6b9e511e881883fb0b802f..1197df9a20e4bfa384bc016f657356b8daa2f3ae 100644 (file)
--- a/src/scopes.h
+++ b/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;
   }

diff --git a/src/variables.cc b/src/variables.cc
index dd67052663e2699667bfa92f5aadc73fc2d2487b..4f1f589ebee9617e298a354cf309ed0059df64c0 100644 (file)
--- a/src/variables.cc
+++ b/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),

diff --git a/src/variables.h b/src/variables.h
index 4696e0543d30a0d3315495de19ec7fd1ae5d3e62..31fc4a60ed21feaf251028eb1f4d7ce90fa2d928 100644 (file)
--- a/src/variables.h
+++ b/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_;