LGTM, but it could use a few extra comments and there seems to be something fishy going on if deleted_index is 0. This should probably be addressed before you submit.
http://codereview.chromium.org/3140/diff/1/2 File src/objects.cc (right): http://codereview.chromium.org/3140/diff/1/2#newcode2390 Line 2390: if (deleted_index > 0) i = deleted_index; What if deleted_index is 0? Shouldn't you use that slot then? http://codereview.chromium.org/3140/diff/1/2#newcode2406 Line 2406: if (deleted_index > 0) { How about reusing if deleted_index is 0? Isn't that the right thing to do? http://codereview.chromium.org/3140/diff/1/2#newcode2432 Line 2432: if (key->IsNull()) { Maybe add a comment here that explains that we're skipping deleted entries? http://codereview.chromium.org/3140/diff/1/2#newcode2460 Line 2460: array->set_null(index - 1); // key Add comment that explains that deleted entries are dealt with specially when looking up in the cache so they can't be undefined (must be null). http://codereview.chromium.org/3140 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
