Fix increment on nullable

lib/src/codegen/js_codegen.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.
 
 library ddc.src.codegen.js_codegen;
 
 import 'dart:collection' show HashSet;
 import 'dart:io' show Directory, File;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
@@ skipping 1047 matching lines @@
 
   bool typeIsNonNullablePrimitiveInJS(DartType t) =>
       typeIsPrimitiveInJS(t) && rules.isNonNullableType(t);
 
   bool binaryOperationIsPrimitive(DartType leftT, DartType rightT) =>
       typeIsPrimitiveInJS(leftT) && typeIsPrimitiveInJS(rightT);
 
   bool unaryOperationIsPrimitive(DartType t) => typeIsPrimitiveInJS(t);
 
   bool _isNonNullableExpression(Expression expr) {
+    // If the type is non-nullable, no further checking needed.
+    if (rules.isNonNullableType(rules.getStaticType(expr))) return true;
+
     // TODO(vsm): Revisit whether we really need this when we get
     // better non-nullability in the type system.
 
     if (expr is Literal && expr is! NullLiteral) {
       return true;
     }
     if (expr is ParenthesizedExpression) {
       return _isNonNullableExpression(expr.expression);
     }
     DartType type = null;
@@ skipping 27 matching lines @@
           if (!types.split('|').contains('Null')) {
             return true;
           }
         }
       }
     }
     return false;
   }
 
   JS.Expression notNull(Expression expr) {
-    var type = rules.getStaticType(expr);
-    if (rules.isNonNullableType(type) || _isNonNullableExpression(expr)) {
+    if (_isNonNullableExpression(expr)) {
       return _visit(expr);
     } else {
       return js.call('dart.notNull(#)', _visit(expr));
     }
   }
 
   @override
   JS.Expression visitBinaryExpression(BinaryExpression node) {
     var op = node.operator;
     var left = node.leftOperand;
@@ skipping 62 matching lines @@
   // TODO(jmesserly, vsm): Refactor this logic.
   SimpleIdentifier _createTemporary(String name, DartType type) {
     var id =
         new SimpleIdentifier(new StringToken(TokenType.IDENTIFIER, name, 0));
     id.staticElement = new LocalVariableElementImpl.forNode(id);
     id.staticType = type;
     return id;
   }
 
   JS.Expression _emitPostfixIncrement(Expression expr, Token op) {
-    var tmp = _createTemporary('\$tmp', rules.getStaticType(expr));
+    var type = rules.getStaticType(expr);
+    assert(type != null);
+    var tmp = _createTemporary('\$tmp', type);
 
     // Increment and write
     var one = AstBuilder.integerLiteral(1);
+    one.staticType = rules.provider.intType;
     var increment = AstBuilder.binaryExpression(tmp, op.lexeme[0], one);
+    increment.staticType = type;
     var write = _emitAssignment(expr, increment);
 
     var bindThis = _maybeBindThis(expr);
     return js.call("((#) => (#, #))$bindThis(#)", [
       tmp.name,
       write,
       _visit(tmp),
       _visit(expr)
     ]);
   }
 
   @override
   JS.Expression visitPostfixExpression(PostfixExpression node) {
     var op = node.operator;
     var expr = node.operand;
 
+    if (node.parent is Statement) {
+      // Prefix code is simpler.  If the expr result isn't used, fall to that.
+      return _emitPrefixExpression(op, expr);
+    }
+
     var dispatchType = rules.getStaticType(expr);
     if (unaryOperationIsPrimitive(dispatchType)) {
-      // TODO(vsm): When do Dart ops not map to JS?
-      return js.call('#$op', notNull(expr));
-    } else {
-      assert(op.lexeme == '++' || op.lexeme == '--');
-      return _emitPostfixIncrement(expr, op);
+      if (_isNonNullableExpression(expr)) {
+        return js.call('#$op', _visit(expr));
+      }
     }
+
+    assert(op.lexeme == '++' || op.lexeme == '--');
+    return _emitPostfixIncrement(expr, op);
   }
 
   JS.Expression _emitPrefixIncrement(Token op, Expression expr) {
     var one = AstBuilder.integerLiteral(1);
     var increment = AstBuilder.binaryExpression(expr, op.lexeme[0], one);
     return _emitAssignment(expr, increment);
   }
 
   @override
   JS.Expression visitPrefixExpression(PrefixExpression node) {
     var op = node.operator;

[Jennifer Messerly, 2015/03/04 20:08:47]

I might be tempted to get rid of these assignments now

[vsm, 2015/03/04 20:55:56]

Done.

     var expr = node.operand;
+    return _emitPrefixExpression(op, expr);
+  }
 
+  JS.Expression _emitPrefixExpression(Token op, Expression expr) {
     var dispatchType = rules.getStaticType(expr);
     if (unaryOperationIsPrimitive(dispatchType)) {
-      // TODO(vsm): When do Dart ops not map to JS?
-      return js.call('$op#', notNull(expr));
+      if (_isNonNullableExpression(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 mathop = op.lexeme[0];
+        return js.call('# = # $mathop 1', [_visit(expr), notNull(expr)]);
+      } else {
+        return js.call('$op#', notNull(expr));
+      }
     } else {
-      // Increment or decrement requires expansion
+      // Increment or decrement requires expansion.
       if (op.lexeme == '++' || op.lexeme == '--') {
         return _emitPrefixIncrement(op, expr);
       }
     }
 
     // Call the operator
     var opString = _jsMemberName(op.lexeme, unary: true);
     if (rules.isDynamicTarget(expr)) {
       // dynamic dispatch
       return js.call('dart.dunary(#, #)', [opString, _visit(expr)]);
@@ skipping 764 matching lines @@
 
     // TODO(jmesserly): in many cases marking the end will be unncessary.
     printer.mark(_location(node.end));
   }
 
   String _getIdentifier(AstNode node) {
     if (node is SimpleIdentifier) return node.name;
     return null;
   }
 }