Add support for local variables(including parameters) to cps->js.

pkg/compiler/lib/src/js_backend/codegen/codegen.dart

Index: pkg/compiler/lib/src/js_backend/codegen/codegen.dart
diff --git a/pkg/compiler/lib/src/js_backend/codegen/codegen.dart b/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
index 7d97df0dd884deb1f971ba482dbd71559861cfc8..db5a0f9af0f105990f19ce8c3fe411319ae1042d 100644
--- a/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
+++ b/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
@@ -27,8 +27,10 @@ class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
 
   final Glue glue;
 
+  ExecutableElement currentFunction;
+
   /// Variables to be hoisted at the top of the current function.
-  List<js.VariableDeclaration> variables = <js.VariableDeclaration>[];
+  List<js.VariableInitialization> variables = <js.VariableInitialization>[];
 
   /// Maps variables to their name.
   Map<tree_ir.Variable, String> variableNames = <tree_ir.Variable, String>{};
@@ -37,11 +39,13 @@ class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
   Maplet<VariableElement, String> constantNames =
       new Maplet<VariableElement, String>();
 
-  /// Variables that have had their declaration created.
+  /// Variables that already have had their declaration created.
+  /// We keep this set, so that we can create declarations for the rest after
+  /// visiting a function body.
   Set<tree_ir.Variable> declaredVariables = new Set<tree_ir.Variable>();
 
   /// Variable names that have already been used. Used to avoid name clashes.
-  Set<String> usedVariableNames;
+  Set<String> usedVariableNames = new Set<String>();
 
   List<js.Parameter> parameters = new List<js.Parameter>();
   List<js.Statement> accumulator = new List<js.Statement>();
@@ -53,13 +57,72 @@ class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
   CodeGenerator(this.glue, this.registry);
 
   void buildFunction(tree_ir.FunctionDefinition function) {
+    currentFunction = function.element;
     visitStatement(function.body);
     for (tree_ir.Variable parameter in function.parameters) {
-      parameters.add(new js.Parameter(variableNames[parameter]));
+      String name = getVariableName(parameter);
+      parameters.add(new js.Parameter(name));
+      declaredVariables.add(parameter);
+    }
+
+    // Some of the variable declarations have already been added
+    // if their first assignment could be pulled into the initializer.
+    // Add the remaining variable declarations now.
+    for (tree_ir.Variable variable in variableNames.keys) {
+      if (!declaredVariables.contains(variable)) {
+        addDeclaration(variable);
+      }
+    }
+    if (variables.length > 0) {
+      // Would be nice to avoid inserting at the beginning of list.
+      accumulator.insert(0, new js.ExpressionStatement(
+          new js.VariableDeclarationList(variables)));
     }
     body = new js.Block(accumulator);
   }
 
+  void addDeclaration(tree_ir.Variable variable, [js.Expression initializer]) {
+    assert(!declaredVariables.contains(variable));
+    String name = getVariableName(variable);
+    js.VariableInitialization decl = new js.VariableInitialization(
+        new js.VariableDeclaration(name),
+        initializer);
+    declaredVariables.add(variable);
+    variables.add(decl);
+  }
+
+  /// Generates a name for the given variable. First trying with the name of
+  /// the [Variable.element] if it is non-null.
+  String getVariableName(tree_ir.Variable variable) {
+    // TODO(sigurdm): Handle case where the variable belongs to an enclosing
+    // function.
+    if (variable.host.element != currentFunction) giveup(variable);
+
+    // Get the name if we already have one.
+    String name = variableNames[variable];
+    if (name != null) {
+      return name;
+    }
+
+    // Synthesize a variable name that isn't used elsewhere.
+    // The [usedVariableNames] set is shared between nested emitters,
+    // so this also prevents clash with variables in an enclosing/inner scope.
+    // The renaming phase after codegen will further prefix local variables
+    // so they cannot clash with top-level variables or fields.
+    String prefix = variable.element == null ? 'v' : variable.element.name;
+    int counter = 0;
+    name = glue.safeVariableName(variable.element == null
+        ? '$prefix$counter'
+        : variable.element.name);
+    while (!usedVariableNames.add(name)) {
+      ++counter;
+      name = '$prefix$counter';
+    }
+    variableNames[variable] = name;
+
+    return name;
+  }
+
   List<js.Expression> visitArguments(List<tree_ir.Expression> arguments) {
     return arguments.map(visitExpression).toList();
   }
@@ -166,7 +229,7 @@ class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
 
   @override
   js.Expression visitVariable(tree_ir.Variable node) {
-    return giveup(node);
+    return new js.VariableUse(getVariableName(node));
     // TODO: implement visitVariable
   }
 
@@ -203,8 +266,42 @@ class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
 
   @override
   void visitAssign(tree_ir.Assign node) {
-    giveup(node);
-    // TODO: implement visitAssign
+    // Try to emit a local function declaration. This is useful for functions
+    // that may occur in expression context, but could not be inlined anywhere.
+    if (node.variable.element is FunctionElement &&

[floitsch, 2014/11/18 19:00:43]

I don't think we need to special case function-expressions.
The visitFunctionExpression will not be implemented anytime soon, and when we
do, we can look into how we want to do it.
Until then just remove the 'if', and let the visitFunctionExpression giveUp.

+        node.definition is tree_ir.FunctionExpression &&
+        !declaredVariables.contains(node.variable)) {
+      // TODO(sigurdm): implement function expressions.
+      giveup(node.definition);
+    }
+
+    bool isFirstOccurrence = (variableNames[node.variable] == null);
+    bool isDeclaredHere = node.variable.host.element == currentFunction;
+    String name = getVariableName(node.variable);
+    tree_ir.Expression value = node.definition;
+    js.Expression definition = visitExpression(value);
+
+    // Emit a variable declaration if we are required to do so.
+    // This is to ensure that a fresh closure variable is created.

[floitsch, 2014/11/18 19:00:44]

That doesn't make sense in the dart2js backend.

I would just remove this if too, and remove the declaredVariables list (and
"addDeclaredVariable" function) entirely.

+    if (node.isDeclaration) {
+      if (isNullLiteral(definition)) {
+        definition = null;
+      }
+      js.VariableInitialization initialization =
+          new js.VariableInitialization(new js.VariableDeclaration(name),
+                                        definition);
+      declaredVariables.add(node.variable);
+      accumulator.add(new js.ExpressionStatement(
+          new js.VariableDeclarationList([initialization])));
+      visitStatement(node.next);
+      return;
+    }
+
+    accumulator.add(new js.ExpressionStatement(new js.Assignment(
+        visitVariable(node.variable),
+        definition)));
+    visitStatement(node.next);
+
   }
 
   @override
@@ -231,4 +328,7 @@ class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
       accumulator.add(new js.Return(visitExpression(node.value)));
     }
   }
+
+  bool isNullLiteral(js.Expression exp) => exp is js.LiteralNull;
+
 }