Reimplement Function.prototype.bind.

Reimplement Function.prototype.bind.

Make instanceof work correctly.

BUG=v8:893
Committed: http://code.google.com/p/v8/source/detail?r=9659

[PhistucK, 2011-10-10 06:44:44]

Just a passerby.

http://codereview.chromium.org/8199004/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/8199004/diff/1/src/objects.h#newcode4960
src/objects.h:4960: // Allows to use byte-witgh instructions.
Almost there, s/witgh/width.
;)

[Lasse Reichstein, 2011-10-13 13:10:38]

Andreas, any comments? I'm particularly wary at the proxy-interaction. I hope I
got it right.

http://codereview.chromium.org/8199004/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/8199004/diff/1/src/objects.h#newcode4960
src/objects.h:4960: // Allows to use byte-witgh instructions.
Good catch :)

[rossberg, 2011-10-13 14:25:01]

LGTM.

I just ran some additional proxy tests against this patch, and it seems to work
fine except for the length issue (see below).

http://codereview.chromium.org/8199004/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/8199004/diff/1/src/runtime.cc#newcode7993
src/runtime.cc:7993: bindee = Execution::GetFunctionDelegate(bindee);
Unfortunately, that is not quite right for proxies. For a proxy, the spec will
probably require reading the length property from the proxy itself (via the get
trap), and if it's not a positive integer, default to 0. That is what the
current code does (although I just realised that there is no test for that).

I guess that could be a TODO. Main annoyance is that with the new code, that
would have to be done in C++.

[Lasse Reichstein, 2011-10-14 11:19:37]

Please take another look.
I fixed the length calculation, by doing it in JS.
I also had to fix the proxies-function test, because we now need to have a
length property on the function proxy. Fixed the layout and added missing
semicolons while I was there (semicolons: it's not just a good idea, it's the
law).

http://codereview.chromium.org/8199004/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/8199004/diff/1/src/runtime.cc#newcode7993
src/runtime.cc:7993: bindee = Execution::GetFunctionDelegate(bindee);
I think it's an easy fix, if we just read the length from JavaScript and pass it
as an argument to the runtime function.
Reading the spec for .bind, there doesn't appear to be any user-visible action
between checking for the input being callable and reading the length property:
"Let L be the length property of Target minus the length of A". I guess this is
one of the cases that need to be rewritten to account for proxies (because it
assumes that it is a function, so it has an unconfigurable integer own property
called "length", so we don't have to be explicit about using [[Get]] and doing
conversions).

[rossberg, 2011-10-17 09:38:02]

Have you actually uploaded your latest changes? ;)

[Lasse Reichstein, 2011-10-17 10:53:16]

I did upload it ... just not with the correct issue number *cough*.
Please look now :)

[rossberg, 2011-10-17 11:49:08]

LGTM

> Fixed the layout and added missing
semicolons while I was there (semicolons: it's not just a good idea, it's the
law).

Hm, I would have preferred you not to do the latter at this point, since it will
likely produce quite a few merge conflicts with my open CLs...

http://codereview.chromium.org/8199004/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/8199004/diff/1/src/runtime.cc#newcode7993
src/runtime.cc:7993: bindee = Execution::GetFunctionDelegate(bindee);
On 2011/10/14 11:19:37, Lasse Reichstein wrote:
> I think it's an easy fix, if we just read the length from JavaScript and pass
it
> as an argument to the runtime function.
> Reading the spec for .bind, there doesn't appear to be any user-visible action
> between checking for the input being callable and reading the length property:
> "Let L be the length property of Target minus the length of A". I guess this
is
> one of the cases that need to be rewritten to account for proxies (because it
> assumes that it is a function, so it has an unconfigurable integer own
property
> called "length", so we don't have to be explicit about using [[Get]] and doing
> conversions).

Yes, that's one of the spec issues I raised. The current consensus is what I
desribed above.

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/function-bind.js
File test/mjsunit/function-bind.js (right):

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/function-bind.j...
test/mjsunit/function-bind.js:107: // Several parameters can be given, and given
in different bind invokations.
Typo: invocations

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
File test/mjsunit/harmony/proxies-function.js (right):

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
test/mjsunit/harmony/proxies-function.js:35: handler.fix = function() { return {
length: { value: 2 } }; };
This shouldn't be necessary, because Object.freeze always sets the length
property separately anyway.

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
test/mjsunit/harmony/proxies-function.js:51: var handler = {
Could you move this up to the helpers section?

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
test/mjsunit/harmony/proxies-function.js:208: var f =
Proxy.createFunction(handler, callTrap);
None of this should actually need the length property.

[Lasse Reichstein, 2011-10-17 12:38:39]

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/function-bind.js
File test/mjsunit/function-bind.js (right):

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/function-bind.j...
test/mjsunit/function-bind.js:107: // Several parameters can be given, and given
in different bind invokations.
Fixed.

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
File test/mjsunit/harmony/proxies-function.js (right):

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
test/mjsunit/harmony/proxies-function.js:32: 
I'll undo the manual semicolon insertions so you won't get too many conflicts -
then you can add them yourself :)

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
test/mjsunit/harmony/proxies-function.js:35: handler.fix = function() { return {
length: { value: 2 } }; };
Ok, removing fix for fix.

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
test/mjsunit/harmony/proxies-function.js:51: var handler = {
Done.

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
test/mjsunit/harmony/proxies-function.js:208: var f =
Proxy.createFunction(handler, callTrap);
Right, no call to bind here. Undoing.

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies...
test/mjsunit/harmony/proxies-function.js:247: fix: function() { return { length:
{ value: 2 } }; },
Removing length from fix here too.