From 59d6e59f97d262099001b527a0b3ac80228653e8 Mon Sep 17 00:00:00 2001 From: Jeremy Ashkenas Date: Wed, 6 Oct 2010 20:54:08 -0400 Subject: [PATCH] Fixing Issue 730 -- and removing garbage collection of tempvars (which was totally unsafe.) --- lib/lexer.js | 18 ++++----- lib/nodes.js | 80 ++++++++++++++++++++-------------------- lib/rewriter.js | 8 ++-- lib/scope.js | 29 ++------------- src/nodes.coffee | 9 +++-- src/scope.coffee | 23 ++---------- test/test_classes.coffee | 28 +++++++++++++- 7 files changed, 94 insertions(+), 101 deletions(-) diff --git a/lib/lexer.js b/lib/lexer.js index 92aa8510..e9f216ab 100644 --- a/lib/lexer.js +++ b/lib/lexer.js @@ -190,7 +190,7 @@ return true; }; Lexer.prototype.heregexToken = function(match) { - var _i, _len, _ref2, _ref3, _this, body, flags, heregex, re, tag, tokens, value; + var _i, _len, _ref2, _ref3, _ref4, _ref5, _this, body, flags, heregex, re, tag, tokens, value; _ref2 = match, heregex = _ref2[0], body = _ref2[1], flags = _ref2[2]; this.i += heregex.length; if (0 > body.indexOf('#{')) { @@ -201,11 +201,11 @@ this.token('IDENTIFIER', 'RegExp'); this.tokens.push(['CALL_START', '(']); tokens = []; - _ref2 = this.interpolateString(body, { + _ref3 = this.interpolateString(body, { regex: true }); - for (_i = 0, _len = _ref2.length; _i < _len; _i++) { - _ref3 = _ref2[_i], tag = _ref3[0], value = _ref3[1]; + for (_i = 0, _len = _ref3.length; _i < _len; _i++) { + _ref4 = _ref3[_i], tag = _ref4[0], value = _ref4[1]; if (tag === 'TOKENS') { tokens.push.apply(tokens, value); } else { @@ -218,7 +218,7 @@ tokens.push(['+', '+']); } tokens.pop(); - if ((((_ref2 = tokens[0]) != null) ? _ref2[0] !== 'STRING' : undefined)) { + if ((((_ref5 = tokens[0]) != null) ? _ref5[0] !== 'STRING' : undefined)) { this.tokens.push(['STRING', '""'], ['+', '+']); } (_this = this.tokens).push.apply(_this, tokens); @@ -394,7 +394,7 @@ return accessor ? 'accessor' : false; }; Lexer.prototype.sanitizeHeredoc = function(doc, options) { - var _ref2, attempt, herecomment, indent, match; + var _ref2, _ref3, attempt, herecomment, indent, match; _ref2 = options, indent = _ref2.indent, herecomment = _ref2.herecomment; if (herecomment && 0 > doc.indexOf('\n')) { return doc; @@ -402,7 +402,7 @@ if (!(herecomment)) { while (match = HEREDOC_INDENT.exec(doc)) { attempt = match[1]; - if (indent === null || (0 < (_ref2 = attempt.length)) && (_ref2 < indent.length)) { + if (indent === null || (0 < (_ref3 = attempt.length)) && (_ref3 < indent.length)) { indent = attempt; } } @@ -487,7 +487,7 @@ return !i ? false : str.slice(0, i); }; Lexer.prototype.interpolateString = function(str, options) { - var _len, _ref2, _this, expr, heredoc, i, inner, interpolated, letter, nested, pi, regex, tag, tokens, value; + var _len, _ref2, _ref3, _this, expr, heredoc, i, inner, interpolated, letter, nested, pi, regex, tag, tokens, value; _ref2 = options || (options = {}), heredoc = _ref2.heredoc, regex = _ref2.regex; tokens = []; pi = 0; @@ -535,7 +535,7 @@ this.tokens.push(['STRING', '""'], ['+', '+']); } for (i = 0, _len = tokens.length; i < _len; i++) { - _ref2 = tokens[i], tag = _ref2[0], value = _ref2[1]; + _ref3 = tokens[i], tag = _ref3[0], value = _ref3[1]; if (i) { this.token('+', '+'); } diff --git a/lib/nodes.js b/lib/nodes.js index 38544074..da915b2e 100644 --- a/lib/nodes.js +++ b/lib/nodes.js @@ -32,13 +32,7 @@ this.tab = o.indent; top = this.topSensitive() ? this.options.top : del(this.options, 'top'); closure = this.isStatement(o) && !this.isPureStatement() && !top && !this.options.asStatement && !(this instanceof CommentNode) && !this.containsPureStatement(); - if (!o.keepLevel) { - o.scope.startLevel(); - } code = closure ? this.compileClosure(this.options) : this.compileNode(this.options); - if (!o.keepLevel) { - o.scope.endLevel(); - } return code; }; BaseNode.prototype.compileClosure = function(o) { @@ -526,7 +520,7 @@ return node; }; CallNode.prototype.compileNode = function(o) { - var _i, _len, _ref2, _result, arg, args, left, node, rite, val; + var _i, _j, _len, _len2, _ref2, _ref3, _ref4, _result, arg, args, left, node, rite, val; if (node = this.unfoldSoak(o)) { return node.compile(o); } @@ -546,17 +540,17 @@ rite = rite.compile(o); return ("(" + left + " ? undefined : " + rite + ")"); } - _ref2 = this.args; - for (_i = 0, _len = _ref2.length; _i < _len; _i++) { - arg = _ref2[_i]; + _ref3 = this.args; + for (_i = 0, _len = _ref3.length; _i < _len; _i++) { + arg = _ref3[_i]; if (arg instanceof SplatNode) { return this.compileSplat(o); } } args = (function() { - _result = []; _ref2 = this.args; - for (_i = 0, _len = _ref2.length; _i < _len; _i++) { - arg = _ref2[_i]; + _result = []; _ref4 = this.args; + for (_j = 0, _len2 = _ref4.length; _j < _len2; _j++) { + arg = _ref4[_j]; _result.push((arg.parenthetical = true) && arg.compile(o)); } return _result; @@ -680,17 +674,17 @@ __extends(RangeNode, BaseNode); RangeNode.prototype.children = ['from', 'to']; RangeNode.prototype.compileVariables = function(o) { - var _ref2, parts; + var _ref2, _ref3, _ref4, parts; o = merge(o, { top: true }); _ref2 = this.from.compileReference(o, { precompile: true }), this.from = _ref2[0], this.fromVar = _ref2[1]; - _ref2 = this.to.compileReference(o, { + _ref3 = this.to.compileReference(o, { precompile: true - }), this.to = _ref2[0], this.toVar = _ref2[1]; - _ref2 = [this.fromVar.match(SIMPLENUM), this.toVar.match(SIMPLENUM)], this.fromNum = _ref2[0], this.toNum = _ref2[1]; + }), this.to = _ref3[0], this.toVar = _ref3[1]; + _ref4 = [this.fromVar.match(SIMPLENUM), this.toVar.match(SIMPLENUM)], this.fromNum = _ref4[0], this.toNum = _ref4[1]; parts = []; if (this.from !== this.fromVar) { parts.push(this.from); @@ -791,7 +785,7 @@ ObjectNode.prototype.children = ['properties']; ObjectNode.prototype.topSensitive = YES; ObjectNode.prototype.compileNode = function(o) { - var _i, _len, _ref2, _result, i, indent, join, lastNoncom, nonComments, obj, prop, props, top; + var _i, _len, _len2, _ref2, _ref3, _result, _result2, i, indent, join, lastNoncom, nonComments, obj, prop, props, top; top = del(o, 'top'); o.indent = this.idt(1); nonComments = (function() { @@ -806,10 +800,10 @@ }).call(this); lastNoncom = last(nonComments); props = (function() { - _result = []; _ref2 = this.properties; - for (i = 0, _len = _ref2.length; i < _len; i++) { - prop = _ref2[i]; - _result.push((function() { + _result2 = []; _ref3 = this.properties; + for (i = 0, _len2 = _ref3.length; i < _len2; i++) { + prop = _ref3[i]; + _result2.push((function() { join = ",\n"; if ((prop === lastNoncom) || (prop instanceof CommentNode)) { join = "\n"; @@ -826,7 +820,7 @@ return indent + prop.compile(o) + join; }).call(this)); } - return _result; + return _result2; }).call(this); props = props.join(''); obj = '{' + (props ? '\n' + props + '\n' + this.idt() : '') + '}'; @@ -891,7 +885,7 @@ return this; }; ClassNode.prototype.compileNode = function(o) { - var _i, _len, _ref2, _ref3, access, applied, className, constScope, construct, constructor, extension, func, me, pname, prop, props, pvar, returns, val; + var _i, _len, _ref2, _ref3, _ref4, access, applied, apply, className, constScope, construct, constructor, extension, func, me, pname, prop, props, pvar, ref, returns, val; if (this.variable === '__temp__') { this.variable = literal(o.scope.freeVariable('ctor')); } @@ -911,7 +905,15 @@ for (_i = 0, _len = _ref2.length; _i < _len; _i++) { prop = _ref2[_i]; _ref3 = [prop.variable, prop.value], pvar = _ref3[0], func = _ref3[1]; - if (pvar && pvar.base.value === 'constructor' && func instanceof CodeNode) { + if (pvar && pvar.base.value === 'constructor') { + if (!(func instanceof CodeNode)) { + _ref4 = func.compileReference(o), func = _ref4[0], ref = _ref4[1]; + if (func !== ref) { + props.push(func); + } + apply = new CallNode(new ValueNode(ref, [new AccessorNode(literal('apply'))]), [literal('this'), literal('arguments')]); + func = new CodeNode([], new Expressions([apply])); + } if (func.bound) { throw new Error("cannot define a constructor as a bound function."); } @@ -1008,7 +1010,7 @@ return top || this.parenthetical ? val : ("(" + val + ")"); }; AssignNode.prototype.compilePatternMatch = function(o) { - var _len, _ref2, accessClass, assigns, code, i, idx, isObject, obj, objects, olength, otop, splat, top, val, valVar, value; + var _len, _ref2, _ref3, accessClass, assigns, code, i, idx, isObject, obj, objects, olength, otop, splat, top, val, valVar, value; if ((value = this.value).isStatement(o)) { value = ClosureNode.wrap(value); } @@ -1042,7 +1044,7 @@ idx = i; if (isObject) { if (obj instanceof AssignNode) { - _ref2 = [obj.value, obj.variable.base], obj = _ref2[0], idx = _ref2[1]; + _ref3 = [obj.value, obj.variable.base], obj = _ref3[0], idx = _ref3[1]; } else { idx = obj.tags["this"] ? obj.properties[0].name : obj; } @@ -1099,7 +1101,7 @@ __extends(CodeNode, BaseNode); CodeNode.prototype.children = ['params', 'body']; CodeNode.prototype.compileNode = function(o) { - var _i, _len, _ref2, _ref3, _result, close, code, empty, func, i, open, param, params, sharedScope, splat, top, value; + var _i, _j, _len, _len2, _len3, _ref2, _ref3, _result, close, code, empty, func, i, open, param, params, sharedScope, splat, top, value; sharedScope = del(o, 'sharedScope'); top = del(o, 'top'); o.scope = sharedScope || new Scope(o.scope, this.body, this); @@ -1138,7 +1140,7 @@ } params = (function() { _result = []; - for (_i = 0, _len = params.length; _i < _len; _i++) { + for (_i = 0, _len2 = params.length; _i < _len2; _i++) { param = params[_i]; _result.push(param.compile(o)); } @@ -1147,8 +1149,8 @@ if (!(empty)) { this.body.makeReturn(); } - for (_i = 0, _len = params.length; _i < _len; _i++) { - param = params[_i]; + for (_j = 0, _len3 = params.length; _j < _len3; _j++) { + param = params[_j]; (o.scope.parameter(param)); } if (this.className) { @@ -1410,10 +1412,10 @@ return [this.first.compile(o), this.operator, this.second.compile(o)].join(' '); }; OpNode.prototype.compileChain = function(o) { - var _ref2, first, second, shared; + var _ref2, _ref3, first, second, shared; shared = this.first.unwrap().second; _ref2 = shared.compileReference(o), this.first.second = _ref2[0], shared = _ref2[1]; - _ref2 = [this.first.compile(o), this.second.compile(o), shared.compile(o)], first = _ref2[0], second = _ref2[1], shared = _ref2[2]; + _ref3 = [this.first.compile(o), this.second.compile(o), shared.compile(o)], first = _ref3[0], second = _ref3[1], shared = _ref3[2]; return "(" + first + ") && (" + shared + " " + (this.operator) + " " + second + ")"; }; OpNode.prototype.compileAssignment = function(o) { @@ -1478,11 +1480,11 @@ return "(" + (tests.join(' || ')) + ")"; }; InNode.prototype.compileLoopTest = function(o) { - var _ref2, i, l, prefix; + var _ref2, _ref3, i, l, prefix; _ref2 = this.array.compileReference(o, { precompile: true }), this.arr1 = _ref2[0], this.arr2 = _ref2[1]; - _ref2 = [o.scope.freeVariable('i'), o.scope.freeVariable('len')], i = _ref2[0], l = _ref2[1]; + _ref3 = [o.scope.freeVariable('i'), o.scope.freeVariable('len')], i = _ref3[0], l = _ref3[1]; prefix = this.obj1 !== this.obj2 ? this.obj1 + '; ' : ''; return "(function(){ " + prefix + "for (var " + i + "=0, " + l + "=" + (this.arr1) + ".length; " + i + "<" + l + "; " + i + "++) { if (" + (this.arr2) + "[" + i + "] === " + (this.obj2) + ") return true; } return false; }).call(this)"; }; @@ -1761,7 +1763,7 @@ return this; }; SwitchNode.prototype.compileNode = function(o) { - var _i, _j, _len, _len2, _ref2, _ref3, block, code, condition, conditions, exprs, idt, pair; + var _i, _j, _len, _len2, _ref2, _ref3, _ref4, block, code, condition, conditions, exprs, idt, pair; idt = (o.indent = this.idt(2)); o.top = true; code = ("" + (this.tab) + "switch (" + (this.subject.compile(o)) + ") {"); @@ -1770,9 +1772,9 @@ pair = _ref2[_i]; _ref3 = pair, conditions = _ref3[0], block = _ref3[1]; exprs = block.expressions; - _ref3 = flatten([conditions]); - for (_j = 0, _len2 = _ref3.length; _j < _len2; _j++) { - condition = _ref3[_j]; + _ref4 = flatten([conditions]); + for (_j = 0, _len2 = _ref4.length; _j < _len2; _j++) { + condition = _ref4[_j]; if (this.tags.subjectless) { condition = new OpNode('!!', new ParentheticalNode(condition)); } diff --git a/lib/rewriter.js b/lib/rewriter.js index 5b77efe4..94fccc2c 100644 --- a/lib/rewriter.js +++ b/lib/rewriter.js @@ -135,13 +135,13 @@ var action, condition, stack; stack = []; condition = function(token, i) { - var _ref, one, tag, three, two; + var _ref, _ref2, one, tag, three, two; if ((this.tag(i + 1) === 'HERECOMMENT' || this.tag(i - 1) === 'HERECOMMENT')) { return false; } _ref = this.tokens.slice(i + 1, i + 4), one = _ref[0], two = _ref[1], three = _ref[2]; tag = token[0]; - return ('TERMINATOR' === tag || 'OUTDENT' === tag) && !(((two != null) ? two[0] === ':' : undefined) || ((one != null) ? one[0] === '@' : undefined) && ((three != null) ? three[0] === ':' : undefined)) || tag === ',' && !('IDENTIFIER' === (_ref = ((one != null) ? one[0] : undefined)) || 'STRING' === _ref || '@' === _ref || 'TERMINATOR' === _ref || 'OUTDENT' === _ref); + return ('TERMINATOR' === tag || 'OUTDENT' === tag) && !(((two != null) ? two[0] === ':' : undefined) || ((one != null) ? one[0] === '@' : undefined) && ((three != null) ? three[0] === ':' : undefined)) || tag === ',' && !('IDENTIFIER' === (_ref2 = ((one != null) ? one[0] : undefined)) || 'STRING' === _ref2 || '@' === _ref2 || 'TERMINATOR' === _ref2 || 'OUTDENT' === _ref2); }; action = function(token, i) { return this.tokens.splice(i, 0, ['}', '}', token[2]]); @@ -225,7 +225,7 @@ }; exports.Rewriter.prototype.addImplicitIndentation = function() { return this.scanTokens(function(token, i, tokens) { - var _ref, action, condition, indent, outdent, starter, tag; + var _ref, _ref2, action, condition, indent, outdent, starter, tag; tag = token[0]; if (tag === 'ELSE' && this.tag(i - 1) !== 'OUTDENT') { tokens.splice.apply(tokens, [i, 0].concat(this.indentation(token))); @@ -237,7 +237,7 @@ } if (include(SINGLE_LINERS, tag) && this.tag(i + 1) !== 'INDENT' && !(tag === 'ELSE' && this.tag(i + 1) === 'IF')) { starter = tag; - _ref = this.indentation(token), indent = _ref[0], outdent = _ref[1]; + _ref2 = this.indentation(token), indent = _ref2[0], outdent = _ref2[1]; if (starter === 'THEN') { indent.fromThen = true; } diff --git a/lib/scope.js b/lib/scope.js index 75a2d1a0..eb8ff3ba 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -11,31 +11,13 @@ this.variables = { 'arguments': 'arguments' }; - if (this.parent) { - this.garbage = this.parent.garbage; - } else { - this.garbage = []; + if (!(this.parent)) { Scope.root = this; } return this; }; })(); Scope.root = null; - Scope.prototype.startLevel = function() { - return this.garbage.push([]); - }; - Scope.prototype.endLevel = function() { - var _i, _len, _ref2, _result, name, vars; - vars = this.variables; - _result = []; _ref2 = this.garbage.pop(); - for (_i = 0, _len = _ref2.length; _i < _len; _i++) { - name = _ref2[_i]; - if (vars[name] === 'var') { - _result.push(vars[name] = 'reuse'); - } - } - return _result; - }; Scope.prototype.find = function(name, options) { if (this.check(name, options)) { return true; @@ -72,13 +54,10 @@ Scope.prototype.freeVariable = function(type) { var index, temp; index = 0; - while (this.check(temp = this.temporary(type, index)) && this.variables[temp] !== 'reuse') { + while (this.check(temp = this.temporary(type, index))) { index++; } this.variables[temp] = 'var'; - if (this.garbage.length) { - last(this.garbage).push(temp); - } return temp; }; Scope.prototype.assign = function(name, value) { @@ -89,7 +68,7 @@ }; Scope.prototype.hasDeclarations = function(body) { return body === this.expressions && this.any(function(k, val) { - return ('var' === val || 'reuse' === val); + return val === 'var'; }); }; Scope.prototype.hasAssignments = function(body) { @@ -104,7 +83,7 @@ for (key in _ref2) { if (!__hasProp.call(_ref2, key)) continue; val = _ref2[key]; - if (('var' === val || 'reuse' === val)) { + if (val === 'var') { _result.push(key); } } diff --git a/src/nodes.coffee b/src/nodes.coffee index 5c7fa373..a15c4a76 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -46,9 +46,7 @@ exports.BaseNode = class BaseNode closure = @isStatement(o) and not @isPureStatement() and not top and not @options.asStatement and this not instanceof CommentNode and not @containsPureStatement() - o.scope.startLevel() if not o.keepLevel code = if closure then @compileClosure(@options) else @compileNode(@options) - o.scope.endLevel() if not o.keepLevel code # Statements converted into expressions via closure-wrapping share a scope @@ -787,7 +785,12 @@ exports.ClassNode = class ClassNode extends BaseNode for prop in @properties [pvar, func] = [prop.variable, prop.value] - if pvar and pvar.base.value is 'constructor' and func instanceof CodeNode + if pvar and pvar.base.value is 'constructor' + if func not instanceof CodeNode + [func, ref] = func.compileReference o + props.push func if func isnt ref + apply = new CallNode(new ValueNode(ref, [new AccessorNode literal 'apply']), [literal('this'), literal('arguments')]) + func = new CodeNode [], new Expressions([apply]) throw new Error "cannot define a constructor as a bound function." if func.bound func.name = className func.body.push new ReturnNode literal 'this' diff --git a/src/scope.coffee b/src/scope.coffee index 3a1aad31..772dcf2a 100644 --- a/src/scope.coffee +++ b/src/scope.coffee @@ -19,21 +19,7 @@ exports.Scope = class Scope # it wraps. constructor: (@parent, @expressions, @method) -> @variables = {'arguments'} - if @parent - @garbage = @parent.garbage - else - @garbage = [] - Scope.root = this - - # Create a new garbage level - startLevel: -> - @garbage.push [] - - # Return to the previous garbage level and erase referenced temporary - # variables in current level from scope. - endLevel: -> - vars = @variables - (vars[name] = 'reuse') for name in @garbage.pop() when vars[name] is 'var' + Scope.root = this unless @parent # Look up a variable name in lexical scope, and declare it if it does not # already exist. @@ -71,9 +57,8 @@ exports.Scope = class Scope # compiler-generated variable. `_var`, `_var2`, and so on... freeVariable: (type) -> index = 0 - index++ while @check(temp = @temporary type, index) and @variables[temp] isnt 'reuse' + index++ while @check(temp = @temporary type, index) @variables[temp] = 'var' - last(@garbage).push temp if @garbage.length temp # Ensure that an assignment is made at the top of this scope @@ -84,7 +69,7 @@ exports.Scope = class Scope # Does this scope reference any variables that need to be declared in the # given function body? hasDeclarations: (body) -> - body is @expressions and @any (k, val) -> val in ['var', 'reuse'] + body is @expressions and @any (k, val) -> val is 'var' # Does this scope reference any assignments that need to be declared at the # top of the given function body? @@ -93,7 +78,7 @@ exports.Scope = class Scope # Return the list of variables first declared in this scope. declaredVariables: -> - (key for key, val of @variables when val in ['var', 'reuse']).sort() + (key for key, val of @variables when val is 'var').sort() # Return the list of assignments that are supposed to be made at the top # of this scope. diff --git a/test/test_classes.coffee b/test/test_classes.coffee index 0b0727c8..ea1df23c 100644 --- a/test/test_classes.coffee +++ b/test/test_classes.coffee @@ -14,9 +14,11 @@ SecondChild = class extends FirstChild func: (string) -> super('two/') + string +thirdCtor = -> + @array = [1, 2, 3] + class ThirdChild extends SecondChild - constructor: -> - @array = [1, 2, 3] + constructor: thirdCtor # Gratuitous comment for testing. func: (string) -> @@ -34,6 +36,8 @@ result = (new ThirdChild).func 'four' ok result is '9two/three/four' +ok (new ThirdChild).array.join(' ') is '1 2 3' + class TopClass constructor: (arg) -> @@ -265,3 +269,23 @@ ok Static.static.two is 2 # Nothing classes. c = class ok c instanceof Function + + +# Classes with value'd constructors. +counter = 0 +classMaker = -> + counter += 1 + inner = counter + -> + @value = inner + +class One + constructor: classMaker() + +class Two + constructor: classMaker() + +ok (new One).value is 1 +ok (new Two).value is 2 +ok (new One).value is 1 +ok (new Two).value is 2