Reapplies (again) "Don't try to box immutable objects."

lib/compiler/implementation/ssa/closure.dart

 // Copyright (c) 2012, 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.
 
 class ClosureFieldElement extends Element {
   ClosureFieldElement(SourceString name, ClassElement enclosing)
       : super(name, ElementKind.FIELD, enclosing);
 
   bool isInstanceMember() => true;
   bool isAssignable() => false;
 
   String toString() => "ClosureFieldElement($name)";
 }
 
 class ClosureClassElement extends ClassElement {
   ClosureClassElement(Compiler compiler, Element enclosingElement)
       : super(compiler.closureClass.name, enclosingElement) {
     isResolved = true;
     compiler.closureClass.ensureResolved(compiler);
     supertype = compiler.closureClass.computeType(compiler);
   }
 }
 
+class BoxElement extends Element {
+  BoxElement(SourceString name, Element enclosingElement)
+      : super(name, ElementKind.VARIABLE, enclosingElement);
+}
+
+class ThisElement extends Element {
+  ThisElement(Element enclosing)
+      : super(const SourceString('this'), ElementKind.PARAMETER, enclosing);
+
+  bool isAssignable() => false;
+  bool isInstanceMember() => false;

[ngeoffray, 2012/04/18 08:12:54]

I don't think you need this last method here, it's already the same in the
Element class.

[floitsch, 2012/04/18 10:57:39]

Done.

+}
+
 // The box-element for a scope, and the captured variables that need to be
 // stored in the box.
 class ClosureScope {
   Element boxElement;
   Map<Element, Element> capturedVariableMapping;
   // If the scope is attached to a [For] contains the variables that are
   // declared in the initializer of the [For] and that need to be boxed.
   // Otherwise contains the empty List.
   List<Element> boxedLoopVariables;
 
   ClosureScope(this.boxElement, this.capturedVariableMapping)
       : boxedLoopVariables = const <Element>[];
 
   bool hasBoxedLoopVariables() => !boxedLoopVariables.isEmpty();
 }
 
 class ClosureData {
   // The closure's element before any translation. Will be null for methods.
   final FunctionElement closureElement;
   // The closureClassElement will be null for methods that are not local
   // closures.
   final ClassElement closureClassElement;
   // The callElement will be null for methods that are not local closures.
   final FunctionElement callElement;
   // The [thisElement] makes handling 'this' easier by treating it like any
   // other argument. It is only set for instance-members.
-  final Element thisElement;
+  final ThisElement thisElement;
 
   // Maps free locals, arguments and function elements to their captured
   // copies.
   final Map<Element, Element> freeVariableMapping;
   // Maps closure-fields to their captured elements. This is somehow the inverse
   // mapping of [freeVariableMapping], but whereas [freeVariableMapping] does
   // not deal with boxes, here we map instance-fields (which might represent
   // boxes) to their boxElement.
   final Map<Element, Element> capturedFieldMapping;
 
@@ skipping 14 matching lines @@
         this.capturedFieldMapping = new Map<Element, Element>(),
         this.capturingScopes = new Map<Node, ClosureScope>(),
         this.usedVariablesInTry = new Set<Element>();
 
   bool isClosure() => closureElement !== null;
 }
 
 class ClosureTranslator extends AbstractVisitor {
   final Compiler compiler;
   final TreeElements elements;
-  int boxCounter = 0;
+  int closureFieldCounter = 0;
   bool inTryCatchOrFinally = false;
   final Map<Node, ClosureData> closureDataCache;
 
   // Map of captured variables. Initially they will map to themselves. If
   // a variable needs to be boxed then the scope declaring the variable
   // will update this mapping.
   Map<Element, Element> capturedVariableMapping;
   // List of encountered closures.
   List<FunctionExpression> closures;
 
@@ skipping 53 matching lines @@
         } else {
           // A boxed element.
           freeVariableMapping[fromElement] = updatedElement;
           Element boxElement = updatedElement.enclosingElement;
           assert(boxElement.kind == ElementKind.VARIABLE);
           fieldCaptures.add(boxElement);
         }
       });
       ClassElement closureElement = data.closureClassElement;
       assert(closureElement != null || fieldCaptures.isEmpty());
-      for (Element boxElement in fieldCaptures) {
-        Element fieldElement =
-            new ClosureFieldElement(boxElement.name, closureElement);
+      for (Element capturedElement in fieldCaptures) {
+        SourceString name;
+        if (capturedElement is BoxElement) {
+          // The name is already mangled.
+          name = capturedElement.name;
+        } else {
+          int id = closureFieldCounter++;
+          name = new SourceString("${capturedElement.name.slowToString()}_$id");
+        }
+        Element fieldElement = new ClosureFieldElement(name, closureElement);
         closureElement.backendMembers =
             closureElement.backendMembers.prepend(fieldElement);
-        data.capturedFieldMapping[fieldElement] = boxElement;
-        freeVariableMapping[boxElement] = fieldElement;
+        data.capturedFieldMapping[fieldElement] = capturedElement;
+        freeVariableMapping[capturedElement] = fieldElement;
       }
     }
   }
 
   void useLocal(Element element) {
     // TODO(floitsch): replace this with a general solution.
     Element functionElement = currentFunctionElement;
     if (functionElement.kind === ElementKind.GENERATIVE_CONSTRUCTOR_BODY) {
       ConstructorBodyElement body = functionElement;
       functionElement = body.constructor;
@@ skipping 57 matching lines @@
   }
 
   // If variables that are declared in the [node] scope are captured and need
   // to be boxed create a box-element and update the [capturingScopes] in the
   // current [closureData].
   // The boxed variables are updated in the [capturedVariableMapping].
   void attachCapturedScopeVariables(Node node) {
     Element box = null;
     Map<Element, Element> scopeMapping = new Map<Element, Element>();
     for (Element element in scopeVariables) {
+      // No need to box non-assignable elements.
+      if (!element.isAssignable()) continue;
       if (capturedVariableMapping.containsKey(element)) {
         if (box == null) {
           // TODO(floitsch): construct better box names.
-          SourceString boxName = new SourceString("box${boxCounter++}");
-          box = new Element(boxName,
-                            ElementKind.VARIABLE,
-                            currentFunctionElement);
+          SourceString boxName =
+              new SourceString("box_${closureFieldCounter++}");
+          box = new BoxElement(boxName, currentFunctionElement);
         }
         // TODO(floitsch): construct better boxed names.
         String elementName = element.name.slowToString();
         // We are currently using the name in an HForeign which could replace
         // "$X" with something else.
         String escaped = elementName.replaceAll("\$", "_");
-        SourceString boxedName = new SourceString("${escaped}_${boxCounter++}");
+        SourceString boxedName =
+            new SourceString("${escaped}_${closureFieldCounter++}");
         Element boxed = new Element(boxedName, ElementKind.FIELD, box);
         scopeMapping[element] = boxed;
         capturedVariableMapping[element] = boxed;
       }
     }
     if (!scopeMapping.isEmpty()) {
       ClosureScope scope = new ClosureScope(box, scopeMapping);
       closureData.capturingScopes[node] = scope;
     }
   }
@@ skipping 72 matching lines @@
       // the body.
       if (element.isInstanceMember() ||
           element.kind == ElementKind.GENERATIVE_CONSTRUCTOR) {
         // TODO(floitsch): currently all variables are considered to be
         // declared in the GENERATIVE_CONSTRUCTOR. Including the 'this'.
         Element thisEnclosingElement = element;
         if (element.kind === ElementKind.GENERATIVE_CONSTRUCTOR_BODY) {
           ConstructorBodyElement body = element;
           thisEnclosingElement = body.constructor;
         }
-        thisElement = new Element(const SourceString("this"),
-                                  ElementKind.PARAMETER,
-                                  thisEnclosingElement);
+        thisElement = new ThisElement(thisEnclosingElement);
       }
       closureData = new ClosureData(null, null, null, thisElement);
     }
     scopeVariables = new List<Element>();
 
-    // TODO(floitsch): a named function is visible from inside itself. Add
-    // the element to the block.
-
     // We have to declare the implicit 'this' parameter.
     if (!insideClosure && closureData.thisElement !== null) {
       declareLocal(closureData.thisElement);
     }
     // If we are inside a named closure we have to declare ourselve. For
     // simplicity we declare the local even if the closure does not have a name
     // It will simply not be used.
     if (insideClosure) {
       declareLocal(element);
     }
@@ skipping 38 matching lines @@
     declareLocal(elements[node]);
   }
 
   visitTryStatement(TryStatement node) {
     // TODO(ngeoffray): implement finer grain state.
     inTryCatchOrFinally = true;
     node.visitChildren(this);
     inTryCatchOrFinally = false;
   }
 }