Thanks a lot for review, Lasse.
Running throw the tests and submitting.
http://codereview.chromium.org/1036002/diff/1/2
File src/builtins.cc (right):
http://codereview.chromium.org/1036002/diff/1/2#newcode741
src/builtins.cc:741: FixedArray* elms = NULL;
On 2010/03/16 10:33:30, Lasse Reichstein wrote:
elms isn't used anywhere, so the function with side-effect is wasted
here.
Wouldn't it be simpler to just check manually:
if(!arg->IsJSArray() || !JSArray::cast(arg)->HasFastElements()) {
Sure.
http://codereview.chromium.org/1036002/diff/1/2#newcode748
src/builtins.cc:748: result_len += len;
On 2010/03/16 09:50:14, Lasse Reichstein wrote:
Might want comment stating why this can't overflow:
(FixedArray::kMaxLength <
(max positive int / 2)), so being <= kMaxLength means that adding
another
array's length won't overflow.
Done. And static assert added.
http://codereview.chromium.org/1036002
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
