LGTM

Please make sure that we have test cases for the different cases as for
Array.prototype.shift.


http://codereview.chromium.org/600124/diff/2001/3002
File src/builtins.cc (right):

http://codereview.chromium.org/600124/diff/2001/3002#newcode251
src/builtins.cc:251: if (to_add == 0)
Please either use one-liner or use braces around the body.

http://codereview.chromium.org/600124/diff/2001/3002#newcode318
src/builtins.cc:318: FixedArray* elms, JSObject* prototype) {
Please use one-parameter per line indentation here.

http://codereview.chromium.org/600124/diff/2001/3002#newcode366
src/builtins.cc:366: if (to_add == 0)
Braces or one-liner.

http://codereview.chromium.org/600124/diff/2001/3002#newcode399
src/builtins.cc:399: for (int index = 0; index < len; index++) {
Just use 'i' as above?

http://codereview.chromium.org/600124/diff/2001/3002#newcode409
src/builtins.cc:409: for (int index = 0; index < to_add; index++) {
Just use 'i' as above?

http://codereview.chromium.org/600124

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

Reply via email to