Generating functions from comprehensions doesn't close over variables..

[foot]

TEST: [f1, f2]: (() -> n) for n in [1,2] ok f1() is 1 # Fails (f1() is 2) ok f2() is 2 # Passes

Coffee generates:

(function(){
  var _a, f1, f2;
  // Generating functions from a comprehension
  _a = (function() {
    var _b, _c, _d, _e, n;
    _b = []; _d = [1, 2];
    for (_c = 0, _e = _d.length; _c < _e; _c++) {
      n = _d[_c];
      _b.push((function() {
        return n;
      }));
    }
    return _b;
  })();
  f1 = _a[0];
  f2 = _a[1];
})();

but should generate something like this probably?

(function(){
  var _a, f1, f2;
  // Generating functions from a comprehension
  _a = (function() {
    var _b, _c, _d, _e;
    _b = []; _d = [1, 2];
    for (_c = 0, _e = _d.length; _c < _e; _c++) {
      (function(){
        var n = _d[_c];
        _b.push((function() {
          return n;
        }));
      })();
    }
    return _b;
  })();
  f1 = _a[0];
  f2 = _a[1];
})();

Ta

[jashkenas]

I'm afraid that this is a feature... Comprehensions are supposed to have the same speed and roughly the same semantics as the equivalent JavaScript for loop. So, like for loops, variables will not be closed over between iterations.

If you'd like them to, I'd recommend using forEach or map, if you're on Node.js or an ECMA5. Otherwise, you can save the variable by using an external function:

returns: (x) -> -> x
[f1, f2]: returns n for n in [1, 2]

[foot]

Ahhh bummer that sucks!

If it ever comes up again I'm totally +1 in favour in breaking with semantics for this case! For perf.. could test for anon function at compile time and only wrap in this case? Though messy semantics grrr.

But thanks for the response! I think using map makes the code even clearer anyway. [rsin, rcos]: [Math.sin, Math.cos].map((f) -> (a) -> Math.round(r * f(a))) Cheers

[jashkenas]

That's nice, or you can keep it simple:

rsin: (n) -> Math.round r * Math.sin n
rcos: (n) -> Math.round r * Math.cos n

But yep, performance is the main thing here. It actually would be possible to test for function literals in the body of the loop... Do you think that would be desirable in all circumstances?

[tcr]

"returns (variable at time of invocation)" to create an implicit closure isn't that bad of a feature. Is there any comparable keyword in any language?

Edit: Or something like:

nodes: for k, v of hash
    closure -> "Key: " + k + ", Value: " + v
alert(node()) for node in nodes

[hen-x]

I think closing over loop variables is a great idea, and testing for function literals in the loop body would be a solid way to decide whether or not to capture them. Is there any situation where one might define a function inside a loop, but reasonably rely on the loop variables not being captured?

[jashkenas]

Alright folks -- I think we have this one licked. Take a close look at this test case, because I want to make sure we have the right semantics here:

# Ensure that the closure wrapper preserves local variables.
obj: {}

for method in ['one', 'two', 'three']
  obj[method]: ->
    "I'm " + method

ok obj.one()   is "I'm one"
ok obj.two()   is "I'm two"
ok obj.three() is "I'm three"

# Even when referenced in the filter.
list: ['one', 'two', 'three']

methods: for num in list when num isnt 'two'
  -> num

ok methods.length is 2
ok methods[0]() is 'one'
ok methods[1]() is 'three'

Here's the patch (includes some formatting):

http://github.com/jashkenas/coffee-script/commit/b0a45e5b93d0534bc160d218fb80900609fd1034

Closing the ticket.

[foot]

Cool! Awesome stuff.

I couldn't think of or find any situations where you wouldn't want this behavior in the end. I'm still in support of the patch.

Did come across some interesting links though:

• Discussing block-scope and potential design choices and benefits behind it: http://lambda-the-ultimate.org/node/1775 They mention python behaves similarly which was a shock to me, so I checked it and...

  In [1]: f1, f2 = [ lambda: x for x in (1,2) ]

  In [2]: assert(f1() is 1)

  AssertionError Traceback (most recent call last)

Wowo! And in Firefox 3.6.3:

<script type="application/javascript;version=1.8">
  var fs = [ (function(){ return n; }) for each (n in [1,2]) ];
  console.log(fs[0]()); // 2
  console.log(fs[1]()); // 2
</script> 

So I guess this patch is more of a feature than a bug fix. (cool feature though.)

• LtU led to this link discussing potential bugs that can be introduced using the anonymous function wrapper, interesting, but doesn't apply in this case. http://wiki.ecmascript.org/doku.php?id=discussion:block_expressions#rationale

[hen-x]

@jashkenas: that test case definitely looks right.

@foot : thanks for the illuminating language-design links! As a spectator on this issue tracker, it's great being able to place this stuff in context.

[TrevorBurnham]

I've raised an issue to discuss the removal of this feature, which has proven problematic: Issue 959.