LGTM

http://codereview.chromium.org/92123/diff/9/1008
File src/array.js (right):

http://codereview.chromium.org/92123/diff/9/1008#newcode713
Line 713: if (!(length >= 2)) return this;
I find the introduction of a negation makes this harder to read.

http://codereview.chromium.org/92123/diff/9/1010
File src/objects.h (right):

http://codereview.chromium.org/92123/diff/9/1010#newcode1182
Line 1182: // Undefined values are placed after non-un defined values.
spurious space.

http://codereview.chromium.org/92123/diff/3001/3003
File src/objects.cc (right):

http://codereview.chromium.org/92123/diff/3001/3003#newcode6400
Line 6400: static const uint32_t kMaxUInt32 = 4294967295u;
This should probably go next to kMaxInt in globals.h.  And in hex!

http://codereview.chromium.org/92123/diff/3001/3003#newcode6406
Line 6406: HeapNumber* result_double;
Please initialize to NULL and ASSERT it is not null before using.

http://codereview.chromium.org/92123/diff/3001/3003#newcode6498
Line 6498: limit = elements_length ;
Do we have a test that excercises this?

http://codereview.chromium.org/92123/diff/3001/3003#newcode6504
Line 6504: HeapNumber* result_double;
And here.

http://codereview.chromium.org/92123

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

Reply via email to