fixes #516, nested cascade code generation

lib/src/compiler/code_generator.dart

 // Copyright (c) 2015, 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.
 
 import 'dart:collection' show HashMap, HashSet;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/dart/ast/ast.dart' hide ConstantEvaluator;
 import 'package:analyzer/dart/ast/token.dart' show Token, TokenType;
 import 'package:analyzer/dart/element/element.dart';
@@ skipping 63 matching lines @@
   Expression _cascadeTarget;
 
   /// The variable for the current catch clause
   SimpleIdentifier _catchParameter;
 
   /// In an async* function, this represents the stream controller parameter.
   JS.TemporaryId _asyncStarController;
 
   final _privateNames =
       new HashMap<LibraryElement, HashMap<String, JS.TemporaryId>>();
-  final _temps = new HashMap<Element, JS.TemporaryId>();
+  final _initializingFormalTemps =
+      new HashMap<ParameterElement, JS.TemporaryId>();
 
   final _dartxVar = new JS.Identifier('dartx');
   final _runtimeLibVar = new JS.Identifier('dart');
   final namedArgumentTemp = new JS.TemporaryId('opts');
 
   final _hasDeferredSupertype = new HashSet<ClassElement>();
 
   /// The  type provider from the current Analysis [context].
   final TypeProvider types;
 
@@ skipping 1811 matching lines @@
       var tearOff = element is MethodElement && !inInvocationContext(node);
       var code = (tearOff) ? 'dart.bind(this, #)' : 'this.#';
       return js.call(code, member);
     }
 
     if (element is ParameterElement) {
       return _emitParameter(element);
     }
 
     if (element is TemporaryVariableElement) {
-      if (name[0] == '#') {
-        return new JS.InterpolatedExpression(name.substring(1));
-      } else {
-        return _getTemp(element, name);
-      }
+      return element.variable;

[vsm, 2016/04/21 21:50:48]

This doesn't match the return type of this function, right?

[Jennifer Messerly, 2016/04/21 21:53:25]

JS.Expression visitSimpleIdentifier ... and we're returning a JS.Expression, I
don't see the problem?
if it helps I can rename it to "jsVariable"?

[vsm, 2016/04/21 22:06:22]

Oh, I missed that TemporaryVariableElement is our class, not the analyzer's.  I
was confused how it had a JS.Expression on it (even you edited it below :-)).

     }
 
     return new JS.Identifier(name);
   }
 
   JS.Identifier _emitParameter(ParameterElement element,
       {bool declaration: false}) {
     // initializing formal parameter, e.g. `Point(this._x)`
     // TODO(jmesserly): type ref is not attached in this case.
     if (element.isInitializingFormal && element.isPrivate) {
       /// Rename private names so they don't shadow the private field symbol.
       /// The renamer would handle this, but it would prefer to rename the
       /// temporary used for the private symbol. Instead rename the parameter.
-      return _getTemp(element, '${element.name.substring(1)}');
+      return _initializingFormalTemps.putIfAbsent(
+          element, () => new JS.TemporaryId(element.name.substring(1)));
     }
 
     var type = declaration ? emitTypeRef(element.type) : null;
     return new JS.Identifier(element.name, type: type);
   }
 
-  JS.TemporaryId _getTemp(Element key, String name) =>
-      _temps.putIfAbsent(key, () => new JS.TemporaryId(name));
-
   List<Annotation> _parameterMetadata(FormalParameter p) =>
       (p is NormalFormalParameter)
           ? p.metadata
           : (p as DefaultFormalParameter).parameter.metadata;
 
   JS.ArrayInitializer _emitTypeNames(List<DartType> types,
       [List<FormalParameter> parameters]) {
     var result = <JS.Expression>[];
     for (int i = 0; i < types.length; ++i) {
       var metadata =
@@ skipping 125 matching lines @@
       Expression left, Expression right, String op, MethodElement element,
       {Expression context}) {
     if (op == '??') {
       // Desugar `l ??= r` as ((x) => x == null ? l = r : x)(l)
       // Note that if `x` contains subexpressions, we need to ensure those
       // are also evaluated only once. This is similar to desguaring for
       // postfix expressions like `i++`.
 
       // Handle the left hand side, to ensure each of its subexpressions are
       // evaluated only once.
-      var vars = <String, JS.Expression>{};
+      var vars = <JS.MetaLetVariable, JS.Expression>{};
       var x = _bindLeftHandSide(vars, left, context: left);
       // Capture the result of evaluating the left hand side in a temp.
       var t = _bindValue(vars, 't', x, context: x);
       return new JS.MetaLet(vars, [
         js.call('# == null ? # : #', [_visit(t), _emitSet(x, right), _visit(t)])
       ]);
     }
 
     // Desugar `x += y` as `x = x + y`, ensuring that if `x` has subexpressions
     // (for example, x is IndexExpression) we evaluate those once.
-    var vars = <String, JS.Expression>{};
+    var vars = <JS.MetaLetVariable, JS.Expression>{};
     var lhs = _bindLeftHandSide(vars, left, context: context);
     var inc = AstBuilder.binaryExpression(lhs, op, right);
     inc.staticElement = element;
     inc.staticType = getStaticType(left);
     return new JS.MetaLet(vars, [_emitSet(lhs, inc)]);
   }
 
   JS.Expression _emitSet(Expression lhs, Expression rhs) {
     if (lhs is IndexExpression) {
       var target = _getTarget(lhs);
@@ skipping 28 matching lines @@
 
   JS.Expression _emitNullSafeSet(PropertyAccess node, Expression right) {
     // Emit `obj?.prop = expr` as:
     //
     //     (_ => _ == null ? null : _.prop = expr)(obj).
     //
     // We could use a helper, e.g.:  `nullSafeSet(e1, _ => _.v = e2)`
     //
     // However with MetaLet, we get clean code in statement or void context,
     // or when one of the expressions is stateless, which seems common.
-    var vars = <String, JS.Expression>{};
+    var vars = <JS.MetaLetVariable, JS.Expression>{};
     var left = _bindValue(vars, 'l', node.target);
     var body = js.call('# == null ? null : #',
         [_visit(left), _emitSet(_stripNullAwareOp(node, left), right)]);
     return new JS.MetaLet(vars, [body]);
   }
 
   @override
   JS.Block visitExpressionFunctionBody(ExpressionFunctionBody node) {
     var savedFunction = _currentFunction;
     _currentFunction = node;
@@ skipping 555 matching lines @@
         code = '${bang}dart.equals(#, #)';
       }
       return js.call(code, [_visit(left), _visit(right)]);
     }
 
     if (op.type.lexeme == '??') {
       // TODO(jmesserly): leave RHS for debugging?
       // This should be a hint or warning for dead code.
       if (!isNullable(left)) return _visit(left);
 
-      var vars = <String, JS.Expression>{};
+      var vars = <JS.MetaLetVariable, JS.Expression>{};
       // Desugar `l ?? r` as `l != null ? l : r`
       var l = _visit(_bindValue(vars, 'l', left, context: left));
       return new JS.MetaLet(vars, [
         js.call('# != null ? # : #', [l, l, _visit(right)])
       ]);
     }
 
     if (binaryOperationIsPrimitive(leftType, rightType) ||
         leftType == types.stringType && op.type == TokenType.PLUS) {
       // special cases where we inline the operation
@@ skipping 28 matching lines @@
     if (_isNull(left) || _isNull(right)) return true;
 
     var leftType = _canonicalizeNumTypes(getStaticType(left));
     var rightType = _canonicalizeNumTypes(getStaticType(right));
     return isPrimitiveType(leftType) && leftType == rightType;
   }
 
   bool _isNull(Expression expr) => expr is NullLiteral;
 
   SimpleIdentifier _createTemporary(String name, DartType type,
-      {bool nullable: true}) {
+      {bool nullable: true, JS.Expression variable}) {
     // We use an invalid source location to signal that this is a temporary.
     // See [_isTemporary].
     // TODO(jmesserly): alternatives are
     // * (ab)use Element.isSynthetic, which isn't currently used for
     //   LocalVariableElementImpl, so we could repurpose to mean "temp".
     // * add a new property to LocalVariableElementImpl.
     // * create a new subtype of LocalVariableElementImpl to mark a temp.
     var id =
         new SimpleIdentifier(new StringToken(TokenType.IDENTIFIER, name, -1));
-    id.staticElement = new TemporaryVariableElement.forNode(id);
+
+    variable ??= new JS.TemporaryId(name);
+
+    id.staticElement = new TemporaryVariableElement.forNode(id, variable);
     id.staticType = type;
     DynamicInvoke.set(id, type.isDynamic);
     addTemporaryVariable(id.staticElement, nullable: nullable);
     return id;
   }
 
   JS.Expression _emitConst(JS.Expression expr()) {
     // TODO(jmesserly): emit the constants at top level if possible.
     // This wasn't quite working, so disabled for now.
     return js.call('dart.const(#)', expr());
   }
 
   /// Returns a new expression, which can be be used safely *once* on the
   /// left hand side, and *once* on the right side of an assignment.
   /// For example: `expr1[expr2] += y` can be compiled as
   /// `expr1[expr2] = expr1[expr2] + y`.
   ///
   /// The temporary scope will ensure `expr1` and `expr2` are only evaluated
   /// once: `((x1, x2) => x1[x2] = x1[x2] + y)(expr1, expr2)`.
   ///
   /// If the expression does not end up using `x1` or `x2` more than once, or
   /// if those expressions can be treated as stateless (e.g. they are
   /// non-mutated variables), then the resulting code will be simplified
   /// automatically.
   ///
   /// [scope] can be mutated to contain any new temporaries that were created,
   /// unless [expr] is a SimpleIdentifier, in which case a temporary is not
   /// needed.
   Expression _bindLeftHandSide(
-      Map<String, JS.Expression> scope, Expression expr,
+      Map<JS.MetaLetVariable, JS.Expression> scope, Expression expr,
       {Expression context}) {
     Expression result;
     if (expr is IndexExpression) {
       IndexExpression index = expr;
       result = new IndexExpression.forTarget(
           _bindValue(scope, 'o', index.target, context: context),
           index.leftBracket,
           _bindValue(scope, 'i', index.index, context: context),
           index.rightBracket);
     } else if (expr is PropertyAccess) {
@@ skipping 23 matching lines @@
   /// Creates a temporary to contain the value of [expr]. The temporary can be
   /// used multiple times in the resulting expression. For example:
   /// `expr ** 2` could be compiled as `expr * expr`. The temporary scope will
   /// ensure `expr` is only evaluated once: `(x => x * x)(expr)`.
   ///
   /// If the expression does not end up using `x` more than once, or if those
   /// expressions can be treated as stateless (e.g. they are non-mutated
   /// variables), then the resulting code will be simplified automatically.
   ///
   /// [scope] will be mutated to contain the new temporary's initialization.
-  Expression _bindValue(
-      Map<String, JS.Expression> scope, String name, Expression expr,
+  Expression _bindValue(Map<JS.MetaLetVariable, JS.Expression> scope,
+      String name, Expression expr,
       {Expression context}) {
     // No need to do anything for stateless expressions.
     if (isStateless(_currentFunction, expr, context)) return expr;
 
-    var t = _createTemporary('#$name', getStaticType(expr));
-    scope[name] = _visit(expr);
+    var variable = new JS.MetaLetVariable(name);
+    var t = _createTemporary(name, getStaticType(expr), variable: variable);
+    scope[variable] = _visit(expr);
     return t;
   }
 
   /// Desugars postfix increment.
   ///
   /// In the general case [expr] can be one of [IndexExpression],
   /// [PrefixExpression] or [PropertyAccess] and we need to
   /// ensure sub-expressions are evaluated once.
   ///
   /// We also need to ensure we can return the original value of the expression,
@@ skipping 17 matching lines @@
     if (unaryOperationIsPrimitive(dispatchType)) {
       if (!isNullable(expr)) {
         return js.call('#$op', _visit(expr));
       }
     }
 
     assert(op.lexeme == '++' || op.lexeme == '--');
 
     // Handle the left hand side, to ensure each of its subexpressions are
     // evaluated only once.
-    var vars = <String, JS.Expression>{};
+    var vars = <JS.MetaLetVariable, JS.Expression>{};
     var left = _bindLeftHandSide(vars, expr, context: expr);
 
     // Desugar `x++` as `(x1 = x0 + 1, x0)` where `x0` is the original value
     // and `x1` is the new value for `x`.
     var x = _bindValue(vars, 'x', left, context: expr);
 
     var one = AstBuilder.integerLiteral(1)..staticType = types.intType;
     var increment = AstBuilder.binaryExpression(x, op.lexeme[0], one)
       ..staticElement = node.staticElement
       ..staticType = getStaticType(expr);
 
     var body = <JS.Expression>[_emitSet(left, increment), _visit(x)];
     return new JS.MetaLet(vars, body, statelessResult: true);
   }
 
   @override
   JS.Expression visitPrefixExpression(PrefixExpression node) {
     var op = node.operator;
     var expr = node.operand;
 
     var dispatchType = getStaticType(expr);
     if (unaryOperationIsPrimitive(dispatchType)) {
       if (!isNullable(expr)) {
         return js.call('$op#', _visit(expr));
       } else if (op.lexeme == '++' || op.lexeme == '--') {
         // We need a null check, so the increment must be expanded out.
-        var vars = <String, JS.Expression>{};
+        var vars = <JS.MetaLetVariable, JS.Expression>{};
         var x = _bindLeftHandSide(vars, expr, context: expr);
 
         var one = AstBuilder.integerLiteral(1)..staticType = types.intType;
         var increment = AstBuilder.binaryExpression(x, op.lexeme[0], one)
           ..staticElement = node.staticElement
           ..staticType = getStaticType(expr);
 
         return new JS.MetaLet(vars, [_emitSet(x, increment)]);
       } else {
         return js.call('$op#', notNull(expr));
@@ skipping 12 matching lines @@
     return _emitSend(expr, op.lexeme[0], []);
   }
 
   // Cascades can contain [IndexExpression], [MethodInvocation] and
   // [PropertyAccess]. The code generation for those is handled in their
   // respective visit methods.
   @override
   JS.Node visitCascadeExpression(CascadeExpression node) {
     var savedCascadeTemp = _cascadeTarget;
 
-    var vars = <String, JS.Expression>{};
+    var vars = <JS.MetaLetVariable, JS.Expression>{};
     _cascadeTarget = _bindValue(vars, '_', node.target, context: node);
     var sections = _visitList(node.cascadeSections) as List<JS.Expression>;
     sections.add(_visit(_cascadeTarget));
     var result = new JS.MetaLet(vars, sections, statelessResult: true);
     _cascadeTarget = savedCascadeTemp;
     return result;
   }
 
   @override
   visitParenthesizedExpression(ParenthesizedExpression node) =>
@@ skipping 798 matching lines @@
 
 // TODO(jacobr): we would like to do something like the following
 // but we don't have summary support yet.
 // bool _supportJsExtensionMethod(AnnotatedNode node) =>
 //    _getAnnotation(node, "SupportJsExtensionMethod") != null;
 
 /// A special kind of element created by the compiler, signifying a temporary
 /// variable. These objects use instance equality, and should be shared
 /// everywhere in the tree where they are treated as the same variable.
 class TemporaryVariableElement extends LocalVariableElementImpl {
-  TemporaryVariableElement.forNode(Identifier name) : super.forNode(name);
+  final JS.Expression variable;
+  TemporaryVariableElement.forNode(Identifier name, this.variable)
+      : super.forNode(name);
 
   int get hashCode => identityHashCode(this);
   bool operator ==(Object other) => identical(this, other);
 }
 
 bool isLibraryPrefix(Expression node) =>
     node is SimpleIdentifier && node.staticElement is PrefixElement;
 
 LibraryElement _getLibrary(AnalysisContext c, String uri) =>
     c.computeLibraryElement(c.sourceFactory.forUri(uri));
 
 bool _isDartRuntime(LibraryElement l) =>
     l.isInSdk && l.source.uri.toString() == 'dart:_runtime';