catch does not create its own lexical environment
michaelficarra opened this issue · 11 comments
... which causes us to leak globals:
$ coffee -bsp
try fn() catch e then
e = 0
try {
fn();
} catch (e) {
}
e = 0;
e
should be let-scoped to the catch block (see ES5 §12.14). The e
outside should be seen as the first assignment to e
and therefore auto-declared at the top of its closest containing scope.
Ah, I couldn't find that. @jashkenas: what should we do here? I say fuck JScript.
What's the argument for which would be more useful?
It depends: either decision is a cognitive load we are forcing upon our users. If we keep the current semantics, CS is internally consistent, but inconsistent with standard JS. If we make the change, we're adopting the odd catch scoping semantics from JS, and forcing our users (assuming they all care about IE) to account for the JScript behaviour.
Potential errors with the current implementation: accidentally leaking globals. Potential errors with the proposed implementation: IE executions will potentially overwrite a variable that was assumed to be shadowed. I believe the catch variable may also leak to its containing scope if undeclared there, but I don't have a windows machine to test it on.
I say we go with my proposed solution. The negative behaviour with that solution will be much less commonly encountered.
Would it be possible to mangle the variable name in the catch block to satisfy both conditions?
Better yet, maybe
try fn() catch e then
e = 0
should compile to
var e;
try {
fn();
} catch (_1) {
e = _1;
}
e = 0;
to maintain internal consistency, not break JScript, &c.
var e; ... } catch (_1) { e = _1;
Yep, that would solve the conflict. It would further simplify our scope rules (function scope all the way), and might actually be useful as we often see the pattern catch e then err = e
.
@erisdiscord: That won't scope e
to the catch block, mimicking JS semantics. Though it would make CS scoping semantics more consistent.
edit: @satyr beat me to it.
+1 for var
, i'd ran into this issue already.
@michaelficarra yep, I think it's a worthwhile break from "correct" JavaScript semantics, both for consistency and for the sake of that common pattern @satyr mentioned.
Sounds good to me.
Take a look at the above patch/diff, and let me know if that doesn't look alright.