Fixes a few different issues related to block scope.

gen.dart

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Top level generator object for writing code and keeping track of
  * dependencies.
  *
  * Should have two compilation models, but only one implemented so far.
  *
@@ skipping 339 matching lines @@
     return factType;
   }
 }
 
 
 class BlockScope {
   MethodGenerator enclosingMethod;
   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;
 
   /**
    * True if the block is reentrant while the current method is executing.
    * This is only used for the blocks within loops.
    */
   bool reentrant;
 
   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;
 
     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;
   }
 
   Value create(String name, Type type, Node location) {
     var jsName = world.toJsIdentifier(name);
     if (_vars.containsKey(name)) {
       if (location != null) {
         world.error('duplicate name "$name"', location.span);
       } else {
         world.internalError('conflict with temporary name "$name"');
       }
     }
 
-    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;
   }
 
   /** Declares a variable in the current scope for this identifier. */
   Value declare(DeclaredIdentifier id) {
     var type = enclosingMethod.method.resolveType(id.type, false);
     return create(id.name.name, type, id);
   }
@@ skipping 1741 matching lines @@
       result.add(new Value(world.varType, '\$$i', false, /*needsTemp:*/false));
     }
     for (int i = bareCount; i < length; i++) {
       var name = getName(i);
       if (name == null) name = '\$$i';
       result.add(new Value(world.varType, name, false, /*needsTemp:*/false));
     }
     return new Arguments(nodes, result);
   }
 }