Clean up Scope::CollectUsedVariables.

src/scopeinfo.cc

Index: src/scopeinfo.cc
diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc
index 5b652e13d4d102f6c6ec99d7b0690d7b1078ff47..15ad63f8886ee5db70fc34062e906021637b7212 100644
--- a/src/scopeinfo.cc
+++ b/src/scopeinfo.cc
@@ -47,36 +47,15 @@ static int CompareLocal(Variable* const* v, Variable* const* w) {
 
 
 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;
-    }
-  }
+  const int stack_local_count = scope->StackLocalCount();
+  const int context_local_count = scope->ContextLocalCount();
+  ZoneList<Variable*> stack_locals(stack_local_count);
+  ZoneList<Variable*> context_locals(context_local_count);
+  scope->CollectStackAndContextLocals(&stack_locals, &context_locals);
+  // Make sure we allocate the correct amount.
+  ASSERT(stack_locals.length() == stack_local_count);
+  ASSERT(context_locals.length() == context_local_count);

[Kevin Millikin (Chromium), 2011/11/03 13:12:20]

Are these asserts true for the case where some variables are unused?  It seems
like nothing really depends on these invariants, if we just use
stack_locals.length() instead of stack_local_count everywhere below.

[Steven, 2011/11/03 14:55:59]

Yep. Those counts actually reflect how many times  AllocateHeapSlot /
AllocateStackSlot are called. Unused variables are not allocated so the asserts
should always hold. Will still use stack_locals.length() though.
On 2011/11/03 13:12:20, Kevin Millikin wrote:

 
   // Determine use and location of the function variable if it is present.
   FunctionVariableInfo function_name_info;
@@ -99,8 +78,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);