Temporary variables should not be accessible from user code

[taku0]

Running the following code result in a TypeError "Cannot call method 'push' of null" since the compiler-generated temporary variable "_results" is overriden.

x = ((_results = null; i) for i in [1, 2, 3])

The compiler should generate a fresh variable that is never accessed from user code.
To accomplish this, we might use 2-pass strategy: one for scan all variables used in the code, and one for generating JavaScript code.

Another storategy is reserving all words starting with special prefix (e.g. '__coffee_script_compiler_generated') and prohibiting users from using them as variable names.

[breckinloggins]

Potential issue with this: are there valid use cases for meta-programming wizardry that need to use or redefine these variables inside of CoffeeScript code?

[michaelficarra]

@breckinloggins: these variables are implementation details that aren't (supposed to be) exposed to the coffeescript user, so no

[breckinloggins]

I like the strategy of checking for a special prefix. I assume it would be faster than another pass.

__csvar_ maybe?

[michaelficarra]

@breckinloggins: That would make the output JS considerably less readable in my opinion. It's an option of course, but one I'd like to avoid.

[taku0]

Another option: collecting all identifiers in lexer or parser.
Looping over token streams might faster than complex recursive traversals over AST, though this is my conjecture.

Yet anothor option is an optimistic way: just generate codes as before. When the compiler detect a name crush, record the crushed name, throw a exception, and retry generating the codes from beginning.
This would be faster than the 2-pass storategy since retrying would not happen except pathological cases.
Moreover, modification of the compiler will be smaller: adding a field in Scope, checking name crushes inside compileNode of Literal, and catching exceptions to retry on the top level. All other nodes are not changed.

[breckinloggins]

@taku0 Your second option has architecture and memory advantages since we're not having the lexer or parser keep track of extra information just for the sake of this somewhat pathological case.

I assume when you say "checking name crushes", you're just referring to any mention of a name that's been declared in Scope's tempVars anywhere up the current scope chain?

Let's say someone declares _var3 and one level up the scope chain we find our own _var3, do we have to regenerate from the root or just from that scope chain one level up?

The other thing that would be nice is to fix:

declaredVariables: ->
    realVars = []
    tempVars = []
    for v in @variables when v.type is 'var'
        (if v.name.charAt(0) is '_' then tempVars else realVars).push v.name
    realVars.sort().concat tempVars.sort()

Here the code is assuming that anything that starts with a _ is a CoffeeScript-generated temp var. But that's not necessarily true. This has the affect or reordering the declarations in Block::compileWithDeclarations and unintentionally grouping a user's real var among the temp vars. This is probably not a big deal, but it would be nice to fix while we're at it.

Also, the way the compiler handes utility functions is different from variables. It treats them as reserved words and doesn't let you use them in code. I suppose we could do it this way for temp vars except it would be much less friendly because you couldn't predict in advance what would be reserved and what would not be reserved.

[taku0]

@breckinloggins

I assume when you say "checking name crushes", you're just referring to any mention of a name that's been declared in Scope's tempVars anywhere up the current scope chain?
Let's say someone declares _var3 and one level up the scope chain we find our own _var3, do we have to regenerate from the root or just from that scope chain one level up?

I meant to scan declared names up to the root of scope chain, not just from that scope chain one level up (I have confused "clash" with "crush". "name clashing" is the correct words). On the other hand, I noticed now that it is enough to regenerate from one level up if we declare the variable in the inner scope. In the example below, inserting var _i; at the beginning of the compiled code of f will avoid name clashing:

for x in [1, 2, 3]
  f = ->
    _i = 0
  f()

Declareing the variable in the inner scope would be safe since the temporary variables are used only in the declared scope but not inner scopes, with respect to the current compiler.