Thanks a lot for review, Kasper. Submitting.
http://codereview.chromium.org/3595013/diff/1/2
File src/handles.cc (right):
http://codereview.chromium.org/3595013/diff/1/2#newcode638
src/handles.cc:638: static void CheckKeys(Handle<FixedArray> array) {
On 2010/10/05 12:39:12, Kasper Lund wrote:
Maybe turns this into a function that returns a bool and call it like:
ASSERT(ContainsOnlyValidKeys(array))
and let the implementation be valid in release mode too. Less
ifdef'ing makes me
happy.
Done. That requires USE for Release mode though
http://codereview.chromium.org/3595013/diff/1/3
File src/objects.cc (right):
http://codereview.chromium.org/3595013/diff/1/3#newcode3606
src/objects.cc:3606: // We cannot optimize if this is empty, as other my
have holes
On 2010/10/05 12:39:12, Kasper Lund wrote:
Maybe quote 'this'. Maybe explain why 'this' cannot have holes.
my => may
not keys => non keys?
Almost done. I failed to find proper explanation why this shouldn't
contain any holes and just added another check, even though it is
somewhat redundant.
http://codereview.chromium.org/3595013/diff/1/4
File test/cctest/test-api.cc (right):
http://codereview.chromium.org/3595013/diff/1/4#newcode11486
test/cctest/test-api.cc:11486:
On 2010/10/05 12:39:12, Kasper Lund wrote:
Two newlines between the functions definitions.
Sorry, all addressed, haven't done enough style adjustment to foreign
code, mea culpa.
http://codereview.chromium.org/3595013/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
