LGTM

http://codereview.chromium.org/954001/diff/4001/5002
File src/builtins.cc (right):

http://codereview.chromium.org/954001/diff/4001/5002#newcode742
src/builtins.cc:742: if (!IsJSArrayWithFastElements(receiver_obj,
&receiver_elms)
I personally don't like test functions with side effects. Could you just
do the assignments after the if-statement instead?

http://codereview.chromium.org/954001/diff/4001/5002#newcode755
src/builtins.cc:755: ASSERT(receiver_len <= (Smi::kMaxValue - arg_len));
Maybe comment that this assertion holds because both arrays have fast
elements (possibly rewriting it in terms of FixedArray::kMaxLength).

http://codereview.chromium.org/954001/diff/4001/5002#newcode758
src/builtins.cc:758: if (result_len > FixedArray::kMaxSize) {
FixedArray::kMaxLength (length is in elements, size is in bytes).

http://codereview.chromium.org/954001

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

Reply via email to