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.js#newcode107
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-function.js
File test/mjsunit/harmony/proxies-function.js (right):

http://codereview.chromium.org/8199004/diff/9001/test/mjsunit/harmony/proxies-function.js#newcode35
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-function.js#newcode51
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-function.js#newcode208
test/mjsunit/harmony/proxies-function.js:208: var f =
Proxy.createFunction(handler, callTrap);
None of this should actually need the length property.

http://codereview.chromium.org/8199004/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to