Clean up Scope::CollectUsedVariables.

src/scopeinfo.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 29 matching lines @@
 
 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;
-    }
-  }
+  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;
   VariableMode function_variable_mode;
   if (scope->is_function_scope() && scope->function() != NULL) {
     Variable* var = scope->function()->var();
     if (!var->is_used()) {
       function_name_info = UNUSED;
     } else if (var->IsContextSlot()) {
       function_name_info = CONTEXT;
     } else {
       ASSERT(var->IsStackLocal());
       function_name_info = STACK;
     }
     function_variable_mode = var->mode();
   } else {
     function_name_info = NONE;
     function_variable_mode = VAR;
   }
 
   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);
 
   Handle<ScopeInfo> scope_info = FACTORY->NewScopeInfo(length);
 
   // Encode the flags.
   int flags = TypeField::encode(scope->type()) |
       CallsEvalField::encode(scope->calls_eval()) |
       StrictModeField::encode(scope->is_strict_mode()) |
@@ skipping 416 matching lines @@
             Context::MIN_CONTEXT_SLOTS,
             ContextLocalNameEntriesIndex(),
             ContextLocalNameEntriesIndex() + ContextLocalCount(),
             this);
 
   PrintF("}\n");
 }
 #endif  // DEBUG
 
 } }  // namespace v8::internal