LGTM with one real comment and a few nits.
https://codereview.chromium.org/11413179/diff/1/src/builtins.cc File src/builtins.cc (right): https://codereview.chromium.org/11413179/diff/1/src/builtins.cc#newcode579 src/builtins.cc:579: accessor->CopyElements( Move the accessor->CopyElements into the first line, this will get rid of one indentation level. https://codereview.chromium.org/11413179/diff/1/src/builtins.cc#newcode627 src/builtins.cc:627: accessor->CopyElements( Likewise. https://codereview.chromium.org/11413179/diff/1/src/builtins.cc#newcode790 src/builtins.cc:790: accessor->CopyElements( Likewise. https://codereview.chromium.org/11413179/diff/1/src/elements.cc File src/elements.cc (right): https://codereview.chromium.org/11413179/diff/1/src/elements.cc#newcode156 src/elements.cc:156: ASSERT(to->map() != HEAP->fixed_cow_array_map()); We are assuming that this function does not allocate, so we should add a no-allocation scope to the top of the function. AssertNoAllocation no_allocation; https://codereview.chromium.org/11413179/diff/1/src/elements.cc#newcode201 src/elements.cc:201: int copy_size = raw_copy_size; Likewise. https://codereview.chromium.org/11413179/diff/1/test/mjsunit/regress/regress-121407.js File test/mjsunit/regress/regress-121407.js (right): https://codereview.chromium.org/11413179/diff/1/test/mjsunit/regress/regress-121407.js#newcode40 test/mjsunit/regress/regress-121407.js:40: } I hope this adds a trailing newline instead of removing one? I cannot tell from the diff. https://codereview.chromium.org/11413179/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
