Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

loop variables also global if name in global #643

Closed
onnlucky opened this issue Aug 24, 2010 · 5 comments
Closed

loop variables also global if name in global #643

onnlucky opened this issue Aug 24, 2010 · 5 comments
Labels
bug fixed

Comments

@onnlucky
Copy link 

rules = []
rule = (name, run) -> rules.push([name, run])

rule "test", -> print "ok"

run = () ->
  for rule in rules
    console.log rule.name
run()

rule "more", -> print "bitten"

Looks perfectly normal, but does not work!

The question is, should the for rule in rules make rule a local? I doubt the current behavior is expected. Just spend an hour debugging same issue, but just a lot less obvious ...

I know this is related to the issue 238 that I raised earlier: http://github.com/jashkenas/coffee-script/issues/issue/238 .

However this one is even less obvious. And if we would rewrite it:

rules.forEach (rule) ->
  console.log rule.name

It would work. Nobody expects the for rule in rules to be different I guess?
@jashkenas
Copy link
Owner

Here's a patch that forces loop parameters to declare themselves in the closest possible scope:

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

I'm afraid that it doesn't fix 100% of the cases, because if there's no difference in scope between the variable and the loop, it'll still override it -- that's simply JavaScript's behavior that we aren't going to change... But it does fix your case, and others that are similar to it.

@onnlucky
Copy link
Author

I think it will be good enough, because re-using the same variable locally is asking for trouble, and is at least somewhat obvious.

great work; for me coffeescript is officially the most productive language

@LeifW
Copy link

LeifW commented Jul 18, 2011

I ran into this with something like:
i = "fish"
squares = (i*i for i in [1..10])
console.log(i) // gives 11

There's nothing to be done for that?

@LeifW
Copy link

LeifW commented Jul 18, 2011

Actually, the issue's plain to see right on the CoffeeScript splash page:
In the example:
cubes = (math.cube num for num in list)
num gets declared at the very top of the generated JavaScript, and thus is in scope with a value of 5 after running that line. Why can't num be declared in that function that gets generated for the comprehension?
That's ought to scare people off from using comprehensions in this language, when they can just use Array.map and avoid polluting the top scope with counter variables.

@geraldalewis geraldalewis mentioned this issue Apr 23, 2012
Closed
@jashkenas
Copy link
Owner

Looks like this change was a mistake ... looking at reverting.

jashkenas added a commit that referenced this issue Apr 24, 2012
@jashkenas
Fixes #2273, reverts #643 -- no special variable treatment for loop v…
afdcdcf 
…ariables.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jashkenas @LeifW @onnlucky