dart2dart: Preserve variable names throughout the IR.

sdk/lib/_internal/compiler/implementation/ir/ir_builder.dart

 // Copyright (c) 2013, 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 dart2js.ir_builder;
 
 import 'ir_nodes.dart' as ir;
 import '../elements/elements.dart';
 import '../dart2jslib.dart';
 import '../dart_types.dart';
@@ skipping 173 matching lines @@
   // list of references.  When the delimited subexpression is plugged into the
   // surrounding context, the free occurrences can be captured or become free
   // occurrences in the next outer delimited subexpression.
   //
   // Each nested visitor maintains a list that maps indexes of variables
   // assigned in the delimited subexpression to their reaching definition ---
   // that is, the definition in effect at the hole in 'current'.  These are
   // used to determine if a join-point continuation needs to be passed
   // arguments, and what the arguments are.
   final Map<Element, int> variableIndex;
+  final List<Element> index2variable;
   final List<ir.Parameter> freeVars;
   final List<ir.Primitive> assignedVars;
 
   /// Construct a top-level visitor.
   IrBuilder(TreeElements elements, Compiler compiler, this.sourceFile)
       : returnContinuation = new ir.Continuation.retrn(),
         parameters = <ir.Parameter>[],
         variableIndex = <Element, int>{},
         freeVars = null,
         assignedVars = <ir.Primitive>[],
+        index2variable = <Element>[],
         super(elements, compiler);
 
   /// Construct a delimited visitor.
   IrBuilder.delimited(IrBuilder parent)
       : sourceFile = parent.sourceFile,
         returnContinuation = parent.returnContinuation,
         parameters = parent.parameters,
         variableIndex = parent.variableIndex,
         freeVars = new List<ir.Parameter>.generate(
             parent.assignedVars.length, (_) => new ir.Parameter(null),
             growable: false),
         assignedVars = new List<ir.Primitive>.generate(
             parent.assignedVars.length, (_) => null),
+        index2variable = new List<Element>.from(parent.index2variable),
         super(parent.elements, parent.compiler);
 
   /**
    * Builds the [ir.FunctionDefinition] for a function element. In case the
    * function uses features that cannot be expressed in the IR, this function
    * returns `null`.
    */
   ir.FunctionDefinition buildFunction(FunctionElement functionElement) {
     return nullIfGiveup(() => buildFunctionInternal(functionElement));
   }
 
   ir.FunctionDefinition buildFunctionInternal(FunctionElement element) {
     assert(invariant(element, element.isImplementation));
     ast.FunctionExpression function = element.node;
     assert(function != null);
     assert(!function.modifiers.isExternal);
     assert(elements[function] != null);
 
     root = current = null;
 
     FunctionSignature signature = element.functionSignature;
     signature.orderedForEachParameter((parameterElement) {
       ir.Parameter parameter = new ir.Parameter(parameterElement);
       parameters.add(parameter);
       variableIndex[parameterElement] = assignedVars.length;
       assignedVars.add(parameter);
+      index2variable.add(parameterElement);
     });
 
     visit(function.body);
     ensureReturn(function);
     return new ir.FunctionDefinition(returnContinuation, parameters, root);
   }
 
   ConstantSystem get constantSystem => compiler.backend.constantSystem;
 
   bool get isOpen => root == null || current != null;
@@ skipping 85 matching lines @@
       // The last assignments, if any, reaching the end of the two subterms.
       ir.Primitive leftAssignment = leftBuilder.assignedVars[i];
       ir.Primitive rightAssignment = rightBuilder.assignedVars[i];
 
       if (leftAssignment != null || rightAssignment != null) {
         // The corresponsing argument is the reaching definition if any, or a
         // free occurrence.  In the case that control does not reach both the
         // left and right subterms we will still have a join continuation with
         // possibly arguments passed to it.  Such singly-used continuations
         // are eliminated by the shrinking conversions.
-        parameters.add(new ir.Parameter(null));
+        parameters.add(new ir.Parameter(index2variable[i]));
         ir.Primitive reachingDefinition =
             assignedVars[i] == null ? freeVars[i] : assignedVars[i];
         leftArguments.add(
             leftAssignment == null ? reachingDefinition : leftAssignment);
         rightArguments.add(
             rightAssignment == null ? reachingDefinition : rightAssignment);
       }
     }
     return parameters;
   }
@@ skipping 29 matching lines @@
     for (int i = 0; i < assignedVars.length; ++i) {
       // Was there an assignment in the body?
       ir.Definition reachingAssignment = bodyBuilder.assignedVars[i];
       // If not, was there an assignment in the condition?
       if (reachingAssignment == null) {
         reachingAssignment = condBuilder.assignedVars[i];
       }
       // If not, no value needs to be passed to the join point.
       if (reachingAssignment == null) continue;
 
-      parameters.add(new ir.Parameter(null));
+      parameters.add(new ir.Parameter(index2variable[i]));
       ir.Definition entryAssignment = assignedVars[i];
       entryArguments.add(
           entryAssignment == null ? freeVars[i] : entryAssignment);
       loopArguments.add(reachingAssignment);
     }
     return parameters;
   }
 
   /// Capture free variables in the arms of a branch.
   ///
@@ skipping 212 matching lines @@
   ir.Primitive visitVariableDefinitions(ast.VariableDefinitions node) {
     assert(isOpen);
     for (ast.Node definition in node.definitions.nodes) {
       Element element = elements[definition];
       // Definitions are either SendSets if there is an initializer, or
       // Identifiers if there is no initializer.
       if (definition is ast.SendSet) {
         assert(!definition.arguments.isEmpty);
         assert(definition.arguments.tail.isEmpty);
         ir.Primitive initialValue = visit(definition.arguments.head);
+        // If a temporary was introduced for the initializer expression,

[Kevin Millikin (Google), 2014/06/10 11:33:01]

Hmm, the comment says "If" but the code is unconditional.  Maybe something more
like: "Record the variable name as a hint in case the initial value is named in
the output."  Or is that not quite right?

[asgerf, 2014/06/10 12:22:24]

The 'if' I mention is inside the method (if element == null). For instance, if
the initializer is a parameter reference, we don't want to overwrite its
element.

But I agree that the comment is not that great. I've rewritten the comment a
bit. In particular, saying "in case," makes more sense than saying "if".

+        // do not use a separate variable for it.
+        useElementForVariable(initialValue, element);
         variableIndex[element] = assignedVars.length;
         assignedVars.add(initialValue);
+        index2variable.add(element);
       } else {
         assert(definition is ast.Identifier);
         // The initial value is null.
         // TODO(kmillikin): Consider pooling constants.
         ir.Constant constant = new ir.Constant(constantSystem.createNull());
+        useElementForVariable(constant, element);
         add(new ir.LetPrim(constant));
         variableIndex[element] = assignedVars.length;
         assignedVars.add(constant);
+        index2variable.add(element);
       }
     }
     return null;
   }
 
   // Build(Return(e), C) = C'[InvokeContinuation(return, x)]
   //   where (C', x) = Build(e, C)
   //
   // Return without a subexpression is translated as if it were return null.
   ir.Primitive visitReturn(ast.Return node) {
@@ skipping 430 matching lines @@
 
   ir.Primitive visitSendSet(ast.SendSet node) {
     assert(isOpen);
     Element element = elements[node];
     if (node.assignmentOperator.source != '=') return giveup();
     if (Elements.isLocal(element)) {
       // Exactly one argument expected for a simple assignment.
       assert(!node.arguments.isEmpty);
       assert(node.arguments.tail.isEmpty);
       ir.Primitive result = visit(node.arguments.head);
+      useElementForVariable(result, element);
       assignedVars[variableIndex[element]] = result;
       return result;
     } else if (Elements.isStaticOrTopLevel(element)) {
       // TODO(asgerf): static and top-level
       return giveup();
     } else if (node.receiver == null) {
       // Nodes that fall in this case:
       // - Unresolved top-level
       // - Assignment to final variable (will not be resolved)
       return giveup();
@@ skipping 69 matching lines @@
       arguments.add(visit(part.expression));
       arguments.add(visitLiteralString(part.string));
     }
     ir.Parameter v = new ir.Parameter(null);
     ir.Continuation k = new ir.Continuation([v]);
     ir.ConcatenateStrings concat = new ir.ConcatenateStrings(k, arguments);
     add(new ir.LetCont(k, concat));
     return v;
   }
 
+  /// Associates [primitive] with the given [element].
+  /// This is used as a hint when choosing variables names later when
+  /// translating out of the IR again.
+  void useElementForVariable(ir.Primitive primitive, Element element) {

[Kevin Millikin (Google), 2014/06/10 11:33:01]

This works better as a method on Primitive, doesn't it?

[asgerf, 2014/06/10 12:22:24]

Yes, good idea.

+    // Use the first assigned element
+    if (primitive.element != null) return;
+
+    primitive.element = element;
+  }
+
   static final String ABORT_IRNODE_BUILDER = "IrNode builder aborted";
 
   ir.Primitive giveup() => throw ABORT_IRNODE_BUILDER;
 
   ir.FunctionDefinition nullIfGiveup(ir.FunctionDefinition action()) {
     try {
       return action();
     } catch(e) {
       if (e == ABORT_IRNODE_BUILDER) return null;
       rethrow;
@@ skipping 38 matching lines @@
   bool visitType(DartType type, Null _) => false;
 
   bool visitDynamicType(DynamicType type, Null _) => true;
 
   bool visitVoidType(VoidType type, Null _) => true;
 
   // Currently, InterfaceType and TypedefType are supported so long as they
   // do not have type parameters.  They are subclasses of GenericType.
   bool visitGenericType(GenericType type, Null _) => !type.isGeneric;
 }