Overhaul tree IR visitor and rename IR classes.

pkg/compiler/lib/src/tree_ir/optimization/variable_merger.dart

 // Copyright (c) 2015, 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.
 
 library tree_ir.optimization.variable_merger;
 
 import 'optimization.dart' show Pass, PassMixin;
 import '../tree_ir_nodes.dart';
 import '../../elements/elements.dart' show Local, ParameterElement;
 
 /// Merges variables based on liveness and source variable information.
 ///
 /// This phase cleans up artifacts introduced by the translation through CPS,
 /// where each source variable is translated into several copies. The copies
 /// are merged again when they are not live simultaneously.
-class VariableMerger extends RecursiveVisitor with PassMixin {
+class VariableMerger extends RecursiveVisitor implements Pass {
   String get passName => 'Variable merger';
 
+  void rewrite(RootNode node) {
+    rewriteFunction(node);
+    node.forEachBody(visitStatement);
+  }
+
   @override
-  void rewriteExecutableDefinition(ExecutableDefinition node) {
-    visitExecutableDefinition(node);
+  void visitInnerFunction(FunctionDefinition node) {
+    rewriteFunction(node);
   }
 
   /// Rewrites the given function.
   /// This is called for the outermost function and inner functions.
-  void rewriteFunction(ExecutableDefinition node) {
-    BlockGraphBuilder builder = new BlockGraphBuilder();
-    builder.visitExecutableDefinition(node);
-    _computeLiveness(builder.blocks);
-    Map<Variable, Variable> subst = _computeRegisterAllocation(builder.blocks);
-    new SubstituteVariables(subst).visitExecutableDefinition(node);
-  }
-
-  visitFunctionDefinition(FunctionDefinition node) {
-    super.visitFunctionDefinition(node); // Recurse to visit inner functions.
-    rewriteFunction(node);
-  }
-
-  visitFieldDefinition(FieldDefinition node) {
-    super.visitFieldDefinition(node);
-    rewriteFunction(node);
-  }
-
-  visitConstructorDefinition(ConstructorDefinition node) {
-    super.visitConstructorDefinition(node);
-    rewriteFunction(node);
+  void rewriteFunction(RootNode node) {
+    node.forEachBody((Statement body) {
+      BlockGraphBuilder builder = new BlockGraphBuilder();
+      builder.build(node.parameters, body);
+      _computeLiveness(builder.blocks);
+      Map<Variable, Variable> subst =
+          _computeRegisterAllocation(builder.blocks);
+      new SubstituteVariables(subst).apply(node);
+    });
   }
 }
 
 /// A read or write access to a variable.
 class VariableAccess {
   Variable variable;
   bool isRead;
   bool get isWrite => !isRead;
 
   VariableAccess.read(this.variable) : isRead = true;
@@ skipping 52 matching lines @@
   Map<Label, Block> _jumpTarget = <Label, Block>{};
   Block _currentBlock;
   List<Block> blocks = <Block>[];
 
   /// Variables with an assignment that should be treated as final.
   ///
   /// Such variables cannot be merged with any other variables, so we exclude
   /// them from the control-flow graph entirely.
   Set<Variable> _ignoredVariables = new Set<Variable>();
 
-  BlockGraphBuilder() {
+  void build(List<Variable> parameters, Statement body) {
     _currentBlock = newBlock();
+    parameters.forEach(write);
+    visitStatement(body);
+  }
+
+  @override
+  void visitInnerFunction(FunctionDefinition node) {
+    // Do nothing. Inner functions are traversed in VariableMerger.
   }
 
   /// Creates a new block with the current exception handler or [catchBlock]
   /// if provided.
   Block newBlock({Block catchBlock}) {
     if (catchBlock == null && _currentBlock != null) {
       catchBlock = _currentBlock.catchBlock;
     }
     Block block = new Block(catchBlock);
     blocks.add(block);
@@ skipping 106 matching lines @@
     visitExpression(node.left);
     // TODO(asgerf): When assignment expressions are added, this is no longer
     // sound; then we need to handle as a branch.
     visitExpression(node.right);
   }
 
   visitFunctionDeclaration(FunctionDeclaration node) {
     // The function variable is final, hence cannot be merged.
     ignoreVariable(node.variable);
     visitStatement(node.next);
-    // Do not traverse inner function.
-  }
-
-  visitFunctionExpression(FunctionExpression node) {
-    // Do not traverse inner function.
-  }
-
-  visitFunctionDefinition(FunctionDefinition node) {
-    // Function parameters are treated as write operations at the entry point,
-    // so they can potentially be merged with other copies of the parameter.
-    // Note that function parameters always have distinct source variables,
-    // so we don't risk accidentally merging two parameters.
-    node.parameters.forEach(write);
-    visitStatement(node.body);
-  }
-
-  visitConstructorDefinition(ConstructorDefinition node) {
-    node.parameters.forEach(write);
-    node.initializers.forEach(visitInitializer);
-    visitStatement(node.body);
   }
 }
 
 /// Computes liveness information of the given control-flow graph.
 ///
 /// The results are stored in [Block.liveIn] and [Block.liveOut].
 void _computeLiveness(List<Block> blocks) {
   // We use a LIFO queue as worklist. Blocks are given in AST order, so by
   // inserting them in this order, we initially visit them backwards, which
   // is a good ordering.
@@ skipping 190 matching lines @@
       subst[v1] = v1;
     } else {
       subst[v1] = potential.first;
     }
   }
 
   return subst;
 }
 
 /// Performs variable substitution and removes redundant assignments.
-class SubstituteVariables extends RecursiveVisitor {
+class SubstituteVariables extends RecursiveTransformer {
 
   Map<Variable, Variable> mapping;
 
   SubstituteVariables(this.mapping);
 
   Variable replaceRead(Variable variable) {
     Variable w = mapping[variable];
     if (w == null) return variable; // Skip ignored variables.
     w.readCount++;
     variable.readCount--;
     return w;
   }
 
   Variable replaceWrite(Variable variable) {
     Variable w = mapping[variable];
     if (w == null) return variable; // Skip ignored variables.
     w.writeCount++;
     variable.writeCount--;
     return w;
   }
 
-  void replaceParameters(List<Variable> parameters) {
-    for (int i = 0; i < parameters.length; ++i) {
-      parameters[i] = replaceWrite(parameters[i]);
+  void apply(RootNode node) {
+    for (int i=0; i < node.parameters.length; i++) {

[Kevin Millikin (Google), 2015/04/08 15:10:15]

Hmmm.  It was better before.

[asgerf, 2015/04/09 09:58:23]

Well the diff is a bit misleading here.

I'm not sure what you're asking for. You want the for-loop in a method that
we're only calling once?

[Kevin Millikin (Google), 2015/04/09 10:56:32]

No, there should be a space around the binary assignment operator.  And I much
prefer to read ++i instead of i++: it matches English word order ('increment
i'), it matches all the other unary operators (!,-,~), and I find it easier to
recognize extra characters at the start of an identifier than at the end of one.

[asgerf, 2015/04/09 11:01:18]

Ah. Done.

+      node.parameters[i] = replaceWrite(node.parameters[i]);
     }
+    node.replaceEachBody(visitStatement);
   }
 
-  visitVariableUse(VariableUse node) {
-    node.variable = replaceRead(node.variable);
+  @override
+  void visitInnerFunction(FunctionDefinition node) {
+    // Do nothing. Inner functions are traversed in VariableMerger.
   }
 
-  visitFunctionDefinition(FunctionDefinition node) {
-    replaceParameters(node.parameters);
-    node.body = visitStatement(node.body);
+  Expression visitVariableUse(VariableUse node) {
+    node.variable = replaceRead(node.variable);
+    return node;
   }
 
-  visitConstructorDefinition(ConstructorDefinition node) {
-    replaceParameters(node.parameters);
-    node.initializers.forEach(visitInitializer);
-    node.body = visitStatement(node.body);
-  }
-
-  visitFieldInitializer(FieldInitializer node) {
-    node.body = visitStatement(node.body);
-  }
-
-  visitSuperInitializer(SuperInitializer node) {
-    for (int i = 0; i<node.arguments.length; ++i) {
-      node.arguments[i] = visitStatement(node.arguments[i]);
-    }
-  }
-
-  // Statement visitors should return the transformed statement so we
-  // can remove redundant assignments.
-  Statement visitStatement(Statement node) => super.visitStatement(node);
-
   Statement visitAssign(Assign node) {
     node.variable = replaceWrite(node.variable);
 
     visitExpression(node.value);
     node.next = visitStatement(node.next);
 
     // Remove assignments of form "x := x"
     if (node.value is VariableUse) {
       VariableUse value = node.value;
       if (value.variable == node.variable) {
         value.variable.readCount--;
         node.variable.writeCount--;
         return node.next;
       }
     }
 
     return node;
   }
-
-  Statement visitLabeledStatement(LabeledStatement node) {
-    node.body = visitStatement(node.body);
-    node.next = visitStatement(node.next);
-    return node;
-  }
-
-  Statement visitReturn(Return node) {
-    visitExpression(node.value);
-    return node;
-  }
-
-  Statement visitBreak(Break node) {
-    return node;
-  }
-
-  Statement visitContinue(Continue node) {
-    return node;
-  }
-
-  Statement visitIf(If node) {
-    visitExpression(node.condition);
-    node.thenStatement = visitStatement(node.thenStatement);
-    node.elseStatement = visitStatement(node.elseStatement);
-    return node;
-  }
-
-  Statement visitWhileTrue(WhileTrue node) {
-    node.body = visitStatement(node.body);
-    return node;
-  }
-
-  Statement visitWhileCondition(WhileCondition node) {
-    visitExpression(node.condition);
-    node.body = visitStatement(node.body);
-    node.next = visitStatement(node.next);
-    return node;
-  }
-
-  Statement visitFunctionDeclaration(FunctionDeclaration node) {
-    node.next = visitStatement(node.next);
-    return node;
-  }
-
-  Statement visitExpressionStatement(ExpressionStatement node) {
-    visitExpression(node.expression);
-    node.next = visitStatement(node.next);
-    return node;
-  }
-
-  Statement visitTry(Try node) {
-    node.tryBody = visitStatement(node.tryBody);
-    node.catchBody = visitStatement(node.catchBody);
-    return node;
-  }
-
-  Statement visitSetField(SetField node) {
-    visitExpression(node.object);
-    visitExpression(node.value);
-    node.next = visitStatement(node.next);
-    return node;
-  }
 }