mirror of
https://github.com/jashkenas/coffeescript.git
synced 2022-11-09 12:23:24 -05:00
Fix #3778: Make for loops more consistent
The following two lines might seem equivalent: for n in [1, 2, 3] by a then a = 4; n for n in [1, 2, 3] by +a then a = 4; n But they used not to be, because `+a` was cached into a `ref`, while the plain `a` wasn’t. Now even simple identifiers are cached, making the two lines equivalent as expected.
This commit is contained in:
parent
934bd2acc7
commit
996a171a4e
3 changed files with 48 additions and 25 deletions
|
@ -1,6 +1,6 @@
|
|||
// Generated by CoffeeScript 1.9.0
|
||||
(function() {
|
||||
var Access, Arr, Assign, Base, Block, Call, Class, Code, CodeFragment, Comment, Existence, Expansion, Extends, For, HEXNUM, IDENTIFIER, IDENTIFIER_STR, IS_REGEX, IS_STRING, If, In, Index, LEVEL_ACCESS, LEVEL_COND, LEVEL_LIST, LEVEL_OP, LEVEL_PAREN, LEVEL_TOP, Literal, METHOD_DEF, NEGATE, NO, NUMBER, Obj, Op, Param, Parens, RESERVED, Range, Return, SIMPLENUM, STRICT_PROSCRIBED, Scope, Slice, Splat, Switch, TAB, THIS, Throw, Try, UTILITIES, Value, While, YES, addLocationDataFn, compact, del, ends, extend, flatten, fragmentsToText, isLiteralArguments, isLiteralThis, last, locationDataToString, merge, multident, parseNum, ref1, ref2, some, starts, throwSyntaxError, unfoldSoak, utility,
|
||||
var Access, Arr, Assign, Base, Block, Call, Class, Code, CodeFragment, Comment, Existence, Expansion, Extends, For, HEXNUM, IDENTIFIER, IDENTIFIER_STR, IS_REGEX, IS_STRING, If, In, Index, LEVEL_ACCESS, LEVEL_COND, LEVEL_LIST, LEVEL_OP, LEVEL_PAREN, LEVEL_TOP, Literal, METHOD_DEF, NEGATE, NO, NUMBER, Obj, Op, Param, Parens, RESERVED, Range, Return, SIMPLENUM, STRICT_PROSCRIBED, Scope, Slice, Splat, Switch, TAB, THIS, Throw, Try, UTILITIES, Value, While, YES, addLocationDataFn, compact, del, ends, extend, flatten, fragmentsToText, isComplexOrAssignable, isLiteralArguments, isLiteralThis, last, locationDataToString, merge, multident, parseNum, ref1, ref2, some, starts, throwSyntaxError, unfoldSoak, utility,
|
||||
extend1 = function(child, parent) { for (var key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; },
|
||||
hasProp = {}.hasOwnProperty,
|
||||
indexOf = [].indexOf || function(item) { for (var i = 0, l = this.length; i < l; i++) { if (i in this && this[i] === item) return i; } return -1; },
|
||||
|
@ -112,19 +112,20 @@
|
|||
return parts;
|
||||
};
|
||||
|
||||
Base.prototype.cache = function(o, level, reused) {
|
||||
var ref, sub;
|
||||
if (!this.isComplex()) {
|
||||
ref = level ? this.compileToFragments(o, level) : this;
|
||||
return [ref, ref];
|
||||
} else {
|
||||
ref = new Literal(reused || o.scope.freeVariable('ref'));
|
||||
Base.prototype.cache = function(o, level, isComplex) {
|
||||
var complex, ref, sub;
|
||||
complex = isComplex != null ? isComplex(this) : this.isComplex();
|
||||
if (complex) {
|
||||
ref = new Literal(o.scope.freeVariable('ref'));
|
||||
sub = new Assign(ref, this);
|
||||
if (level) {
|
||||
return [sub.compileToFragments(o, level), [this.makeCode(ref.value)]];
|
||||
} else {
|
||||
return [sub, ref];
|
||||
}
|
||||
} else {
|
||||
ref = level ? this.compileToFragments(o, level) : this;
|
||||
return [ref, ref];
|
||||
}
|
||||
};
|
||||
|
||||
|
@ -1124,14 +1125,15 @@
|
|||
}
|
||||
|
||||
Range.prototype.compileVariables = function(o) {
|
||||
var ref3, ref4, ref5, ref6, step;
|
||||
var isComplex, ref3, ref4, ref5, ref6, step;
|
||||
o = merge(o, {
|
||||
top: true
|
||||
});
|
||||
ref3 = this.cacheToCodeFragments(this.from.cache(o, LEVEL_LIST)), this.fromC = ref3[0], this.fromVar = ref3[1];
|
||||
ref4 = this.cacheToCodeFragments(this.to.cache(o, LEVEL_LIST)), this.toC = ref4[0], this.toVar = ref4[1];
|
||||
isComplex = del(o, 'isComplex');
|
||||
ref3 = this.cacheToCodeFragments(this.from.cache(o, LEVEL_LIST, isComplex)), this.fromC = ref3[0], this.fromVar = ref3[1];
|
||||
ref4 = this.cacheToCodeFragments(this.to.cache(o, LEVEL_LIST, isComplex)), this.toC = ref4[0], this.toVar = ref4[1];
|
||||
if (step = del(o, 'step')) {
|
||||
ref5 = this.cacheToCodeFragments(step.cache(o, LEVEL_LIST)), this.step = ref5[0], this.stepVar = ref5[1];
|
||||
ref5 = this.cacheToCodeFragments(step.cache(o, LEVEL_LIST, isComplex)), this.step = ref5[0], this.stepVar = ref5[1];
|
||||
}
|
||||
ref6 = [this.fromVar.match(NUMBER), this.toVar.match(NUMBER)], this.fromNum = ref6[0], this.toNum = ref6[1];
|
||||
if (this.stepVar) {
|
||||
|
@ -2773,7 +2775,7 @@
|
|||
kvar = (this.range && name) || index || ivar;
|
||||
kvarAssign = kvar !== ivar ? kvar + " = " : "";
|
||||
if (this.step && !this.range) {
|
||||
ref4 = this.cacheToCodeFragments(this.step.cache(o, LEVEL_LIST)), step = ref4[0], stepVar = ref4[1];
|
||||
ref4 = this.cacheToCodeFragments(this.step.cache(o, LEVEL_LIST, isComplexOrAssignable)), step = ref4[0], stepVar = ref4[1];
|
||||
stepNum = stepVar.match(NUMBER);
|
||||
}
|
||||
if (this.pattern) {
|
||||
|
@ -2787,7 +2789,8 @@
|
|||
forPartFragments = source.compileToFragments(merge(o, {
|
||||
index: ivar,
|
||||
name: name,
|
||||
step: this.step
|
||||
step: this.step,
|
||||
isComplex: isComplexOrAssignable
|
||||
}));
|
||||
} else {
|
||||
svar = this.source.compile(o, LEVEL_LIST);
|
||||
|
@ -3189,6 +3192,10 @@
|
|||
return (node instanceof Literal && node.value === 'this' && !node.asKey) || (node instanceof Code && node.bound) || (node instanceof Call && node.isSuper);
|
||||
};
|
||||
|
||||
isComplexOrAssignable = function(node) {
|
||||
return node.isComplex() || (typeof node.isAssignable === "function" ? node.isAssignable() : void 0);
|
||||
};
|
||||
|
||||
unfoldSoak = function(o, parent, name) {
|
||||
var ifn;
|
||||
if (!(ifn = parent[name].unfoldSoak(o))) {
|
||||
|
|
|
@ -102,14 +102,15 @@ exports.Base = class Base
|
|||
# If `level` is passed, then returns `[val, ref]`, where `val` is the compiled value, and `ref`
|
||||
# is the compiled reference. If `level` is not passed, this returns `[val, ref]` where
|
||||
# the two values are raw nodes which have not been compiled.
|
||||
cache: (o, level, reused) ->
|
||||
unless @isComplex()
|
||||
ref = if level then @compileToFragments o, level else this
|
||||
[ref, ref]
|
||||
else
|
||||
ref = new Literal reused or o.scope.freeVariable 'ref'
|
||||
cache: (o, level, isComplex) ->
|
||||
complex = if isComplex? then isComplex this else @isComplex()
|
||||
if complex
|
||||
ref = new Literal o.scope.freeVariable 'ref'
|
||||
sub = new Assign ref, this
|
||||
if level then [sub.compileToFragments(o, level), [@makeCode(ref.value)]] else [sub, ref]
|
||||
else
|
||||
ref = if level then @compileToFragments o, level else this
|
||||
[ref, ref]
|
||||
|
||||
cacheToCodeFragments: (cacheValues) ->
|
||||
[fragmentsToText(cacheValues[0]), fragmentsToText(cacheValues[1])]
|
||||
|
@ -793,9 +794,10 @@ exports.Range = class Range extends Base
|
|||
# But only if they need to be cached to avoid double evaluation.
|
||||
compileVariables: (o) ->
|
||||
o = merge o, top: true
|
||||
[@fromC, @fromVar] = @cacheToCodeFragments @from.cache o, LEVEL_LIST
|
||||
[@toC, @toVar] = @cacheToCodeFragments @to.cache o, LEVEL_LIST
|
||||
[@step, @stepVar] = @cacheToCodeFragments step.cache o, LEVEL_LIST if step = del o, 'step'
|
||||
isComplex = del o, 'isComplex'
|
||||
[@fromC, @fromVar] = @cacheToCodeFragments @from.cache o, LEVEL_LIST, isComplex
|
||||
[@toC, @toVar] = @cacheToCodeFragments @to.cache o, LEVEL_LIST, isComplex
|
||||
[@step, @stepVar] = @cacheToCodeFragments step.cache o, LEVEL_LIST, isComplex if step = del o, 'step'
|
||||
[@fromNum, @toNum] = [@fromVar.match(NUMBER), @toVar.match(NUMBER)]
|
||||
@stepNum = @stepVar.match(NUMBER) if @stepVar
|
||||
|
||||
|
@ -1971,7 +1973,7 @@ exports.For = class For extends While
|
|||
kvar = (@range and name) or index or ivar
|
||||
kvarAssign = if kvar isnt ivar then "#{kvar} = " else ""
|
||||
if @step and not @range
|
||||
[step, stepVar] = @cacheToCodeFragments @step.cache o, LEVEL_LIST
|
||||
[step, stepVar] = @cacheToCodeFragments @step.cache o, LEVEL_LIST, isComplexOrAssignable
|
||||
stepNum = stepVar.match NUMBER
|
||||
name = ivar if @pattern
|
||||
varPart = ''
|
||||
|
@ -1979,7 +1981,8 @@ exports.For = class For extends While
|
|||
defPart = ''
|
||||
idt1 = @tab + TAB
|
||||
if @range
|
||||
forPartFragments = source.compileToFragments merge(o, {index: ivar, name, @step})
|
||||
forPartFragments = source.compileToFragments merge o,
|
||||
{index: ivar, name, @step, isComplex: isComplexOrAssignable}
|
||||
else
|
||||
svar = @source.compile o, LEVEL_LIST
|
||||
if (name or @own) and not IDENTIFIER.test svar
|
||||
|
@ -2291,6 +2294,8 @@ isLiteralThis = (node) ->
|
|||
(node instanceof Code and node.bound) or
|
||||
(node instanceof Call and node.isSuper)
|
||||
|
||||
isComplexOrAssignable = (node) -> node.isComplex() or node.isAssignable?()
|
||||
|
||||
# Unfold a node's child if soak, then tuck the node under created `If`
|
||||
unfoldSoak = (o, parent, name) ->
|
||||
return unless ifn = parent[name].unfoldSoak o
|
||||
|
|
|
@ -555,3 +555,14 @@ test "splats in destructuring in comprehensions", ->
|
|||
test "#156: expansion in destructuring in comprehensions", ->
|
||||
list = [[0, 1, 2], [2, 3, 4], [4, 5, 6]]
|
||||
arrayEq (last for [..., last] in list), [2, 4, 6]
|
||||
|
||||
test "#3778: Consistently always cache for loop range boundaries and steps, even
|
||||
if they are simple identifiers", ->
|
||||
a = 1; arrayEq [1, 2, 3], (for n in [1, 2, 3] by a then a = 4; n)
|
||||
a = 1; arrayEq [1, 2, 3], (for n in [1, 2, 3] by +a then a = 4; n)
|
||||
a = 1; arrayEq [1, 2, 3], (for n in [a..3] then a = 4; n)
|
||||
a = 1; arrayEq [1, 2, 3], (for n in [+a..3] then a = 4; n)
|
||||
a = 3; arrayEq [1, 2, 3], (for n in [1..a] then a = 4; n)
|
||||
a = 3; arrayEq [1, 2, 3], (for n in [1..+a] then a = 4; n)
|
||||
a = 1; arrayEq [1, 2, 3], (for n in [1..3] by a then a = 4; n)
|
||||
a = 1; arrayEq [1, 2, 3], (for n in [1..3] by +a then a = 4; n)
|
||||
|
|
Loading…
Reference in a new issue