[CS2] Restore bound class methods via runtime check to avoid premature calling of bound method before binding (#4561)

* bound method runtime check

* restore bound method tests

* bound method tests

* test bound method in prop-named class

* run check against parent class

* dummy commit

* remove comment

* rename to boundMethodCheck

* fixes from code review

* use ref to own class for check

* fixes from code review

* remove unneeded param
This commit is contained in:
Julian Rosse 2017-06-14 17:11:53 -05:00 committed by Geoffrey Booth
parent 76e70a6c81
commit 9a48566b24
6 changed files with 338 additions and 14 deletions

View File

@ -1782,6 +1782,14 @@
} else if ((this.name == null) && o.level === LEVEL_TOP) {
node = new Parens(node);
}
if (this.boundMethods.length && this.parent) {
if (this.variable == null) {
this.variable = new IdentifierLiteral(o.scope.freeVariable('_class'));
}
if (this.variableRef == null) {
[this.variable, this.variableRef] = this.variable.cache(o);
}
}
if (this.variable) {
node = new Assign(this.variable, node, null, {moduleDeclaration: this.moduleDeclaration});
}
@ -1795,7 +1803,7 @@
compileClassDeclaration(o) {
var ref1, result;
if (this.externalCtor) {
if (this.externalCtor || this.boundMethods.length) {
if (this.ctor == null) {
this.ctor = this.makeDefaultConstructor();
}
@ -1803,6 +1811,9 @@
if ((ref1 = this.ctor) != null) {
ref1.noReturn = true;
}
if (this.boundMethods.length) {
this.proxyBoundMethods();
}
o.indent += TAB;
result = [];
result.push(this.makeCode("class "));
@ -1850,6 +1861,7 @@
walkBody() {
var assign, end, executableBody, expression, expressions, exprs, i, initializer, initializerExpression, j, k, len1, len2, method, properties, pushSlice, ref1, start;
this.ctor = null;
this.boundMethods = [];
executableBody = null;
initializer = [];
({expressions} = this.body);
@ -1903,6 +1915,8 @@
this.ctor = method;
} else if (method.isStatic && method.bound) {
method.context = this.name;
} else if (method.bound) {
this.boundMethods.push(method);
}
}
}
@ -1958,8 +1972,8 @@
if (methodName.value === 'constructor') {
method.ctor = (this.parent ? 'derived' : 'base');
}
if (method.bound) {
method.error('Methods cannot be bound functions');
if (method.bound && method.ctor) {
method.error('Cannot define a constructor as a bound (fat arrow) function');
}
}
return method;
@ -1981,6 +1995,25 @@
return ctor;
}
proxyBoundMethods() {
var method, name;
this.ctor.thisAssignments = (function() {
var j, len1, ref1, results;
ref1 = this.boundMethods;
results = [];
for (j = 0, len1 = ref1.length; j < len1; j++) {
method = ref1[j];
if (this.parent) {
method.classVariable = this.variableRef;
}
name = new Value(new ThisLiteral, [method.name]);
results.push(new Assign(name, new Call(new Value(name, [new Access(new PropertyName('bind'))]), [new ThisLiteral])));
}
return results;
}).call(this);
return null;
}
};
Class.prototype.children = ['variable', 'parent', 'body'];
@ -2080,7 +2113,7 @@
return this.body.traverseChildren(false, (node) => {
if (node instanceof ThisLiteral) {
return node.value = this.name;
} else if (node instanceof Code && node.bound) {
} else if (node instanceof Code && node.bound && node.isStatic) {
return node.context = this.name;
}
});
@ -2718,7 +2751,7 @@
}
compileNode(o) {
var answer, body, condition, exprs, haveBodyParam, haveSplatParam, i, ifTrue, j, k, len1, len2, m, methodScope, modifiers, name, param, paramNames, params, paramsAfterSplat, ref, ref1, ref2, ref3, ref4, ref5, signature, splatParamName, thisAssignments, wasEmpty;
var answer, body, boundMethodCheck, condition, exprs, haveBodyParam, haveSplatParam, i, ifTrue, j, k, len1, len2, m, methodScope, modifiers, name, param, paramNames, params, paramsAfterSplat, ref, ref1, ref2, ref3, ref4, ref5, signature, splatParamName, thisAssignments, wasEmpty;
if (this.ctor) {
if (this.isAsync) {
this.name.error('Class constructor may not be async');
@ -2862,6 +2895,10 @@
this.body.expressions.unshift(...thisAssignments);
}
this.body.expressions.unshift(...exprs);
if (this.isMethod && this.bound && !this.isStatic && this.classVariable) {
boundMethodCheck = new Value(new Literal(utility('boundMethodCheck', o)));
this.body.expressions.unshift(new Call(boundMethodCheck, [new Value(new ThisLiteral), this.classVariable]));
}
if (!(wasEmpty || this.noReturn)) {
this.body.makeReturn();
}
@ -4244,6 +4281,9 @@
modulo: function() {
return 'function(a, b) { return (+a % (b = +b) + b) % b; }';
},
boundMethodCheck: function() {
return "function(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new Error('Bound instance method accessed before binding'); } }";
},
hasProp: function() {
return '{}.hasOwnProperty';
},

View File

@ -1308,6 +1308,10 @@ exports.Class = class Class extends Base
# Anonymous classes are only valid in expressions
node = new Parens node
if @boundMethods.length and @parent
@variable ?= new IdentifierLiteral o.scope.freeVariable '_class'
[@variable, @variableRef] = @variable.cache o unless @variableRef?
if @variable
node = new Assign @variable, node, null, { @moduleDeclaration }
@ -1318,9 +1322,11 @@ exports.Class = class Class extends Base
delete @compileNode
compileClassDeclaration: (o) ->
@ctor ?= @makeDefaultConstructor() if @externalCtor
@ctor ?= @makeDefaultConstructor() if @externalCtor or @boundMethods.length
@ctor?.noReturn = true
@proxyBoundMethods() if @boundMethods.length
o.indent += TAB
result = []
@ -1356,6 +1362,7 @@ exports.Class = class Class extends Base
walkBody: ->
@ctor = null
@boundMethods = []
executableBody = null
initializer = []
@ -1401,6 +1408,8 @@ exports.Class = class Class extends Base
@ctor = method
else if method.isStatic and method.bound
method.context = @name
else if method.bound
@boundMethods.push method
if initializer.length isnt expressions.length
@body.expressions = (expression.hoist() for expression in initializer)
@ -1438,7 +1447,7 @@ exports.Class = class Class extends Base
method.name = new (if methodName.shouldCache() then Index else Access) methodName
method.name.updateLocationDataIfMissing methodName.locationData
method.ctor = (if @parent then 'derived' else 'base') if methodName.value is 'constructor'
method.error 'Methods cannot be bound functions' if method.bound
method.error 'Cannot define a constructor as a bound (fat arrow) function' if method.bound and method.ctor
method
@ -1457,6 +1466,15 @@ exports.Class = class Class extends Base
ctor
proxyBoundMethods: ->
@ctor.thisAssignments = for method in @boundMethods
method.classVariable = @variableRef if @parent
name = new Value(new ThisLiteral, [ method.name ])
new Assign name, new Call(new Value(name, [new Access new PropertyName 'bind']), [new ThisLiteral])
null
exports.ExecutableClassBody = class ExecutableClassBody extends Base
children: [ 'class', 'body' ]
@ -1540,7 +1558,7 @@ exports.ExecutableClassBody = class ExecutableClassBody extends Base
@body.traverseChildren false, (node) =>
if node instanceof ThisLiteral
node.value = @name
else if node instanceof Code and node.bound
else if node instanceof Code and node.bound and node.isStatic
node.context = @name
# Make class/prototype assignments for invalid ES properties
@ -2180,6 +2198,9 @@ exports.Code = class Code extends Base
wasEmpty = @body.isEmpty()
@body.expressions.unshift thisAssignments... unless @expandCtorSuper thisAssignments
@body.expressions.unshift exprs...
if @isMethod and @bound and not @isStatic and @classVariable
boundMethodCheck = new Value new Literal utility 'boundMethodCheck', o
@body.expressions.unshift new Call(boundMethodCheck, [new Value(new ThisLiteral), @classVariable])
@body.makeReturn() unless wasEmpty or @noReturn
# Assemble the output
@ -3149,6 +3170,13 @@ exports.If = class If extends Base
UTILITIES =
modulo: -> 'function(a, b) { return (+a % (b = +b) + b) % b; }'
boundMethodCheck: -> "
function(instance, Constructor) {
if (!(instance instanceof Constructor)) {
throw new Error('Bound instance method accessed before binding');
}
}
"
# Shortcuts to speed up the lookup time for native functions.
hasProp: -> '{}.hasOwnProperty'

View File

@ -562,8 +562,9 @@ test "Assignment to variables similar to helper functions", ->
extend = 3
hasProp = 4
value: 5
method: (bind, bind1) -> [bind, bind1, extend, hasProp, @value]
arrayEq [1, 2, 3, 4, 5], new B().method 1, 2
method: (bind, bind1) => [bind, bind1, extend, hasProp, @value]
{method} = new B
arrayEq [1, 2, 3, 4, 5], method 1, 2
modulo = -1 %% 3
eq 2, modulo

View File

@ -136,20 +136,42 @@ test "classes with JS-keyword properties", ->
ok instance.name() is 'class'
test "Classes with methods that are pre-bound statically, to the class", ->
test "Classes with methods that are pre-bound to the instance, or statically, to the class", ->
class Dog
constructor: (name) ->
@name = name
bark: =>
"#{@name} woofs!"
@static = =>
new this('Dog')
spark = new Dog('Spark')
fido = new Dog('Fido')
fido.bark = spark.bark
ok fido.bark() is 'Spark woofs!'
obj = func: Dog.static
ok obj.func().name is 'Dog'
test "a bound function in a bound function", ->
class Mini
num: 10
generate: =>
for i in [1..3]
=>
@num
m = new Mini
eq (func() for func in m.generate()).join(' '), '10 10 10'
test "contructor called with varargs", ->
class Connection
@ -454,6 +476,21 @@ test "#1182: execution order needs to be considered as well", ->
@B: makeFn 2
constructor: makeFn 3
test "#1182: external constructors with bound functions", ->
fn = ->
{one: 1}
this
class B
class A
constructor: fn
method: => this instanceof A
ok (new A).method.call(new B)
test "#1372: bound class methods with reserved names", ->
class C
delete: =>
ok C::delete
test "#1380: `super` with reserved names", ->
class C
do: -> super()
@ -522,7 +559,7 @@ test "#1842: Regression with bound functions within bound class methods", ->
@unbound: ->
eq this, Store
instance: ->
instance: =>
ok this instanceof Store
Store.bound()
@ -685,6 +722,57 @@ test "extending native objects works with and without defining a constructor", -
ok overrideArray instanceof OverrideArray
eq 'yes!', overrideArray.method()
test "#2782: non-alphanumeric-named bound functions", ->
class A
'b:c': =>
'd'
eq (new A)['b:c'](), 'd'
test "#2781: overriding bound functions", ->
class A
a: ->
@b()
b: =>
1
class B extends A
b: =>
2
b = (new A).b
eq b(), 1
b = (new B).b
eq b(), 2
test "#2791: bound function with destructured argument", ->
class Foo
method: ({a}) => 'Bar'
eq (new Foo).method({a: 'Bar'}), 'Bar'
test "#2796: ditto, ditto, ditto", ->
answer = null
outsideMethod = (func) ->
func.call message: 'wrong!'
class Base
constructor: ->
@message = 'right!'
outsideMethod @echo
echo: =>
answer = @message
new Base
eq answer, 'right!'
test "#3063: Class bodies cannot contain pure statements", ->
throws -> CoffeeScript.compile """
class extends S
@ -900,6 +988,9 @@ test "`this` access after `super` in extended classes", ->
eq result.super, this
eq result.param, @param
eq result.method, @method
ok result.method isnt Test::method
method: =>
nonce = {}
new Test nonce, {}
@ -919,6 +1010,8 @@ test "`@`-params and bound methods with multiple `super` paths (blocks)", ->
super 'not param'
eq @name, 'not param'
eq @param, nonce
ok @method isnt Test::method
method: =>
new Test true, nonce
new Test false, nonce
@ -937,13 +1030,16 @@ test "`@`-params and bound methods with multiple `super` paths (expressions)", -
eq (super 'param'), @;
eq @name, 'param';
eq @param, nonce;
ok @method isnt Test::method
)
else
result = (
eq (super 'not param'), @;
eq @name, 'not param';
eq @param, nonce;
ok @method isnt Test::method
)
method: =>
new Test true, nonce
new Test false, nonce
@ -1145,7 +1241,7 @@ test "super in a bound function", ->
make: -> "Making a #{@drink}"
class B extends A
make: (@flavor) ->
make: (@flavor) =>
super() + " with #{@flavor}"
b = new B('Machiato')
@ -1153,7 +1249,7 @@ test "super in a bound function", ->
# super in a bound function in a bound function
class C extends A
make: (@flavor) ->
make: (@flavor) =>
func = () =>
super() + " with #{@flavor}"
func()
@ -1578,3 +1674,137 @@ test "CS6 Class extends a CS1 compiled class with super()", ->
eq B.className(), 'ExtendedCS1'
b = new B('three')
eq b.make(), "making a cafe ole with caramel and three shots of espresso"
test 'Bound method called normally before binding is ok', ->
class Base
constructor: ->
@setProp()
eq @derivedBound(), 3
class Derived extends Base
setProp: ->
@prop = 3
derivedBound: =>
@prop
d = new Derived
test 'Bound method called as callback after super() is ok', ->
class Base
class Derived extends Base
constructor: (@prop = 3) ->
super()
f = @derivedBound
eq f(), 3
derivedBound: =>
@prop
d = new Derived
{derivedBound} = d
eq derivedBound(), 3
test 'Bound method of base class called as callback is ok', ->
class Base
constructor: (@prop = 3) ->
f = @baseBound
eq f(), 3
baseBound: =>
@prop
b = new Base
{baseBound} = b
eq baseBound(), 3
test 'Bound method of prop-named class called as callback is ok', ->
Hive = {}
class Hive.Bee
constructor: (@prop = 3) ->
f = @baseBound
eq f(), 3
baseBound: =>
@prop
b = new Hive.Bee
{baseBound} = b
eq baseBound(), 3
test 'Bound method of class with expression base class called as callback is ok', ->
calledB = no
B = ->
throw new Error if calledB
calledB = yes
class
class A extends B()
constructor: (@prop = 3) ->
super()
f = @derivedBound
eq f(), 3
derivedBound: =>
@prop
b = new A
{derivedBound} = b
eq derivedBound(), 3
test 'Bound method of class with expression class name called as callback is ok', ->
calledF = no
obj = {}
B = class
f = ->
throw new Error if calledF
calledF = yes
obj
class f().A extends B
constructor: (@prop = 3) ->
super()
g = @derivedBound
eq g(), 3
derivedBound: =>
@prop
a = new obj.A
{derivedBound} = a
eq derivedBound(), 3
test 'Bound method of anonymous child class called as callback is ok', ->
f = ->
B = class
class extends B
constructor: (@prop = 3) ->
super()
g = @derivedBound
eq g(), 3
derivedBound: =>
@prop
a = new (f())
{derivedBound} = a
eq derivedBound(), 3
test 'Bound method of immediately instantiated class with expression base class called as callback is ok', ->
calledF = no
obj = {}
B = class
f = ->
throw new Error if calledF
calledF = yes
obj
a = new class f().A extends B
constructor: (@prop = 3) ->
super()
g = @derivedBound
eq g(), 3
derivedBound: =>
@prop
{derivedBound} = a
eq derivedBound(), 3

View File

@ -1585,3 +1585,18 @@ test "CSX error: ambiguous tag-like expression", ->
x = a <b > c
^
'''
test 'Bound method called as callback before binding throws runtime error', ->
class Base
constructor: ->
f = @derivedBound
try
f()
ok no
catch e
eq e.message, 'Bound instance method accessed before binding'
class Derived extends Base
derivedBound: =>
ok no
d = new Derived

View File

@ -107,6 +107,16 @@ test "#1183: super + closures", ->
ret
eq (new B).foo(), 10
test "#2331: bound super regression", ->
class A
@value = 'A'
method: -> @constructor.value
class B extends A
method: => super()
eq (new B).method(), 'A'
test "#3259: leak with @-params within destructured parameters", ->
fn = ({@foo}, [@bar], [{@baz}]) ->
foo = bar = baz = false