Skip to content

Commit

Permalink
Ticket #423. When functions are generated within comprehensions ... t…
Browse files Browse the repository at this point in the history 
…he comprehensions should close over the element instead of sharing it.
  • Loading branch information
@jashkenas
jashkenas committed Jun 14, 2010
1 parent 6f91331 commit b0a45e5
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 66 deletions.
63 changes: 33 additions & 30 deletions lib/command.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,41 +58,44 @@
// compile them. If a directory is passed, recursively compile all
// '.coffee' extension source files in it and all subdirectories.
compileScripts = function() {
var _b, _c, _d, _e, base, compile, source;
var _b, _c, _d, _e;
_b = []; _d = sources;
for (_c = 0, _e = _d.length; _c < _e; _c++) {
source = _d[_c];
_b.push((function() {
base = source;
compile = function(source, topLevel) {
return path.exists(source, function(exists) {
if (!(exists)) {
throw new Error(("File not found: " + source));
}
return fs.stat(source, function(err, stats) {
if (stats.isDirectory()) {
return fs.readdir(source, function(err, files) {
var _f, _g, _h, _i, file;
_f = []; _h = files;
for (_g = 0, _i = _h.length; _g < _i; _g++) {
file = _h[_g];
_f.push(compile(path.join(source, file)));
(function() {
var base, compile;
var source = _d[_c];
return _b.push((function() {
base = source;
compile = function(source, topLevel) {
return path.exists(source, function(exists) {
if (!(exists)) {
throw new Error(("File not found: " + source));
}
return fs.stat(source, function(err, stats) {
if (stats.isDirectory()) {
return fs.readdir(source, function(err, files) {
var _f, _g, _h, _i, file;
_f = []; _h = files;
for (_g = 0, _i = _h.length; _g < _i; _g++) {
file = _h[_g];
_f.push(compile(path.join(source, file)));
}
return _f;
});
} else if (topLevel || path.extname(source) === '.coffee') {
fs.readFile(source, function(err, code) {
return compileScript(source, code.toString(), base);
});
if (options.watch) {
return watch(source, base);
}
return _f;
});
} else if (topLevel || path.extname(source) === '.coffee') {
fs.readFile(source, function(err, code) {
return compileScript(source, code.toString(), base);
});
if (options.watch) {
return watch(source, base);
}
}
});
});
});
};
return compile(source, true);
})());
};
return compile(source, true);
})());
})();
}
return _b;
};
Expand Down
31 changes: 19 additions & 12 deletions lib/nodes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1590,20 +1590,22 @@
// comprehensions. Some of the generated code can be shared in common, and
// some cannot.
ForNode.prototype.compileNode = function(o) {
var body, bodyDent, close, forPart, index, ivar, lvar, name, range, returnResult, rvar, scope, source, sourcePart, stepPart, svar, topLevel, varPart, vars;
var body, close, codeInBody, forPart, index, ivar, lvar, name, namePart, range, returnResult, rvar, scope, source, sourcePart, stepPart, svar, topLevel, varPart, vars;
topLevel = del(o, 'top') && !this.returns;
range = this.source instanceof ValueNode && this.source.base instanceof RangeNode && !this.source.properties.length;
source = range ? this.source.base : this.source;
codeInBody = this.body.contains(function(n) {
return n instanceof CodeNode;
});
scope = o.scope;
name = this.name && this.name.compile(o);
index = this.index && this.index.compile(o);
if (name && !this.pattern) {
if (name && !this.pattern && !codeInBody) {
scope.find(name);
}
if (index) {
if (index && !codeInBody) {
scope.find(index);
}
bodyDent = this.idt(1);
if (!(topLevel)) {
rvar = scope.freeVariable();
}
Expand All @@ -1620,13 +1622,13 @@
svar = scope.freeVariable();
sourcePart = ("" + svar + " = " + (this.source.compile(o)) + ";");
if (this.pattern) {
varPart = new AssignNode(this.name, literal(("" + svar + "[" + ivar + "]"))).compile(merge(o, {
namePart = new AssignNode(this.name, literal(("" + svar + "[" + ivar + "]"))).compile(merge(o, {
indent: this.idt(1),
top: true
})) + "\n";
} else {
if (name) {
varPart = ("" + bodyDent + name + " = " + svar + "[" + ivar + "];\n");
namePart = ("" + name + " = " + svar + "[" + ivar + "]");
}
}
if (!(this.object)) {
Expand All @@ -1638,18 +1640,23 @@
sourcePart = (rvar ? ("" + rvar + " = []; ") : '') + sourcePart;
sourcePart = sourcePart ? ("" + this.tab + sourcePart + "\n" + this.tab) : this.tab;
returnResult = this.compileReturnValue(rvar, o);
if (topLevel && body.contains(function(n) {
return n instanceof CodeNode;
})) {
body = ClosureNode.wrap(body, true);
}
if (!(topLevel)) {
body = PushNode.wrap(rvar, body);
}
this.guard ? (body = Expressions.wrap([new IfNode(this.guard, body)])) : null;
if (codeInBody) {
if (namePart) {
body.unshift(literal(("var " + namePart)));
}
body = ClosureNode.wrap(body, true);
} else {
if (namePart) {
varPart = ("" + (this.idt(1)) + namePart + ";\n");
}
}
this.object ? (forPart = ("" + ivar + " in " + svar + ") { if (" + (utility('hasProp')) + ".call(" + svar + ", " + ivar + ")")) : null;
body = body.compile(merge(o, {
indent: bodyDent,
indent: this.idt(1),
top: true
}));
vars = range ? name : ("" + name + ", " + ivar);
Expand Down
40 changes: 22 additions & 18 deletions src/nodes.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1188,44 +1188,48 @@ exports.ForNode: class ForNode extends BaseNode
# comprehensions. Some of the generated code can be shared in common, and
# some cannot.
compileNode: (o) ->
topLevel: del(o, 'top') and not @returns
topLevel: del(o, 'top') and not @returns
range: @source instanceof ValueNode and @source.base instanceof RangeNode and not @source.properties.length
source: if range then @source.base else @source
codeInBody: @body.contains (n) -> n instanceof CodeNode
scope: o.scope
name: @name and @name.compile o
index: @index and @index.compile o
scope.find name if name and not @pattern
scope.find index if index
bodyDent: @idt 1
scope.find name if name and not @pattern and not codeInBody
scope.find index if index and not codeInBody
rvar: scope.freeVariable() unless topLevel
ivar: if range then name else index or scope.freeVariable()
varPart: ''
varPart: ''
body: Expressions.wrap([@body])
if range
sourcePart: source.compileVariables o
forPart: source.compile merge o, {index: ivar, step: @step}
sourcePart: source.compileVariables o
forPart: source.compile merge o, {index: ivar, step: @step}
else
svar: scope.freeVariable()
sourcePart: "$svar = ${ @source.compile(o) };"
sourcePart: "$svar = ${ @source.compile(o) };"
if @pattern
varPart: new AssignNode(@name, literal("$svar[$ivar]")).compile(merge o, {indent: @idt(1), top: true}) + "\n"
namePart: new AssignNode(@name, literal("$svar[$ivar]")).compile(merge o, {indent: @idt(1), top: true}) + "\n"
else
varPart: "$bodyDent$name = $svar[$ivar];\n" if name
namePart: "$name = $svar[$ivar]" if name
unless @object
lvar: scope.freeVariable()
stepPart: if @step then "$ivar += ${ @step.compile(o) }" else "$ivar++"
forPart: "$ivar = 0, $lvar = ${svar}.length; $ivar < $lvar; $stepPart"
sourcePart: (if rvar then "$rvar = []; " else '') + sourcePart
sourcePart: if sourcePart then "$@tab$sourcePart\n$@tab" else @tab
returnResult: @compileReturnValue(rvar, o)
stepPart: if @step then "$ivar += ${ @step.compile(o) }" else "$ivar++"
forPart: "$ivar = 0, $lvar = ${svar}.length; $ivar < $lvar; $stepPart"
sourcePart: (if rvar then "$rvar = []; " else '') + sourcePart
sourcePart: if sourcePart then "$@tab$sourcePart\n$@tab" else @tab
returnResult: @compileReturnValue(rvar, o)

body: ClosureNode.wrap(body, true) if topLevel and body.contains (n) -> n instanceof CodeNode
body: PushNode.wrap(rvar, body) unless topLevel
if @guard
body: Expressions.wrap([new IfNode(@guard, body)])
if codeInBody
body.unshift literal "var $namePart" if namePart
body: ClosureNode.wrap(body, true)
else
varPart: "${@idt(1)}$namePart;\n" if namePart
if @object
forPart: "$ivar in $svar) { if (${utility('hasProp')}.call($svar, $ivar)"
body: body.compile(merge(o, {indent: bodyDent, top: true}))
forPart: "$ivar in $svar) { if (${utility('hasProp')}.call($svar, $ivar)"
body: body.compile(merge(o, {indent: @idt(1), top: true}))
vars: if range then name else "$name, $ivar"
close: if @object then '}}' else '}'
"${sourcePart}for ($forPart) {\n$varPart$body\n$@tab$close$returnResult"
Expand Down
20 changes: 14 additions & 6 deletions test/test_comprehensions.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,26 @@ ok 2 in evens
# Ensure that the closure wrapper preserves local variables.
obj: {}

methods: ['one', 'two', 'three']

for method in methods
name: method
obj[name]: ->
"I'm " + name
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'


# Naked ranges are expanded into arrays.
array: [0..10]
ok(num % 2 is 0 for num in array by 2)
Expand Down

0 comments on commit b0a45e5

Please sign in to comment.