Overhaul tree IR visitor and rename IR classes.

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

Index: pkg/compiler/lib/src/tree_ir/optimization/variable_merger.dart
diff --git a/pkg/compiler/lib/src/tree_ir/optimization/variable_merger.dart b/pkg/compiler/lib/src/tree_ir/optimization/variable_merger.dart
index dfe41adf29be207780aef22a7c32f6e53b5ae1b7..d3ae5ff8baaa82b075da47ca8150a0ec315cf0e4 100644
--- a/pkg/compiler/lib/src/tree_ir/optimization/variable_merger.dart
+++ b/pkg/compiler/lib/src/tree_ir/optimization/variable_merger.dart
@@ -13,37 +13,30 @@ import '../../elements/elements.dart' show Local, ParameterElement;
 /// 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';
 
-  @override
-  void rewriteExecutableDefinition(ExecutableDefinition node) {
-    visitExecutableDefinition(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.
+  void rewrite(RootNode node) {
     rewriteFunction(node);
+    node.forEachBody(visitStatement);
   }
 
-  visitFieldDefinition(FieldDefinition node) {
-    super.visitFieldDefinition(node);
+  @override
+  void visitInnerFunction(FunctionDefinition node) {
     rewriteFunction(node);
   }
 
-  visitConstructorDefinition(ConstructorDefinition node) {
-    super.visitConstructorDefinition(node);
-    rewriteFunction(node);
+  /// Rewrites the given function.
+  /// This is called for the outermost function and inner functions.
+  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);
+    });
   }
 }
 
@@ -116,8 +109,15 @@ class BlockGraphBuilder extends RecursiveVisitor {
   /// 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]
@@ -244,26 +244,6 @@ class BlockGraphBuilder extends RecursiveVisitor {
     // 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);
   }
 }
 
@@ -474,7 +454,7 @@ Map<Variable, Variable> _computeRegisterAllocation(List<Block> blocks) {
 }
 
 /// Performs variable substitution and removes redundant assignments.
-class SubstituteVariables extends RecursiveVisitor {
+class SubstituteVariables extends RecursiveTransformer {
 
   Map<Variable, Variable> mapping;
 
@@ -496,41 +476,23 @@ class SubstituteVariables extends RecursiveVisitor {
     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);
-  }
-
-  visitFunctionDefinition(FunctionDefinition node) {
-    replaceParameters(node.parameters);
-    node.body = visitStatement(node.body);
-  }
-
-  visitConstructorDefinition(ConstructorDefinition node) {
-    replaceParameters(node.parameters);
-    node.initializers.forEach(visitInitializer);
-    node.body = visitStatement(node.body);
-  }
-
-  visitFieldInitializer(FieldInitializer node) {
-    node.body = visitStatement(node.body);
+  @override
+  void visitInnerFunction(FunctionDefinition node) {
+    // Do nothing. Inner functions are traversed in VariableMerger.
   }
 
-  visitSuperInitializer(SuperInitializer node) {
-    for (int i = 0; i<node.arguments.length; ++i) {
-      node.arguments[i] = visitStatement(node.arguments[i]);
-    }
+  Expression visitVariableUse(VariableUse node) {
+    node.variable = replaceRead(node.variable);
+    return node;
   }
 
-  // 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);
 
@@ -549,66 +511,4 @@ class SubstituteVariables extends RecursiveVisitor {
 
     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;
-  }
 }