From b0a45e5b93d0534bc160d218fb80900609fd1034 Mon Sep 17 00:00:00 2001 From: Jeremy Ashkenas Date: Sun, 13 Jun 2010 21:21:30 -0400 Subject: [PATCH] Ticket #423. When functions are generated within comprehensions ... the comprehensions should close over the element instead of sharing it. --- lib/command.js | 65 +++++++++++++++++---------------- lib/nodes.js | 31 ++++++++++------ src/nodes.coffee | 40 +++++++++++--------- test/test_comprehensions.coffee | 20 +++++++--- 4 files changed, 89 insertions(+), 67 deletions(-) diff --git a/lib/command.js b/lib/command.js index d5d0d3f3..9d8bbe3e 100644 --- a/lib/command.js +++ b/lib/command.js @@ -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))); - } - 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); - } + (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 compile(source, true); - })()); + }; + return compile(source, true); + })()); + })(); } return _b; }; diff --git a/lib/nodes.js b/lib/nodes.js index 6c8a3c54..0cc823af 100644 --- a/lib/nodes.js +++ b/lib/nodes.js @@ -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(); } @@ -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)) { @@ -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); diff --git a/src/nodes.coffee b/src/nodes.coffee index 698e283c..badbbe1d 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -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" diff --git a/test/test_comprehensions.coffee b/test/test_comprehensions.coffee index 3c111963..a3566e5d 100644 --- a/test/test_comprehensions.coffee +++ b/test/test_comprehensions.coffee @@ -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)