Don't box variables for closures that are assigned only once.

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;
@@ skipping 95 matching lines @@
   // 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;
 
   // The variables that have been declared in the current scope.
   List<Element> scopeVariables;
 
+  // Keep track of the mutated variables so that we don't need to box
+  // non-mutated variables.
+  Set<Element> mutatedVariables;
+
   FunctionElement currentFunctionElement;
   // The closureData of the currentFunctionElement.
   ClosureData closureData;
 
   bool insideClosure = false;
 
   Compiler get compiler() => builder.compiler;
 
   ClosureTranslator(SsaBuilder builder)
       : this.builder = builder,
         this.elements = builder.elements,
         capturedVariableMapping = new Map<Element, Element>(),
         closures = <FunctionExpression>[],
+        mutatedVariables = new Set<Element>(),
         this.closureDataCache = builder.builder.closureDataCache;
 
   ClosureData translate(Node node) {
     // Closures have already been analyzed when visiting the surrounding
     // method/function. This also shortcuts for bailout functions.
     ClosureData cached = closureDataCache[node];
     if (cached !== null) return cached;
 
     visit(node);
     // When variables need to be boxed their [capturedVariableMapping] is
@@ skipping 88 matching lines @@
   visitNode(Node node) => node.visitChildren(this);
 
   visitVariableDefinitions(VariableDefinitions node) {
     for (Link<Node> link = node.definitions.nodes;
          !link.isEmpty();
          link = link.tail) {
       Node definition = link.head;
       Element element = elements[definition];
       assert(element !== null);
       declareLocal(element);
+      // We still need to visit the right-hand sides of the init-assignments.
+      // For SendSets don't visit the left again. Otherwise it would be marked
+      // as mutated.
+      if (definition is SendSet) {
+        SendSet assignment = definition;
+        visit(assignment.argumentsNode);
+      } else {
+        visit(definition);
+      }
     }
-    // We still need to visit the right-hand sides of the init-assignments.
-    // Simply visit all children. We will visit the locals again and make them
-    // used, but that should not be a problem.
-    node.visitChildren(this);
   }
 
   visitIdentifier(Identifier node) {
     if (node.isThis()) {
       useLocal(closureData.thisElement);
     }
     node.visitChildren(this);
   }
 
   visitSend(Send node) {
     Element element = elements[node];
     if (Elements.isLocal(element)) {
       useLocal(element);
     } else if (node.receiver === null &&
                Elements.isInstanceSend(node, elements)) {
       useLocal(closureData.thisElement);
     } else if (node.isSuperCall) {
       useLocal(closureData.thisElement);
     }
     node.visitChildren(this);
   }
 
+  visitSendSet(SendSet node) {
+    Element element = elements[node];
+    if (Elements.isLocal(element)) {
+      mutatedVariables.add(element);
+    }
+    super.visitSendSet(node);
+  }
+
   // 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 (!mutatedVariables.contains(element)) continue;
       if (capturedVariableMapping.containsKey(element)) {
         if (box == null) {
           // TODO(floitsch): construct better box names.
           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}_${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;
     }
   }
 
-  visitLoop(Loop node) {
+  void inNewScope(Node node, Function action) {
     List<Element> oldScopeVariables = scopeVariables;
     scopeVariables = new List<Element>();
-    node.visitChildren(this);
+    action();
     attachCapturedScopeVariables(node);
+    for (Element element in scopeVariables) {
+      mutatedVariables.remove(element);
+    }
     scopeVariables = oldScopeVariables;
   }
 
+  visitLoop(Loop node) {
+    inNewScope(node, () {
+      node.visitChildren(this);
+    });
+  }
+
   visitFor(For node) {
     visitLoop(node);
     // See if we have declared loop variables that need to be boxed.
     if (node.initializer === null) return;
     VariableDefinitions definitions = node.initializer.asVariableDefinitions();
     if (definitions == null) return;
     ClosureScope scopeData = closureData.capturingScopes[node];
     if (scopeData === null) return;
     List<Element> result = <Element>[];
     for (Link<Node> link = definitions.definitions.nodes;
@@ skipping 32 matching lines @@
       // visitChildren.
       return node.name.accept(this);
     }
     bool isClosure = (closureData !== null);
 
     if (isClosure) closures.add(node);
 
     bool oldInsideClosure = insideClosure;
     FunctionElement oldFunctionElement = currentFunctionElement;
     ClosureData oldClosureData = closureData;
-    List<Element> oldScopeVariables = scopeVariables;
-
 
     insideClosure = isClosure;
     currentFunctionElement = elements[node];
     if (insideClosure) {
       closureData = globalizeClosure(node);
     } else {
       Element thisElement = null;
       // TODO(floitsch): we should not need to look for generative constructors.
       // At the moment we store only one ClosureData for both the factory and
       // 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 ThisElement(thisEnclosingElement);
       }
       closureData = new ClosureData(null, null, null, thisElement);
     }
-    scopeVariables = new List<Element>();
 
-    // 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);
-    }
+    inNewScope(node, () {
+      // 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);
+      }
 
-    // TODO(ahe): This is problematic. The backend should not repeat
-    // the work of the resolver. It is the resolver's job to create
-    // parameters, etc. Other phases should only visit statements.
-    // TODO(floitsch): we avoid visiting the initializers on purpose so that we
-    // get an error-message later in the builder.
-    if (node.parameters !== null) node.parameters.accept(this);
-    if (node.body !== null) node.body.accept(this);
-
-    attachCapturedScopeVariables(node);
+      // TODO(ahe): This is problematic. The backend should not repeat
+      // the work of the resolver. It is the resolver's job to create
+      // parameters, etc. Other phases should only visit statements.
+      // TODO(floitsch): we avoid visiting the initializers on purpose so that
+      // we get an error-message later in the builder.
+      if (node.parameters !== null) node.parameters.accept(this);
+      if (node.body !== null) node.body.accept(this);
+    });
 
     closureDataCache[node] = closureData;
 
     ClosureData savedClosureData = closureData;
     bool savedInsideClosure = insideClosure;
 
     // Restore old values.
-    scopeVariables = oldScopeVariables;
     insideClosure = oldInsideClosure;
     closureData = oldClosureData;
     currentFunctionElement = oldFunctionElement;
 
     // Mark all free variables as captured and use them in the outer function.
     List<Element> freeVariables =
         savedClosureData.freeVariableMapping.getKeys();
     assert(freeVariables.isEmpty() || savedInsideClosure);
     for (Element freeElement in freeVariables) {
       if (capturedVariableMapping[freeElement] != null &&
@@ skipping 11 matching lines @@
   }
 
   visitTryStatement(TryStatement node) {
     // TODO(ngeoffray): implement finer grain state.
     bool oldInTryStatement = inTryStatement;
     inTryStatement = true;
     node.visitChildren(this);
     inTryStatement = oldInTryStatement;
   }
 }