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

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

 // Copyright (c) 2014, 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 code_generator;
 
 import 'glue.dart';
 
 import '../../tree_ir/tree_ir_nodes.dart' as tree_ir;
 import '../../js/js.dart' as js;
 import '../../elements/elements.dart';
 import '../../util/maplet.dart';
 import '../../constants/values.dart';
 import '../../dart2jslib.dart';
 
 class CodeGenerator extends tree_ir.Visitor<dynamic, js.Expression> {
   static const String UNIMPLEMENTED = "Javascript generation aborted";
 
   final CodegenRegistry registry;
 
   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>{};
 
   /// Maps local constants to their name.
   Maplet<VariableElement, String> constantNames =
       new Maplet<VariableElement, String>();
 
   /// Variables that have had their declaration created.

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

I'm not sure I understand that comment.

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

I tried to improve it.

   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].
   /// The code will be in [body] and the parameters will be in [parameters].
   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();
   }
 
   giveup(tree_ir.Node node) {
     throw UNIMPLEMENTED;
   }
 
   @override
   js.Expression visitConcatenateStrings(tree_ir.ConcatenateStrings node) {
@@ skipping 101 matching lines @@
   }
 
   @override
   js.Expression visitTypeOperator(tree_ir.TypeOperator node) {
     return giveup(node);
     // TODO: implement visitTypeOperator
   }
 
   @override
   js.Expression visitVariable(tree_ir.Variable node) {
-    return giveup(node);
+    return new js.VariableUse(getVariableName(node));
     // TODO: implement visitVariable
   }
 
   @override
   void visitContinue(tree_ir.Continue node) {
     return giveup(node);
     // TODO: implement visitContinue
   }
 
   @override
@@ skipping 16 matching lines @@
   }
 
   @override
   void visitLabeledStatement(tree_ir.LabeledStatement node) {
     giveup(node);
     // TODO: implement visitLabeledStatement
   }
 
   @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
   void visitBreak(tree_ir.Break node) {
     giveup(node);
     // TODO: implement visitBreak
   }
 
   @override
   void visitWhileCondition(tree_ir.WhileCondition node) {
     giveup(node);
     // TODO: implement visitWhileCondition
   }
 
   @override
   void visitWhileTrue(tree_ir.WhileTrue node) {
     giveup(node);
     // TODO: implement visitWhileTrue
   }
 
   @override
   void visitReturn(tree_ir.Return node) {
     if (node.value != null) {
       accumulator.add(new js.Return(visitExpression(node.value)));
     }
   }
 }