Allow variable proxies for the same global variable to share the same

variable object.

Add a map from names to variables for global scopes just like
non-global scopes.  Variables are added to the map by the parser when
it encounters a declaration in a global scope or else at scope
resolution time by a failed variable lookup from the global scope or
an inner one and with no intervening with statements or possible calls
to eval.
Review URL: http://codereview.chromium.org/149245

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2369 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/parser.cc

@@ -1576,10 +1576,10 @@ VariableProxy* AstBuildingParser::Declare(Handle<String> name,
   // to the calling function context.
   if (top_scope_->is_function_scope()) {
     // Declare the variable in the function scope.
-    var = top_scope_->LookupLocal(name);
+    var = top_scope_->LocalLookup(name);
     if (var == NULL) {
       // Declare the name.
-      var = top_scope_->Declare(name, mode);
+      var = top_scope_->DeclareLocal(name, mode);
     } else {
       // The name was declared before; check for conflicting
       // re-declarations. If the previous declaration was a const or the
@@ -3466,8 +3466,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
     while (!done) {
       Handle<String> param_name = ParseIdentifier(CHECK_OK);
       if (!is_pre_parsing_) {
-        top_scope_->AddParameter(top_scope_->Declare(param_name,
+        top_scope_->AddParameter(top_scope_->DeclareLocal(param_name,
-                                                     Variable::VAR));
+                                                          Variable::VAR));
         num_parameters++;
       }
       done = (peek() == Token::RPAREN);

src/scopes.cc

@@ -71,28 +71,28 @@ static bool Match(void* key1, void* key2) {
 
 
 // Dummy constructor
-LocalsMap::LocalsMap(bool gotta_love_static_overloading) : HashMap()  {}
+VariableMap::VariableMap(bool gotta_love_static_overloading) : HashMap() {}
 
-LocalsMap::LocalsMap() : HashMap(Match, &LocalsMapAllocator, 8)  {}
+VariableMap::VariableMap() : HashMap(Match, &LocalsMapAllocator, 8) {}
-LocalsMap::~LocalsMap()  {}
+VariableMap::~VariableMap() {}
 
 
-Variable* LocalsMap::Declare(Scope* scope,
+Variable* VariableMap::Declare(Scope* scope,
-                             Handle<String> name,
+                               Handle<String> name,
-                             Variable::Mode mode,
+                               Variable::Mode mode,
-                             bool is_valid_LHS,
+                               bool is_valid_lhs,
-                             Variable::Kind kind) {
+                               Variable::Kind kind) {
   HashMap::Entry* p = HashMap::Lookup(name.location(), name->Hash(), true);
   if (p->value == NULL) {
     // The variable has not been declared yet -> insert it.
     ASSERT(p->key == name.location());
-    p->value = new Variable(scope, name, mode, is_valid_LHS, kind);
+    p->value = new Variable(scope, name, mode, is_valid_lhs, kind);
   }
   return reinterpret_cast<Variable*>(p->value);
 }
 
 
-Variable* LocalsMap::Lookup(Handle<String> name) {
+Variable* VariableMap::Lookup(Handle<String> name) {
   HashMap::Entry* p = HashMap::Lookup(name.location(), name->Hash(), false);
   if (p != NULL) {
     ASSERT(*reinterpret_cast<String**>(p->key) == *name);
@@ -110,7 +110,7 @@ Variable* LocalsMap::Lookup(Handle<String> name) {
 // Dummy constructor
 Scope::Scope()
   : inner_scopes_(0),
-    locals_(false),
+    variables_(false),
     temps_(0),
     params_(0),
     dynamics_(NULL),
@@ -168,27 +168,26 @@ void Scope::Initialize(bool inside_with) {
   // instead load them directly from the stack. Currently, the only
   // such parameter is 'this' which is passed on the stack when
   // invoking scripts
-  { Variable* var =
+  Variable* var =
-        locals_.Declare(this, Factory::this_symbol(), Variable::VAR,
+      variables_.Declare(this, Factory::this_symbol(), Variable::VAR,
-                        false, Variable::THIS);
+                         false, Variable::THIS);
-    var->rewrite_ = new Slot(var, Slot::PARAMETER, -1);
+  var->rewrite_ = new Slot(var, Slot::PARAMETER, -1);
-    receiver_ = new VariableProxy(Factory::this_symbol(), true, false);
+  receiver_ = new VariableProxy(Factory::this_symbol(), true, false);
-    receiver_->BindTo(var);
+  receiver_->BindTo(var);
-  }
 
   if (is_function_scope()) {
     // Declare 'arguments' variable which exists in all functions.
-    // Note that it may never be accessed, in which case it won't
+    // Note that it might never be accessed, in which case it won't be
-    // be allocated during variable allocation.
+    // allocated during variable allocation.
-    locals_.Declare(this, Factory::arguments_symbol(), Variable::VAR,
+    variables_.Declare(this, Factory::arguments_symbol(), Variable::VAR,
-                    true, Variable::ARGUMENTS);
+                       true, Variable::ARGUMENTS);
   }
 }
 
 
 
-Variable* Scope::LookupLocal(Handle<String> name) {
+Variable* Scope::LocalLookup(Handle<String> name) {
-  return locals_.Lookup(name);
+  return variables_.Lookup(name);
 }
 
 
@@ -196,7 +195,7 @@ Variable* Scope::Lookup(Handle<String> name) {
   for (Scope* scope = this;
        scope != NULL;
        scope = scope->outer_scope()) {
-    Variable* var = scope->LookupLocal(name);
+    Variable* var = scope->LocalLookup(name);
     if (var != NULL) return var;
   }
   return NULL;
@@ -210,18 +209,25 @@ Variable* Scope::DeclareFunctionVar(Handle<String> name) {
 }
 
 
-Variable* Scope::Declare(Handle<String> name, Variable::Mode mode) {
+Variable* Scope::DeclareLocal(Handle<String> name, Variable::Mode mode) {
   // DYNAMIC variables are introduces during variable allocation,
   // INTERNAL variables are allocated explicitly, and TEMPORARY
   // variables are allocated via NewTemporary().
   ASSERT(mode == Variable::VAR || mode == Variable::CONST);
-  return locals_.Declare(this, name, mode, true, Variable::NORMAL);
+  return variables_.Declare(this, name, mode, true, Variable::NORMAL);
+}
+
+
+Variable* Scope::DeclareGlobal(Handle<String> name) {
+  ASSERT(is_global_scope());
+  return variables_.Declare(this, name, Variable::DYNAMIC, true,
+                            Variable::NORMAL);
 }
 
 
 void Scope::AddParameter(Variable* var) {
   ASSERT(is_function_scope());
-  ASSERT(LookupLocal(var->name()) == var);
+  ASSERT(LocalLookup(var->name()) == var);
   params_.Add(var);
 }
 
@@ -291,7 +297,9 @@ void Scope::CollectUsedVariables(List<Variable*, Allocator>* locals) {
       locals->Add(var);
     }
   }
-  for (LocalsMap::Entry* p = locals_.Start(); p != NULL; p = locals_.Next(p)) {
+  for (VariableMap::Entry* p = variables_.Start();
+       p != NULL;
+       p = variables_.Next(p)) {
     Variable* var = reinterpret_cast<Variable*>(p->value);
     if (var->var_uses()->is_used()) {
       locals->Add(var);
@@ -410,8 +418,8 @@ static void PrintVar(PrettyPrinter* printer, int indent, Variable* var) {
 }
 
 
-static void PrintMap(PrettyPrinter* printer, int indent, LocalsMap* map) {
+static void PrintMap(PrettyPrinter* printer, int indent, VariableMap* map) {
-  for (LocalsMap::Entry* p = map->Start(); p != NULL; p = map->Next(p)) {
+  for (VariableMap::Entry* p = map->Start(); p != NULL; p = map->Next(p)) {
     Variable* var = reinterpret_cast<Variable*>(p->value);
     PrintVar(printer, indent, var);
   }
@@ -478,7 +486,7 @@ void Scope::Print(int n) {
   }
 
   Indent(n1, "// local vars\n");
-  PrintMap(&printer, n1, &locals_);
+  PrintMap(&printer, n1, &variables_);
 
   Indent(n1, "// dynamic vars\n");
   if (dynamics_ != NULL) {
@@ -502,7 +510,7 @@ void Scope::Print(int n) {
 
 Variable* Scope::NonLocal(Handle<String> name, Variable::Mode mode) {
   if (dynamics_ == NULL) dynamics_ = new DynamicScopePart();
-  LocalsMap* map = dynamics_->GetMap(mode);
+  VariableMap* map = dynamics_->GetMap(mode);
   Variable* var = map->Lookup(name);
   if (var == NULL) {
     // Declare a new non-local.
@@ -530,7 +538,7 @@ Variable* Scope::LookupRecursive(Handle<String> name,
   bool guess = scope_calls_eval_;
 
   // Try to find the variable in this scope.
-  Variable* var = LookupLocal(name);
+  Variable* var = LocalLookup(name);
 
   if (var != NULL) {
     // We found a variable. If this is not an inner lookup, we are done.
@@ -621,8 +629,7 @@ void Scope::ResolveVariable(Scope* global_scope,
             scope_calls_eval_ || outer_scope_calls_eval_)) {
         // We must have a global variable.
         ASSERT(global_scope != NULL);
-        var = new Variable(global_scope, proxy->name(),
+        var = global_scope->DeclareGlobal(proxy->name());
-                           Variable::DYNAMIC, true, Variable::NORMAL);
 
       } else if (scope_inside_with_) {
         // If we are inside a with statement we give up and look up
@@ -706,26 +713,26 @@ bool Scope::PropagateScopeInfo(bool outer_scope_calls_eval,
 
 
 bool Scope::MustAllocate(Variable* var) {
-  // Give var a read/write use if there is a chance it might be
+  // Give var a read/write use if there is a chance it might be accessed
-  // accessed via an eval() call, or if it is a global variable.
+  // via an eval() call.  This is only possible if the variable has a
-  // This is only possible if the variable has a visible name.
+  // visible name.
   if ((var->is_this() || var->name()->length() > 0) &&
       (var->is_accessed_from_inner_scope_ ||
        scope_calls_eval_ || inner_scope_calls_eval_ ||
-       scope_contains_with_ || var->is_global())) {
+       scope_contains_with_)) {
     var->var_uses()->RecordAccess(1);
   }
-  return var->var_uses()->is_used();
+  // Global variables do not need to be allocated.
+  return !var->is_global() && var->var_uses()->is_used();
 }
 
 
 bool Scope::MustAllocateInContext(Variable* var) {
   // If var is accessed from an inner scope, or if there is a
-  // possibility that it might be accessed from the current or
+  // possibility that it might be accessed from the current or an inner
-  // an inner scope (through an eval() call), it must be allocated
+  // scope (through an eval() call), it must be allocated in the
-  // in the context.
+  // context.  Exception: temporary variables are not allocated in the
-  // Exceptions: Global variables and temporary variables must
+  // context.
-  // never be allocated in the (FixedArray part of the) context.
   return
     var->mode() != Variable::TEMPORARY &&
     (var->is_accessed_from_inner_scope_ ||
@@ -755,7 +762,7 @@ void Scope::AllocateHeapSlot(Variable* var) {
 
 void Scope::AllocateParameterLocals() {
   ASSERT(is_function_scope());
-  Variable* arguments = LookupLocal(Factory::arguments_symbol());
+  Variable* arguments = LocalLookup(Factory::arguments_symbol());
   ASSERT(arguments != NULL);  // functions have 'arguments' declared implicitly
   if (MustAllocate(arguments) && !HasArgumentsParameter()) {
     // 'arguments' is used. Unless there is also a parameter called
@@ -865,7 +872,7 @@ void Scope::AllocateNonParameterLocal(Variable* var) {
   ASSERT(var->rewrite_ == NULL ||
          (!var->IsVariable(Factory::result_symbol())) ||
          (var->slot() == NULL || var->slot()->type() != Slot::LOCAL));
-  if (MustAllocate(var) && var->rewrite_ == NULL) {
+  if (var->rewrite_ == NULL && MustAllocate(var)) {
     if (MustAllocateInContext(var)) {
       AllocateHeapSlot(var);
     } else {
@@ -876,27 +883,21 @@ void Scope::AllocateNonParameterLocal(Variable* var) {
 
 
 void Scope::AllocateNonParameterLocals() {
-  // Each variable occurs exactly once in the locals_ list; all
+  // All variables that have no rewrite yet are non-parameter locals.
-  // variables that have no rewrite yet are non-parameter locals.
-
-  // Sort them according to use such that the locals with more uses
-  // get allocated first.
-  if (FLAG_usage_computation) {
-    // This is currently not implemented.
-  }
-
   for (int i = 0; i < temps_.length(); i++) {
     AllocateNonParameterLocal(temps_[i]);
   }
 
-  for (LocalsMap::Entry* p = locals_.Start(); p != NULL; p = locals_.Next(p)) {
+  for (VariableMap::Entry* p = variables_.Start();
+       p != NULL;
+       p = variables_.Next(p)) {
     Variable* var = reinterpret_cast<Variable*>(p->value);
     AllocateNonParameterLocal(var);
   }
 
-  // Note: For now, function_ must be allocated at the very end.  If
+  // For now, function_ must be allocated at the very end.  If it gets
-  // it gets allocated in the context, it must be the last slot in the
+  // allocated in the context, it must be the last slot in the context,
-  // context, because of the current ScopeInfo implementation (see
+  // because of the current ScopeInfo implementation (see
   // ScopeInfo::ScopeInfo(FunctionScope* scope) constructor).
   if (function_ != NULL) {
     AllocateNonParameterLocal(function_);

src/scopes.h

@@ -35,19 +35,22 @@ namespace v8 {
 namespace internal {
 
 
-// A hash map to support fast local variable declaration and lookup.
+// A hash map to support fast variable declaration and lookup.
-class LocalsMap: public HashMap {
+class VariableMap: public HashMap {
  public:
-  LocalsMap();
+  VariableMap();
 
   // Dummy constructor.  This constructor doesn't set up the map
   // properly so don't use it unless you have a good reason.
-  explicit LocalsMap(bool gotta_love_static_overloading);
+  explicit VariableMap(bool gotta_love_static_overloading);
 
-  virtual ~LocalsMap();
+  virtual ~VariableMap();
 
-  Variable* Declare(Scope* scope, Handle<String> name, Variable::Mode mode,
+  Variable* Declare(Scope* scope,
-                    bool is_valid_LHS, Variable::Kind kind);
+                    Handle<String> name,
+                    Variable::Mode mode,
+                    bool is_valid_lhs,
+                    Variable::Kind kind);
 
   Variable* Lookup(Handle<String> name);
 };
@@ -59,14 +62,14 @@ class LocalsMap: public HashMap {
 // and setup time for scopes that don't need them.
 class DynamicScopePart : public ZoneObject {
  public:
-  LocalsMap* GetMap(Variable::Mode mode) {
+  VariableMap* GetMap(Variable::Mode mode) {
     int index = mode - Variable::DYNAMIC;
     ASSERT(index >= 0 && index < 3);
     return &maps_[index];
   }
 
  private:
-  LocalsMap maps_[3];
+  VariableMap maps_[3];
 };
 
 
@@ -105,7 +108,7 @@ class Scope: public ZoneObject {
   // Declarations
 
   // Lookup a variable in this scope. Returns the variable or NULL if not found.
-  virtual Variable* LookupLocal(Handle<String> name);
+  virtual Variable* LocalLookup(Handle<String> name);
 
   // Lookup a variable in this scope or outer scopes.
   // Returns the variable or NULL if not found.
@@ -116,9 +119,15 @@ class Scope: public ZoneObject {
   // outer scope. Only possible for function scopes; at most one variable.
   Variable* DeclareFunctionVar(Handle<String> name);
 
-  // Declare a variable in this scope. If the variable has been
+  // Declare a local variable in this scope. If the variable has been
   // declared before, the previously declared variable is returned.
-  virtual Variable* Declare(Handle<String> name, Variable::Mode mode);
+  virtual Variable* DeclareLocal(Handle<String> name, Variable::Mode mode);
+
+  // Declare an implicit global variable in this scope which must be a
+  // global scope.  The variable was introduced (possibly from an inner
+  // scope) by a reference to an unresolved variable with no intervening
+  // with statements or eval calls.
+  Variable* DeclareGlobal(Handle<String> name);
 
   // Add a parameter to the parameter list. The parameter must have been
   // declared via Declare. The same parameter may occur more then once in
@@ -288,25 +297,28 @@ class Scope: public ZoneObject {
   Handle<String> scope_name_;
 
   // The variables declared in this scope:
-  // all user-declared variables (incl. parameters)
+  //
-  LocalsMap locals_;
+  // All user-declared variables (incl. parameters).  For global scopes
-  // compiler-allocated (user-invisible) temporaries
+  // variables may be implicitly 'declared' by being used (possibly in
+  // an inner scope) with no intervening with statements or eval calls.
+  VariableMap variables_;
+  // Compiler-allocated (user-invisible) temporaries.
   ZoneList<Variable*> temps_;
-  // parameter list in source order
+  // Parameter list in source order.
   ZoneList<Variable*> params_;
-  // variables that must be looked up dynamically
+  // Variables that must be looked up dynamically.
   DynamicScopePart* dynamics_;
-  // unresolved variables referred to from this scope
+  // Unresolved variables referred to from this scope.
   ZoneList<VariableProxy*> unresolved_;
-  // declarations
+  // Declarations.
   ZoneList<Declaration*> decls_;
-  // convenience variable
+  // Convenience variable.
   VariableProxy* receiver_;
-  // function variable, if any; function scopes only
+  // Function variable, if any; function scopes only.
   Variable* function_;
-  // convenience variable; function scopes only
+  // Convenience variable; function scopes only.
   VariableProxy* arguments_;
-  // convenience variable; function scopes only
+  // Convenience variable; function scopes only.
   VariableProxy* arguments_shadow_;
 
   // Illegal redeclaration.

src/variables.h

@@ -143,6 +143,12 @@ class Variable: public ZoneObject {
     ARGUMENTS
   };
 
+  Variable(Scope* scope,
+           Handle<String> name,
+           Mode mode,
+           bool is_valid_lhs,
+           Kind kind);
+
   // Printing support
   static const char* Mode2String(Mode mode);
 
@@ -196,9 +202,6 @@ class Variable: public ZoneObject {
   SmiAnalysis* type() { return &type_; }
 
  private:
-  Variable(Scope* scope, Handle<String> name, Mode mode, bool is_valid_LHS,
-           Kind kind);
-
   Scope* scope_;
   Handle<String> name_;
   Mode mode_;
@@ -216,13 +219,10 @@ class Variable: public ZoneObject {
   SmiAnalysis type_;
 
   // Code generation.
-  // rewrite_ is usually a Slot or a Property, but maybe any expression.
+  // rewrite_ is usually a Slot or a Property, but may be any expression.
   Expression* rewrite_;
 
-  friend class VariableProxy;
+  friend class Scope;  // Has explicit access to rewrite_.
-  friend class Scope;
-  friend class LocalsMap;
-  friend class AstBuildingParser;
 };