Clean up Scope::CollectUsedVariables.

src/liveedit.cc

Index: src/liveedit.cc
diff --git a/src/liveedit.cc b/src/liveedit.cc
index ce903d1f781cb4e7d52b09fb51f5fe495955e51e..80fa99f4d1d499f5688058d6020bab61c91d8e8d 100644
--- a/src/liveedit.cc
+++ b/src/liveedit.cc
@@ -855,38 +855,29 @@ 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);
 
       // Sort it.
-      for (int k = 1; k < j; k++) {
+      for (int k = 1; k < context_list.length(); k++) {

[Steven, 2011/11/03 12:39:39]

Why does the iteration start at 1 here? Is this maybe a bug?

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

It does look like a bug.  I'm not even sure whether it's a one or an L.  In any
case, it's more obviously correct to have:

context_list.Sort(&CompareLocal)

and expose CompareLocal from scopes.cc (I'd put CompareLocal in in
variables.{h,cc} if it were me, and I'd rename it to CompareIndex or something).

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

Done.

         int l = k;
-        for (int m = k + 1; m < j; m++) {
-          if (list[l]->index() > list[m]->index()) {
+        for (int m = k + 1; m < context_list.length(); m++) {
+          if (context_list[l]->index() > context_list[m]->index()) {
             l = m;
           }
         }
-        list[k] = list[l];
+        context_list[k] = context_list[l];

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

Honestly this whole code looks like a giant bug to me. This is sort of supposed
to be some kind of selection sort, but here one entry is copied over another one
instead of being swapped or so.

       }
-      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,