Skip to content

Commit

Permalink
Fixes #284 - Loops/comprehensions over decreasing ranges don't work (#…
Browse files Browse the repository at this point in the history
…365)

* Fixes #284 - Loops/comprehensions over decreasing ranges don't work

* - Fix code formatting
- Add more unit tests for decreasing numeric ranges
  • Loading branch information
basicer authored and michaelficarra committed Apr 13, 2017
1 parent ad91003 commit a122b37
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 10 deletions.
39 changes: 35 additions & 4 deletions lib/compiler.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 23 additions & 6 deletions src/compiler.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,21 @@ generateSoak = do ->
[tests, e] = fn node
new CS.Conditional (foldl1 tests, (memo, t) -> new CS.LogicalAndOp memo, t), e

extractNumber = (what) ->
return what.data if what.instanceof CS.Int
return false unless what.instanceof CS.UnaryNegateOp
return false unless what.expression.instanceof CS.Int
return 0 - what.expression.data

extractStaticRange = (range) ->
return undefined unless range.instanceof CS.Range
left = extractNumber range.left
right = extractNumber range.right

return undefined if left == false
return undefined if right == false

return [left, right]

helperNames = {}
helpers =
Expand Down Expand Up @@ -392,7 +407,6 @@ for own h, fn of inlineHelpers
helpers[h] = fn



class exports.Compiler

@compile = => (new this).compile arguments...
Expand Down Expand Up @@ -449,17 +463,17 @@ class exports.Compiler
block = forceBlock body
block.body.push stmt helpers.undef() unless block.body.length

numericRange = extractStaticRange(@target)
increment =
if @step? and not ((@step.instanceof CS.Int) and @step.data is 1)
(x) -> new JS.AssignmentExpression '+=', x, step
else if numericRange? and numericRange[1] < numericRange[0]
(x) -> new JS.UpdateExpression '--', yes, x
else
(x) -> new JS.UpdateExpression '++', yes, x

# optimise loops over static, integral ranges
if (@target.instanceof CS.Range) and
# TODO: extract this test to some "static, integral range" helper
((@target.left.instanceof CS.Int) or ((@target.left.instanceof CS.UnaryNegateOp) and @target.left.expression.instanceof CS.Int)) and
((@target.right.instanceof CS.Int) or ((@target.right.instanceof CS.UnaryNegateOp) and @target.right.expression.instanceof CS.Int))
if numericRange?
varDeclaration = new JS.VariableDeclaration 'var', [new JS.VariableDeclarator i, compile @target.left]
update = increment i
if @filter?
Expand All @@ -471,7 +485,10 @@ class exports.Compiler
block.body.unshift stmt new JS.AssignmentExpression '=', keyAssignee, k
if valAssignee?
block.body.unshift stmt new JS.AssignmentExpression '=', valAssignee, i
op = if @target.isInclusive then '<=' else '<'
if numericRange[1] > numericRange[0]
op = if @target.isInclusive then '<=' else '<'
else
op = if @target.isInclusive then '>=' else '>'
return new JS.ForStatement varDeclaration, (new JS.BinaryExpression op, i, compile @target.right), update, block

e = if needsCaching @target then genSym 'cache' else target
Expand Down
14 changes: 14 additions & 0 deletions test/comprehensions.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,17 @@ suite 'Comprehensions', ->
arrayEq [0, 3, 6], (k for v, k in [1..7] by 3)
arrayEq [0, 0, 0], (0 for in list by 3)
arrayEq [0, 0, 0], (0 for in [1..7] by 3)

test '#284: loops/comprehensions over decreasing ranges don\'t work', ->
a = 2
b = -2
arrayEq [5, 4, 3, 2, 1], (n for n in [5..1])
arrayEq [5, 4, 3, 2, 1, 0, -1, -2, -3, -4, -5], (n for n in [5..-5])
arrayEq [2, 1, 0, -1, -2], (n for n in [a..b])
arrayEq [2, 1, 0, -1, -2], (n for n in [a..-2])
arrayEq [2, 1, 0, -1, -2], (n for n in [2..b])

arrayEq [5, 4, 3, 2], (n for n in [5...1])
arrayEq [2, 1, 0, -1], (n for n in [a...b])
arrayEq [2, 1, 0, -1], (n for n in [a...-2])
arrayEq [2, 1, 0, -1], (n for n in [2...b])

0 comments on commit a122b37

Please sign in to comment.