Fixes a few different issues related to block scope.

gen.dart

Index: gen.dart
diff --git a/gen.dart b/gen.dart
index 3e4983982ad60144c3b44954032142e473ccf67f..97ad438d86bf4ff241d373641eea8b6ce0bc34c7 100644
--- a/gen.dart
+++ b/gen.dart
@@ -357,6 +357,12 @@ class BlockScope {
   BlockScope parent;
   Map<String, Value> _vars; // TODO(jimhug): Using a list may improve perf.
 
+  /**
+   * Variables in this method that have been captured by lambdas.
+   * Don't reuse the names in child blocks.
+   */
+  Set<String> _closedOver;
+
   /** If we are in a catch block, this is the exception variable to rethrow. */
   Value rethrow;
 
@@ -369,12 +375,28 @@ class BlockScope {
   BlockScope(this.enclosingMethod, this.parent, [this.reentrant = false])
     : _vars = {} {
 
-    // Blocks within a reentrant block are also reentrant.
-    if (parent != null && parent.enclosingMethod == enclosingMethod) {
+    if (isMethodScope) {
+      _closedOver = new Set<String>();
+    } else {
+      // Blocks within a reentrant block are also reentrant.
       reentrant = reentrant || parent.reentrant;
     }
   }
 
+  /** True if this is the top level scope of the method. */
+  bool get isMethodScope() {
+    return parent == null || parent.enclosingMethod != enclosingMethod;
+  }
+
+  /**
+   * Gets the method scope associated with this block scope (possibly itself).
+   */
+  BlockScope get methodScope() {
+    var s = this;
+    while (!s.isMethodScope) s = s.parent;
+    return s;
+  }
+
   lookup(String name) {
     var ret = _vars[name];
     if (ret != null) return ret;
@@ -382,22 +404,47 @@ class BlockScope {
     for (var s = parent; s != null; s = s.parent) {
       ret = s._vars[name];
       if (ret != null) {
-        // If we're in a lambda and this variable is from a different method
-        // and the scope we found this variable in is reentrant, put this
-        // variable in the list of variables that we're going to capture
-        // with Function.bind
-        if (enclosingMethod.captures != null && s.reentrant &&
-            s.enclosingMethod != enclosingMethod) {
-          enclosingMethod.captures.add(name);
+        // If this variable is from a different method, it means we closed over
+        // it in the child lambda. Time for some bookeeping!
+        if (s.enclosingMethod != enclosingMethod) {
+          // Make sure the parent method doesn't reuse this variable to mean
+          // something else.
+          s.methodScope._closedOver.add(ret.code);
+
+          // If the scope we found this variable in is reentrant, remember the
+          // variable. The lambda we're in will capture it with Function.bind.
+          if (enclosingMethod.captures != null && s.reentrant) {
+            enclosingMethod.captures.add(ret.code);
+          }
         }
+
         return ret;
       }
     }
   }
 
-  bool _isDefined(String name) {
-    if (_vars.containsKey(name)) return true;
-    if (parent != null) return parent._isDefined(name);
+  /**
+   * Returns true if we can't use this name because we would be shadowing
+   * another name in the JS that we might need to access later.
+   */
+  bool _isDefinedInParent(String name) {
+    if (isMethodScope && _closedOver.contains(name)) return true;
+
+    for (var s = parent; s != null; s = s.parent) {
+      if (s._vars.containsKey(name)) return true;
+      // Don't reuse a name that's been closed over
+      if (s.isMethodScope && s._closedOver.contains(name)) return true;
+    }
+
+    // Ensure that we don't shadow another name from the global scope.
+    final type = enclosingMethod.method.declaringType;
+    if (type.resolveMember(name) != null) return true;
+
+    // This lookup might report errors, which is a bit strange.
+    // But probably harmless since we have to pay for the lookup anyway.
+    if (type.library.lookup(name, null) != null) return true;
+
+    // Nobody else needs this name. It's safe to reuse.
     return false;
   }
 
@@ -411,17 +458,11 @@ class BlockScope {
       }
     }
 
-    if (parent != null) {
-      int index = 0;
-      // TODO(jmesserly): we also need to check that no scope in the method

[Jennifer Messerly, 2011/11/02 03:23:19]

this TODO is one of the issues addressed

-      // declares the same name, if we want closures to work right.
-      // In other words, if another sibling block in the method declares a
-      // variable with the same name, it shouldn't be able to mutate the closed
-      //over value.
-      while (parent._isDefined(jsName)) {
-        jsName = '$name${index++}';
-      }
+    int index = 0;
+    while (_isDefinedInParent(jsName)) {
+      jsName = '$name${index++}';
     }
+
     var ret = new Value(type, jsName, false, false); // TODO: needsTemp:false);
     _vars[name] = ret;
     return ret;