Merge pull request jashkenas#3786 from lydell/loop-safety

Fix jashkenas#3778: Make for loops more consistent

Author: jashkenas

lib/coffee-script/nodes.js

@@ -102,14 +102,15 @@ exports.Base = class Base
   # If `level` is passed, then returns `[val, ref]`, where `val` is the compiled value, and `ref`
   # is the compiled reference. If `level` is not passed, this returns `[val, ref]` where
   # the two values are raw nodes which have not been compiled.
-  cache: (o, level, reused) ->
-    unless @isComplex()
-      ref = if level then @compileToFragments o, level else this
-      [ref, ref]
-    else
-      ref = new Literal reused or o.scope.freeVariable 'ref'
+  cache: (o, level, isComplex) ->
+    complex = if isComplex? then isComplex this else @isComplex()
+    if complex
+      ref = new Literal o.scope.freeVariable 'ref'
       sub = new Assign ref, this
       if level then [sub.compileToFragments(o, level), [@makeCode(ref.value)]] else [sub, ref]
+    else
+      ref = if level then @compileToFragments o, level else this
+      [ref, ref]
 
   cacheToCodeFragments: (cacheValues) ->
     [fragmentsToText(cacheValues[0]), fragmentsToText(cacheValues[1])]
@@ -793,9 +794,10 @@ exports.Range = class Range extends Base
   # But only if they need to be cached to avoid double evaluation.
   compileVariables: (o) ->
     o = merge o, top: true
-    [@fromC, @fromVar]  =  @cacheToCodeFragments @from.cache o, LEVEL_LIST
-    [@toC, @toVar]      =  @cacheToCodeFragments @to.cache o, LEVEL_LIST
-    [@step, @stepVar]   =  @cacheToCodeFragments step.cache o, LEVEL_LIST if step = del o, 'step'
+    isComplex = del o, 'isComplex'
+    [@fromC, @fromVar]  =  @cacheToCodeFragments @from.cache o, LEVEL_LIST, isComplex
+    [@toC, @toVar]      =  @cacheToCodeFragments @to.cache o, LEVEL_LIST, isComplex
+    [@step, @stepVar]   =  @cacheToCodeFragments step.cache o, LEVEL_LIST, isComplex if step = del o, 'step'
     [@fromNum, @toNum]  = [@fromVar.match(NUMBER), @toVar.match(NUMBER)]
     @stepNum            = @stepVar.match(NUMBER) if @stepVar
 
@@ -1971,15 +1973,16 @@ exports.For = class For extends While
     kvar      = (@range and name) or index or ivar
     kvarAssign = if kvar isnt ivar then "#{kvar} = " else ""
     if @step and not @range
-      [step, stepVar] = @cacheToCodeFragments @step.cache o, LEVEL_LIST
+      [step, stepVar] = @cacheToCodeFragments @step.cache o, LEVEL_LIST, isComplexOrAssignable
       stepNum = stepVar.match NUMBER
     name      = ivar if @pattern
     varPart   = ''
     guardPart = ''
     defPart   = ''
     idt1      = @tab + TAB
     if @range
-      forPartFragments = source.compileToFragments merge(o, {index: ivar, name, @step})
+      forPartFragments = source.compileToFragments merge o,
+        {index: ivar, name, @step, isComplex: isComplexOrAssignable}
     else
       svar    = @source.compile o, LEVEL_LIST
       if (name or @own) and not IDENTIFIER.test svar
@@ -2291,6 +2294,8 @@ isLiteralThis = (node) ->
     (node instanceof Code and node.bound) or
     (node instanceof Call and node.isSuper)
 
+isComplexOrAssignable = (node) -> node.isComplex() or node.isAssignable?()
+
 # Unfold a node's child if soak, then tuck the node under created `If`
 unfoldSoak = (o, parent, name) ->
   return unless ifn = parent[name].unfoldSoak o

src/nodes.coffee

@@ -555,3 +555,14 @@ test "splats in destructuring in comprehensions", ->
 test "#156: expansion in destructuring in comprehensions", ->
   list = [[0, 1, 2], [2, 3, 4], [4, 5, 6]]
   arrayEq (last for [..., last] in list), [2, 4, 6]
+
+test "#3778: Consistently always cache for loop range boundaries and steps, even
+      if they are simple identifiers", ->
+  a = 1; arrayEq [1, 2, 3], (for n in [1, 2, 3] by  a then a = 4; n)
+  a = 1; arrayEq [1, 2, 3], (for n in [1, 2, 3] by +a then a = 4; n)
+  a = 1; arrayEq [1, 2, 3], (for n in [a..3]          then a = 4; n)
+  a = 1; arrayEq [1, 2, 3], (for n in [+a..3]         then a = 4; n)
+  a = 3; arrayEq [1, 2, 3], (for n in [1..a]          then a = 4; n)
+  a = 3; arrayEq [1, 2, 3], (for n in [1..+a]         then a = 4; n)
+  a = 1; arrayEq [1, 2, 3], (for n in [1..3] by  a    then a = 4; n)
+  a = 1; arrayEq [1, 2, 3], (for n in [1..3] by +a    then a = 4; n)