Mads, please take another look.

Changed thisArg to this_arg and boundArgs to bound_args.
Refactored argument counts to simplify both binding and length calculations.
Added tests for lengths.


http://codereview.chromium.org/3046010/diff/10001/11003
File src/v8natives.js (right):

http://codereview.chromium.org/3046010/diff/10001/11003#newcode1126
src/v8natives.js:1126: args.unshift.apply(args, boundArgs);
On 2010/07/23 06:31:29, Mads Ager wrote:
Could we allocate a bigger array up front and fill it with boundArgs
and args
instead of using unshift here?

Done.

http://codereview.chromium.org/3046010/diff/10001/11003#newcode1134
src/v8natives.js:1134: } else {
On 2010/07/23 06:31:29, Mads Ager wrote:
We should be able to get rid of the else here. There is little
difference
between if and else branch.

Done.

http://codereview.chromium.org/3046010/diff/10001/11003#newcode1150
src/v8natives.js:1150: // try to redefine these as defined by the spec.
On 2010/07/23 06:31:29, Mads Ager wrote:
Could you add to the comment what the spec says?

Done.

http://codereview.chromium.org/3046010/diff/10001/11003#newcode1153
src/v8natives.js:1153: %FunctionSetLength(result, $Math.max(0,
this.length - (n-1)));
On 2010/07/23 06:31:29, Mads Ager wrote:
Math.max can be overwritten. Use simple conditional instead.

Done.

http://codereview.chromium.org/3046010/show

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

Reply via email to