Clean up Scope::CollectUsedVariables.

Review URL: http://codereview.chromium.org/8438071

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

src/liveedit.cc

@@ -855,38 +855,20 @@ class FunctionInfoListener {
       return HEAP->undefined_value();
     }
     do {
-      ZoneList<Variable*> list(10);
-      outer_scope->CollectUsedVariables(&list);
-      int j = 0;
-      for (int i = 0; i < list.length(); i++) {
-        Variable* var1 = list[i];
-        if (var1->IsContextSlot()) {
-          if (j != i) {
-            list[j] = var1;
-          }
-          j++;
-        }
-      }
+      ZoneList<Variable*> stack_list(outer_scope->StackLocalCount());
+      ZoneList<Variable*> context_list(outer_scope->ContextLocalCount());
+      outer_scope->CollectStackAndContextLocals(&stack_list, &context_list);
+      context_list.Sort(&Variable::CompareIndex);
 
-      // Sort it.
-      for (int k = 1; k < j; k++) {
-        int l = k;
-        for (int m = k + 1; m < j; m++) {
-          if (list[l]->index() > list[m]->index()) {
-            l = m;
-          }
-        }
-        list[k] = list[l];
-      }
-      for (int i = 0; i < j; i++) {
+      for (int i = 0; i < context_list.length(); i++) {
         SetElementNonStrict(scope_info_list,
                             scope_info_length,
-                            list[i]->name());
+                            context_list[i]->name());
         scope_info_length++;
         SetElementNonStrict(
             scope_info_list,
             scope_info_length,
-            Handle<Smi>(Smi::FromInt(list[i]->index())));
+            Handle<Smi>(Smi::FromInt(context_list[i]->index())));
         scope_info_length++;
       }
       SetElementNonStrict(scope_info_list,

src/scopeinfo.cc

@@ -38,45 +38,16 @@ namespace v8 {
 namespace internal {
 
 
-static int CompareLocal(Variable* const* v, Variable* const* w) {
-  int x = (*v)->index();
-  int y = (*w)->index();
-  // Consider sorting them according to type as well?
-  return x - y;
-}
-
-
 Handle<ScopeInfo> ScopeInfo::Create(Scope* scope) {
-  ZoneList<Variable*> variables(32);  // 32 is a wild guess
-  ASSERT(variables.is_empty());
-  scope->CollectUsedVariables(&variables);
-
-  ZoneList<Variable*> stack_locals(scope->num_stack_slots());
-  ZoneList<Variable*> context_locals(scope->num_heap_slots());
-
   // Collect stack and context locals.
-  for (int i = 0; i < variables.length(); i++) {
-    Variable* var = variables[i];
-    ASSERT(var->is_used());
-    switch (var->location()) {
-      case Variable::UNALLOCATED:
-      case Variable::PARAMETER:
-        break;
-
-      case Variable::LOCAL:
-        stack_locals.Add(var);
-        break;
-
-      case Variable::CONTEXT:
-        context_locals.Add(var);
-        break;
-
-      case Variable::LOOKUP:
-        // We don't expect lookup variables in the locals list.
-        UNREACHABLE();
-        break;
-    }
-  }
+  ZoneList<Variable*> stack_locals(scope->StackLocalCount());
+  ZoneList<Variable*> context_locals(scope->ContextLocalCount());
+  scope->CollectStackAndContextLocals(&stack_locals, &context_locals);
+  const int stack_local_count = stack_locals.length();
+  const int context_local_count = context_locals.length();
+  // Make sure we allocate the correct amount.
+  ASSERT(scope->StackLocalCount() == stack_local_count);
+  ASSERT(scope->ContextLocalCount() == context_local_count);
 
   // Determine use and location of the function variable if it is present.
   FunctionVariableInfo function_name_info;
@@ -99,8 +70,6 @@ Handle<ScopeInfo> ScopeInfo::Create(Scope* scope) {
 
   const bool has_function_name = function_name_info != NONE;
   const int parameter_count = scope->num_parameters();
-  const int stack_local_count = stack_locals.length();
-  const int context_local_count = context_locals.length();
   const int length = kVariablePartIndex
       + parameter_count + stack_local_count + 2 * context_local_count
       + (has_function_name ? 2 : 0);
@@ -140,7 +109,7 @@ Handle<ScopeInfo> ScopeInfo::Create(Scope* scope) {
   // according to usage, the allocated slot indices may not be in increasing
   // order with the variable list anymore. Thus, we first need to sort them by
   // context slot index before adding them to the ScopeInfo object.
-  context_locals.Sort(&CompareLocal);
+  context_locals.Sort(&Variable::CompareIndex);
 
   // Add context locals' names.
   ASSERT(index == scope_info->ContextLocalNameEntriesIndex());

src/scopes.cc

@@ -529,23 +529,31 @@ Declaration* Scope::CheckConflictingVarDeclarations() {
 }
 
 
-void Scope::CollectUsedVariables(ZoneList<Variable*>* locals) {
-  // Collect variables in this scope.
-  // Note that the function_ variable - if present - is not
-  // collected here but handled separately in ScopeInfo
-  // which is the current user of this function).
+void Scope::CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
+                                         ZoneList<Variable*>* context_locals) {
+  ASSERT(stack_locals != NULL);
+  ASSERT(context_locals != NULL);
+
+  // Collect temporaries which are always allocated on the stack.
   for (int i = 0; i < temps_.length(); i++) {
     Variable* var = temps_[i];
     if (var->is_used()) {
-      locals->Add(var);
+      ASSERT(var->IsStackLocal());
+      stack_locals->Add(var);
     }
   }
+
+  // Collect declared local variables.
   for (VariableMap::Entry* p = variables_.Start();
        p != NULL;
        p = variables_.Next(p)) {
     Variable* var = reinterpret_cast<Variable*>(p->value);
     if (var->is_used()) {
-      locals->Add(var);
+      if (var->IsStackLocal()) {
+        stack_locals->Add(var);
+      } else if (var->IsContextSlot()) {
+        context_locals->Add(var);
+      }
     }
   }
 }
@@ -1165,4 +1173,17 @@ void Scope::AllocateVariablesRecursively() {
   ASSERT(num_heap_slots_ == 0 || num_heap_slots_ >= Context::MIN_CONTEXT_SLOTS);
 }
 
+
+int Scope::StackLocalCount() const {
+  return num_stack_slots() -
+      (function_ != NULL && function_->var()->IsStackLocal() ? 1 : 0);
+}
+
+
+int Scope::ContextLocalCount() const {
+  if (num_heap_slots() == 0) return 0;
+  return num_heap_slots() - Context::MIN_CONTEXT_SLOTS -
+      (function_ != NULL && function_->var()->IsContextSlot() ? 1 : 0);
+}
+
 } }  // namespace v8::internal

src/scopes.h

@@ -303,8 +303,11 @@ class Scope: public ZoneObject {
   // ---------------------------------------------------------------------------
   // Variable allocation.
 
-  // Collect all used locals in this scope.
-  void CollectUsedVariables(ZoneList<Variable*>* locals);
+  // Collect stack and context allocated local variables in this scope. Note
+  // that the function variable - if present - is not collected and should be
+  // handled separately.
+  void CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
+                                    ZoneList<Variable*>* context_locals);
 
   // Resolve and fill in the allocation information for all variables
   // in this scopes. Must be called *after* all scopes have been
@@ -323,6 +326,9 @@ class Scope: public ZoneObject {
   int num_stack_slots() const { return num_stack_slots_; }
   int num_heap_slots() const { return num_heap_slots_; }
 
+  int StackLocalCount() const;
+  int ContextLocalCount() const;
+
   // Make sure this scope and all outer scopes are eagerly compiled.
   void ForceEagerCompilation()  { force_eager_compilation_ = true; }
 

src/variables.cc

@@ -85,4 +85,12 @@ bool Variable::is_global() const {
   return mode_ != TEMPORARY && scope_ != NULL && scope_->is_global_scope();
 }
 
+
+int Variable::CompareIndex(Variable* const* v, Variable* const* w) {
+  int x = (*v)->index();
+  int y = (*w)->index();
+  // Consider sorting them according to type as well?
+  return x - y;
+}
+
 } }  // namespace v8::internal

src/variables.h

@@ -159,6 +159,8 @@ class Variable: public ZoneObject {
     index_ = index;
   }
 
+  static int CompareIndex(Variable* const* v, Variable* const* w);
+
  private:
   Scope* scope_;
   Handle<String> name_;