From c4b72fcc794e235da11248a8a0f39624df5cf13a Mon Sep 17 00:00:00 2001 From: Michael Ficarra Date: Wed, 23 Mar 2011 13:49:15 -0400 Subject: [PATCH 1/5] improved range compilation --- lib/lexer.js | 2 +- lib/nodes.js | 13 ++++++------- src/nodes.coffee | 15 +++++++-------- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/lib/lexer.js b/lib/lexer.js index 945f11d7..8fdee451 100644 --- a/lib/lexer.js +++ b/lib/lexer.js @@ -459,7 +459,7 @@ Lexer.prototype.balancedString = function(str, end) { var i, letter, prev, stack, _ref; stack = [end]; - for (i = 1, _ref = str.length; (1 <= _ref ? i < _ref : i > _ref); (1 <= _ref ? i += 1 : i -= 1)) { + for (i = 1, _ref = str.length; 1 <= _ref ? i < _ref : i > _ref; 1 <= _ref ? i++ : i--) { switch (letter = str.charAt(i)) { case '\\': i++; diff --git a/lib/nodes.js b/lib/nodes.js index 7b2ccd5e..4a50faed 100644 --- a/lib/nodes.js +++ b/lib/nodes.js @@ -782,7 +782,7 @@ } }; Range.prototype.compileNode = function(o) { - var compare, idx, incr, intro, step, stepPart, vars; + var compare, cond, idx, incr, step, vars; this.compileVariables(o); if (!o.index) { return this.compileArray(o); @@ -793,10 +793,9 @@ idx = del(o, 'index'); step = del(o, 'step'); vars = ("" + idx + " = " + this.from) + (this.to !== this.toVar ? ", " + this.to : ''); - intro = "(" + this.fromVar + " <= " + this.toVar + " ? " + idx; - compare = "" + intro + " <" + this.equals + " " + this.toVar + " : " + idx + " >" + this.equals + " " + this.toVar + ")"; - stepPart = step ? step.compile(o) : '1'; - incr = step ? "" + idx + " += " + stepPart : "" + intro + " += " + stepPart + " : " + idx + " -= " + stepPart + ")"; + cond = "" + this.fromVar + " <= " + this.toVar; + compare = "" + cond + " ? " + idx + " <" + this.equals + " " + this.toVar + " : " + idx + " >" + this.equals + " " + this.toVar; + incr = step ? "" + idx + " += " + (step.compile(o)) : "" + cond + " ? " + idx + "++ : " + idx + "--"; return "" + vars + "; " + compare + "; " + incr; }; Range.prototype.compileSimple = function(o) { @@ -816,7 +815,7 @@ if (this.fromNum && this.toNum && Math.abs(this.fromNum - this.toNum) <= 20) { range = (function() { _results = []; - for (var _i = _ref = +this.fromNum, _ref2 = +this.toNum; _ref <= _ref2 ? _i <= _ref2 : _i >= _ref2; _ref <= _ref2 ? _i += 1 : _i -= 1){ _results.push(_i); } + for (var _i = _ref = +this.fromNum, _ref2 = +this.toNum; _ref <= _ref2 ? _i <= _ref2 : _i >= _ref2; _ref <= _ref2 ? _i++ : _i--){ _results.push(_i); } return _results; }).apply(this, arguments); if (this.exclusive) { @@ -834,7 +833,7 @@ } else { vars = ("" + i + " = " + this.from) + (this.to !== this.toVar ? ", " + this.to : ''); clause = "" + this.fromVar + " <= " + this.toVar + " ?"; - body = "var " + vars + "; " + clause + " " + i + " <" + this.equals + " " + this.toVar + " : " + i + " >" + this.equals + " " + this.toVar + "; " + clause + " " + i + " += 1 : " + i + " -= 1"; + body = "var " + vars + "; " + clause + " " + i + " <" + this.equals + " " + this.toVar + " : " + i + " >" + this.equals + " " + this.toVar + "; " + clause + " " + i + "++ : " + i + "--"; } post = "{ " + result + ".push(" + i + "); }\n" + idt + "return " + result + ";\n" + o.indent; return "(function() {" + pre + "\n" + idt + "for (" + body + ")" + post + "}).apply(this, arguments)"; diff --git a/src/nodes.coffee b/src/nodes.coffee index 99370609..c139d23c 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -642,10 +642,9 @@ exports.Range = class Range extends Base idx = del o, 'index' step = del o, 'step' vars = "#{idx} = #{@from}" + if @to isnt @toVar then ", #{@to}" else '' - intro = "(#{@fromVar} <= #{@toVar} ? #{idx}" - compare = "#{intro} <#{@equals} #{@toVar} : #{idx} >#{@equals} #{@toVar})" - stepPart = if step then step.compile(o) else '1' - incr = if step then "#{idx} += #{stepPart}" else "#{intro} += #{stepPart} : #{idx} -= #{stepPart})" + cond = "#{@fromVar} <= #{@toVar}" + compare = "#{cond} ? #{idx} <#{@equals} #{@toVar} : #{idx} >#{@equals} #{@toVar}" + incr = if step then "#{idx} += #{step.compile(o)}" else "#{cond} ? #{idx}++ : #{idx}--" "#{vars}; #{compare}; #{incr}" # Compile a simple range comprehension, with integers. @@ -671,11 +670,11 @@ exports.Range = class Range extends Base pre = "\n#{idt}#{result} = [];" if @fromNum and @toNum o.index = i - body = @compileSimple o + body = @compileSimple o else - vars = "#{i} = #{@from}" + if @to isnt @toVar then ", #{@to}" else '' - clause = "#{@fromVar} <= #{@toVar} ?" - body = "var #{vars}; #{clause} #{i} <#{@equals} #{@toVar} : #{i} >#{@equals} #{@toVar}; #{clause} #{i} += 1 : #{i} -= 1" + vars = "#{i} = #{@from}" + if @to isnt @toVar then ", #{@to}" else '' + cond = "#{@fromVar} <= #{@toVar}" + body = "var #{vars}; #{cond} ? #{i} <#{@equals} #{@toVar} : #{i} >#{@equals} #{@toVar}; #{cond} ? #{i}++ : #{i}--" post = "{ #{result}.push(#{i}); }\n#{idt}return #{result};\n#{o.indent}" "(function() {#{pre}\n#{idt}for (#{body})#{post}}).apply(this, arguments)" From 0497c0742ff537b0e6c6bc133ef3921d025e4349 Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Sun, 27 Mar 2011 19:46:44 +0200 Subject: [PATCH 2/5] fixes #1099: instead of nothing, compile to false "if a in []" --- lib/nodes.js | 3 +++ src/nodes.coffee | 1 + 2 files changed, 4 insertions(+) diff --git a/lib/nodes.js b/lib/nodes.js index 7b2ccd5e..4d956795 100644 --- a/lib/nodes.js +++ b/lib/nodes.js @@ -1719,6 +1719,9 @@ } return _results; }).call(this); + if (tests.length === 0) { + return 'false'; + } tests = tests.join(cnj); if (o.level < LEVEL_OP) { return tests; diff --git a/src/nodes.coffee b/src/nodes.coffee index 99370609..4c43430b 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -1340,6 +1340,7 @@ exports.In = class In extends Base [cmp, cnj] = if @negated then [' !== ', ' && '] else [' === ', ' || '] tests = for item, i in @array.base.objects (if i then ref else sub) + cmp + item.compile o, LEVEL_OP + return 'false' if tests.length is 0 tests = tests.join cnj if o.level < LEVEL_OP then tests else "(#{tests})" From e84e703211dfc5af3653462cdba066cad4bcffb4 Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Sun, 27 Mar 2011 21:22:09 +0200 Subject: [PATCH 3/5] fixes bug mentioned by @satyr in #1108 "[v] = a ? b" must compile to v = (typeof a != "undefined" && a !== null ? a : b)[0]; and not to: v = typeof a != "undefined" && a !== null ? a : b[0]; --- lib/nodes.js | 10 ++++++---- src/nodes.coffee | 8 ++++---- test/assignment.coffee | 4 ++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/nodes.js b/lib/nodes.js index 7b2ccd5e..bb30521c 100644 --- a/lib/nodes.js +++ b/lib/nodes.js @@ -1662,13 +1662,15 @@ Op.prototype.compileExistence = function(o) { var fst, ref; if (this.first.isComplex()) { - ref = o.scope.freeVariable('ref'); - fst = new Parens(new Assign(new Literal(ref), this.first)); + ref = new Literal(o.scope.freeVariable('ref')); + fst = new Parens(new Assign(ref, this.first)); } else { fst = this.first; - ref = fst.compile(o); + ref = fst; } - return new Existence(fst).compile(o) + (" ? " + ref + " : " + (this.second.compile(o, LEVEL_LIST))); + return new If(new Existence(fst), ref, { + type: 'if' + }).addElse(this.second).compile(o); }; Op.prototype.compileUnary = function(o) { var op, parts; diff --git a/src/nodes.coffee b/src/nodes.coffee index 99370609..9965ef1e 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -1301,12 +1301,12 @@ exports.Op = class Op extends Base compileExistence: (o) -> if @first.isComplex() - ref = o.scope.freeVariable 'ref' - fst = new Parens new Assign new Literal(ref), @first + ref = new Literal o.scope.freeVariable 'ref' + fst = new Parens new Assign ref, @first else fst = @first - ref = fst.compile o - new Existence(fst).compile(o) + " ? #{ref} : #{ @second.compile o, LEVEL_LIST }" + ref = fst + new If(new Existence(fst), ref, type: 'if').addElse(@second).compile o # Compile a unary **Op**. compileUnary: (o) -> diff --git a/test/assignment.coffee b/test/assignment.coffee index 077ae6f7..f9fbb5e0 100644 --- a/test/assignment.coffee +++ b/test/assignment.coffee @@ -229,6 +229,10 @@ test "destructuring assignment against an expression", -> eq a, y eq b, z +test "bracket insertion when necessary", -> + [a] = [0] ? [1] + eq a, 0 + # for implicit destructuring assignment in comprehensions, see the comprehension tests test "destructuring assignment with context (@) properties", -> From 9d72208d9e6a0bb14ab5756dc6161ccc6f060b18 Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Sun, 27 Mar 2011 21:35:29 +0200 Subject: [PATCH 4/5] added a test (issue #1099) --- test/operators.coffee | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/operators.coffee b/test/operators.coffee index 97ea24eb..39768e92 100644 --- a/test/operators.coffee +++ b/test/operators.coffee @@ -184,6 +184,9 @@ test "#768: `in` should preserve evaluation order", -> ok a() not in [b(),c()] eq 3, share +test "#1099: empty array after `in` should compile to `false`", -> + eq 5 in [], false + # Chained Comparison From f03bcc24add88611af0e24f7e80ee39622195893 Mon Sep 17 00:00:00 2001 From: Michael Ficarra Date: Sun, 27 Mar 2011 23:42:49 -0400 Subject: [PATCH 5/5] enhanced tests for #1099 fix --- test/operators.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/operators.coffee b/test/operators.coffee index 39768e92..87a0a6f5 100644 --- a/test/operators.coffee +++ b/test/operators.coffee @@ -185,7 +185,8 @@ test "#768: `in` should preserve evaluation order", -> eq 3, share test "#1099: empty array after `in` should compile to `false`", -> - eq 5 in [], false + eq 1, [5 in []].length + eq false, do -> return 0 in [] # Chained Comparison