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 c69eb5f0d237b55b8d9707ec9802f3d931d0dd96..d65ab64791a1b85e92bd1fd41b2c783b564a52cb 100644
--- a/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
+++ b/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
@@ -20,8 +20,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>{};
@@ -34,11 +36,13 @@ class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
   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>();
 
+  tree_ir.Statement firstStatement;
+
   js.Block body;
 
   /// Generates JavaScript code for the body of [function].
@@ -46,13 +50,73 @@ class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
   CodeGenerator(this.glue, this.registry);
 
   void buildFunction(tree_ir.FunctionDefinition function) {
+    currentFunction = function.element;
+    firstStatement = function.body;
     visitStatement(function.body);
     for (tree_ir.Variable parameter in function.parameters) {
       parameters.add(new js.Parameter(variableNames[parameter]));
+      variableNames[parameter] = parameter.element.name;

[floitsch, 2014/11/13 15:59:32]

We will need to filter names here.
There are some valid Dart names, that can't be used in JS. (Ex: 'function').

language/naming2_test should test this.

Should we go through 'getVariableName'?

[sigurdm, 2014/11/18 15:17:46]

Yes I think that is the right thing to do. I think this is not done for
dart2dart because we prefer to preserve the parameter names.

I added a call to the safe name functionality to getVariableName

+      usedVariableNames.add(parameter.element.name);
+      declaredVariables.add(parameter);
+    }
+
+
+    // Some of the variable declarations have already been added
+    // if their first assignment could be pulled into the initializer.

[floitsch, 2014/11/13 15:59:32]

Not completely clear what this means.

+    // 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) throw UNIMPLEMENTED;
+
+    // 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 = 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();
   }
@@ -174,7 +238,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
   }
 
@@ -211,8 +275,52 @@ 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 &&
+        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);
+
+    // Try to pull into initializer.

[floitsch, 2014/11/13 15:59:32]

I don't think we want to do this.
We found that:
var x, y, z;
x = XYZ;
y = abc;
z = SSS;

is smaller than:
var x = XYZ;
var y = abc;
var z = SSS;

(assuming that we can't initialize the variables directly in the declaration
list: var x = XYZ, y = abc, z = SSS);

Also note that the JS tree already does the same thing again.
We might just want to make it possible to move the assignments into the list
(although "var a, b, c, d..." is extremely well gzipped).

[sigurdm, 2014/11/18 15:17:45]

Ok, removed for now.

+    if (firstStatement == node && isFirstOccurrence && isDeclaredHere) {
+      // if (isNullLiteral(definition)) definition = null;

[floitsch, 2014/11/13 15:59:32]

commented code.

[sigurdm, 2014/11/18 15:17:46]

Removed

+      addDeclaration(node.variable, definition);
+      firstStatement = node.next;
+      visitStatement(node.next);
+      return;
+    }
+
+    // Emit a variable declaration if we are required to do so.
+    // This is to ensure that a fresh closure variable is created.
+    if (node.isDeclaration) {
+      assert(isFirstOccurrence);
+      assert(isDeclaredHere);
+      // if (isNullLiteral(definition)) definition = null;

[floitsch, 2014/11/13 15:59:32]

commented code.

[sigurdm, 2014/11/18 15:17:46]

Done.

+      js.VariableInitialization decl =
+          new js.VariableInitialization(new js.VariableDeclaration(name),
+                                        definition);
+      declaredVariables.add(node.variable);
+      accumulator.add(new js.ExpressionStatement(
+          new js.VariableDeclarationList([decl])));
+      visitStatement(node.next);

[floitsch, 2014/11/13 15:59:32]

I'm surprised that the visit-functions have to trigger the visit of the next
instruction. But I guess this is part of the IR visitor...

[sigurdm, 2014/11/18 15:17:46]

Yes - this is a side effect of how the tree IR is structured.

+      return;
+    }
+
+    accumulator.add(new js.ExpressionStatement(new js.Assignment(
+        visitVariable(node.variable),
+        definition)));
+    visitStatement(node.next);
+
   }
 
   @override