Fixes #2525, #1187, #1208, #1758, and many more -- allow looping over an array downwards

Author: jashkenas

lib/coffee-script/nodes.js

@@ -2566,7 +2566,7 @@
     For.prototype.children = ['body', 'source', 'guard', 'step'];
 
     For.prototype.compileNode = function(o) {
-      var body, defPart, forPart, forVarPart, guardPart, idt1, index, ivar, kvar, kvarAssign, lastJumps, lvar, name, namePart, ref, resultPart, returnResult, rvar, scope, source, step, stepPart, stepVar, svar, varPart, _ref2, _ref3;
+      var body, compare, compareDown, declare, declareDown, defPart, down, forPart, guardPart, idt1, increment, index, ivar, kvar, kvarAssign, lastJumps, lvar, name, namePart, ref, resultPart, returnResult, rvar, scope, source, step, stepNum, stepVar, svar, varPart, _ref2, _ref3;
       body = Block.wrap([this.body]);
       lastJumps = (_ref2 = last(body.expressions)) != null ? _ref2.jumps() : void 0;
       if (lastJumps && lastJumps instanceof Return) {
@@ -2590,6 +2590,7 @@
       kvarAssign = kvar !== ivar ? "" + kvar + " = " : "";
       if (this.step && !this.range) {
         _ref3 = this.step.cache(o, LEVEL_LIST), step = _ref3[0], stepVar = _ref3[1];
+        stepNum = stepVar.match(SIMPLENUM);
       }
       if (this.pattern) {
         name = ivar;
@@ -2607,20 +2608,38 @@
       } else {
         svar = this.source.compile(o, LEVEL_LIST);
         if ((name || this.own) && !IDENTIFIER.test(svar)) {
-          defPart = "" + this.tab + (ref = scope.freeVariable('ref')) + " = " + svar + ";\n";
+          defPart += "" + this.tab + (ref = scope.freeVariable('ref')) + " = " + svar + ";\n";
           svar = ref;
         }
         if (name && !this.pattern) {
           namePart = "" + name + " = " + svar + "[" + kvar + "]";
         }
         if (!this.object) {
-          lvar = scope.freeVariable('len');
-          forVarPart = "" + kvarAssign + ivar + " = 0, " + lvar + " = " + svar + ".length";
           if (step !== stepVar) {
-            forVarPart += ", " + step;
+            defPart += "" + this.tab + step + ";\n";
           }
-          stepPart = "" + kvarAssign + (this.step ? "" + ivar + " += " + stepVar : (kvar !== ivar ? "++" + ivar : "" + ivar + "++"));
-          forPart = "" + forVarPart + "; " + ivar + " < " + lvar + "; " + stepPart;
+          if (!(this.step && stepNum && (down = +stepNum < 0))) {
+            lvar = scope.freeVariable('len');
+          }
+          declare = "" + kvarAssign + ivar + " = 0, " + lvar + " = " + svar + ".length";
+          declareDown = "" + kvarAssign + ivar + " = " + svar + ".length - 1";
+          compare = "" + ivar + " < " + lvar;
+          compareDown = "" + ivar + " >= 0";
+          if (this.step) {
+            if (stepNum) {
+              if (down) {
+                compare = compareDown;
+                declare = declareDown;
+              }
+            } else {
+              compare = "" + stepVar + " > 0 ? " + compare + " : " + compareDown;
+              declare = "(" + stepVar + " > 0 ? (" + declare + ") : " + declareDown + ")";
+            }
+            increment = "" + ivar + " += " + stepVar;
+          } else {
+            increment = "" + (kvar !== ivar ? "++" + ivar : "" + ivar + "++");
+          }
+          forPart = "" + declare + "; " + compare + "; " + kvarAssign + increment;
         }
       }
       if (this.returns) {

src/nodes.coffee

@@ -1741,9 +1741,9 @@ exports.For = class For extends While
     ivar      = (@object and index) or scope.freeVariable 'i'
     kvar      = (@range and name) or index or ivar
     kvarAssign = if kvar isnt ivar then "#{kvar} = " else ""
-    # the `_by` variable is created twice in `Range`s if we don't prevent it from being declared here
     if @step and not @range
       [step, stepVar] = @step.cache o, LEVEL_LIST
+      stepNum = stepVar.match SIMPLENUM
     name      = ivar if @pattern
     varPart   = ''
     guardPart = ''
@@ -1754,16 +1754,29 @@ exports.For = class For extends While
     else
       svar    = @source.compile o, LEVEL_LIST
       if (name or @own) and not IDENTIFIER.test svar
-        defPart    = "#{@tab}#{ref = scope.freeVariable 'ref'} = #{svar};\n"
+        defPart    += "#{@tab}#{ref = scope.freeVariable 'ref'} = #{svar};\n"
         svar       = ref
       if name and not @pattern
         namePart   = "#{name} = #{svar}[#{kvar}]"
-      unless @object
-        lvar       = scope.freeVariable 'len'
-        forVarPart = "#{kvarAssign}#{ivar} = 0, #{lvar} = #{svar}.length"
-        forVarPart += ", #{step}" if step isnt stepVar
-        stepPart   = "#{kvarAssign}#{if @step then "#{ivar} += #{stepVar}" else (if kvar isnt ivar then "++#{ivar}" else "#{ivar}++")}"
-        forPart    = "#{forVarPart}; #{ivar} < #{lvar}; #{stepPart}"
+      if not @object
+        defPart += "#{@tab}#{step};\n" if step isnt stepVar
+        lvar = scope.freeVariable 'len' unless @step and stepNum and down = (+stepNum < 0)
+        declare = "#{kvarAssign}#{ivar} = 0, #{lvar} = #{svar}.length"
+        declareDown = "#{kvarAssign}#{ivar} = #{svar}.length - 1"
+        compare = "#{ivar} < #{lvar}"
+        compareDown = "#{ivar} >= 0"
+        if @step
+          if stepNum
+            if down
+              compare = compareDown
+              declare = declareDown
+          else
+            compare = "#{stepVar} > 0 ? #{compare} : #{compareDown}"
+            declare = "(#{stepVar} > 0 ? (#{declare}) : #{declareDown})"
+          increment = "#{ivar} += #{stepVar}"
+        else
+          increment = "#{if kvar isnt ivar then "++#{ivar}" else "#{ivar}++"}"
+        forPart  = "#{declare}; #{compare}; #{kvarAssign}#{increment}"
     if @returns
       resultPart   = "#{@tab}#{rvar} = [];\n"
       returnResult = "\n#{@tab}return #{rvar};"

test/comprehensions.coffee

@@ -244,8 +244,8 @@ test "Optimized range comprehensions.", ->
 
   exxes = ('x' for [0...10])
   ok exxes.join(' ') is 'x x x x x x x x x x'
-  
-  
+
+
 test "Loop variables should be able to reference outer variables", ->
   outer = 1
   do ->
@@ -411,7 +411,8 @@ test "#1326: `by` value is uncached", ->
   rangeCompileSimple = []
 
   #exercises For.compile
-  for v,i in a by f() then forCompile.push i
+  for v, i in a by f()
+    forCompile.push i
 
   #exercises Range.compileSimple
   rangeCompileSimple = (i for i in [0..2] by g())
@@ -491,7 +492,7 @@ test "#2007: Return object literal from comprehension", ->
   eq 2, y.length
   eq 1, y[0].x
   eq 0, y[1].x
-  
+
 test "#2274: Allow @values as loop variables", ->
   obj = {
     item: null
@@ -502,3 +503,41 @@ test "#2274: Allow @values as loop variables", ->
   eq obj.item, null
   obj.method()
   eq obj.item, 3
+
+test "#2525, #1187, #1208, #1758, looping over an array forwards", ->
+  list = [0, 1, 2, 3, 4]
+
+  ident = (x) -> x
+
+  arrayEq (i for i in list), list
+
+  arrayEq (index for i, index in list), list
+
+  arrayEq (i for i in list by 1), list
+
+  arrayEq (i for i in list by ident 1), list
+
+  arrayEq (i for i in list by ident(1) * 2), [0, 2, 4]
+
+  arrayEq (index for i, index in list by ident(1) * 2), [0, 2, 4]
+
+test "#2525, #1187, #1208, #1758, looping over an array backwards", ->
+  list = [0, 1, 2, 3, 4]
+  backwards = [4, 3, 2, 1, 0]
+
+  ident = (x) -> x
+
+  arrayEq (i for i in list by -1), backwards
+
+  arrayEq (index for i, index in list by -1), backwards
+
+  arrayEq (i for i in list by ident -1), backwards
+
+  arrayEq (i for i in list by ident(-1) * 2), [4, 2, 0]
+
+  arrayEq (index for i, index in list by ident(-1) * 2), [4, 2, 0]
+
+
+
+
+