Inconsistent index variable values after loop/comprehension

[TrevorBurnham]

Now that index variables are definitely preserved from loops (issue 118), I think it behooves us to correct this behavior:

console.log i for i in [1, 2, 3]
console.log i # 3
(-> 'func') i for i in [1..3]
console.log i # 3
console.log i for i in [1..3]
console.log i # 4

I'd call that last case a bug; the fix is to undo one step at the end of any for loop where the index variable is named.

Also, i should always have the last step in case of a break or return, but i's value is never set at all if there's a closure and a break or return anywhere!

i = 0
for i in [1..3]
  -> 'func'
  break if false
console.log i # 0

I've added corresponding test cases to test_comprehensions.coffee and patched nodes.coffee. Yes, the hairiest method in the CoffeeScript compiler just got slightly hairier...

[zmthy]

What's with all the two line spaces in the generated JS?

[TrevorBurnham]

Sorry; wasn't looking at the generated JS, just seeing if the tests passed. I pushed a fix: http://github.com/TrevorBurnham/coffee-script/commit/33ac70aec349eb91151f5fe0a1a562120eabf456

[zmthy]

The pure statement thing was my fault, I tidied that up a bit.

[satyr]

Is this for range case only?
$ bin/coffee -bpe 'i for a, i in [1, 2]'
var _len, _ref, a, i;
for (i = 0, _len = (_ref = [1, 2]).length; i < _len; i++) {
a = _ref[i];
i;
}

[zmthy]

Yeah. i will be the length of the array after that loop, which is really the expected behaviour.

[satyr]

Really? I'd expect identical results from them:

$ bin/coffee -e 'i for i in [0..1]; console.log i'
1

$ bin/coffee -e 'i for _, i in [0, 1]; console.log i'
2

And what about these?
$ bin/coffee -e 'break for i in [0..1]; console.log i'
-1

$ bin/coffee -e 'break for i in [0, 1]; console.log i'
0

[zmthy]

It's implied that the for .. in loop translates directly to the equivalent of for (i = 0; i < arr.length; ++i), so I believe that makes the extra value of i expected. The range loop (which should hopefully not use the in keyword pretty soon) has less of this implication, and more of 'go to here and stop'. Once we have an implementation of to, there won't be such an expectation on identical values.

That -1 result looks like a bug, though. I would expect them to both be 0.

[satyr]

We've got variable step, so it's more complicated.
$ bin/coffee -e 'i for _, i in [0, 1, 2] by 2; console.log i'
4
Do we really need this anyway? Are we encouraging the reuse of loop variables?
I'd rather kill the codeInBody hack and this one altoghter.

[zmthy]

Yeah, I'd rather kill the magic as well, in exchange for an easier way to boot up a new scope without having to use new -> or (-> ...)(). Being able to write something like:
for i in foo then scope
val = i
setTimeout (-> puts val), 100
would be much better, because there isn't any implicit magic.