LGTM with two final nits. Note that I only looked at the elements accessors.
https://codereview.chromium.org/11365111/diff/1/src/elements.cc File src/elements.cc (right): https://codereview.chromium.org/11365111/diff/1/src/elements.cc#newcode585 src/elements.cc:585: ? NONE : ABSENT; On 2012/11/07 18:41:42, rossberg wrote:
That was wrong.
Nice catch. https://codereview.chromium.org/11365111/diff/5002/src/elements.cc File src/elements.cc (right): https://codereview.chromium.org/11365111/diff/5002/src/elements.cc#newcode584 src/elements.cc:584: if (key >= ElementsAccessorSubclass::GetCapacityImpl(backing_store)) Curly brackets around body. https://codereview.chromium.org/11365111/diff/5002/src/elements.cc#newcode1543 src/elements.cc:1543: // Aliased parameters and non-aliased elements in a fast backing store I think we can drop all three lines of comments. It adds no valuable information here. At least this is the wrong place to describe our crazy arguments object layout. https://codereview.chromium.org/11365111/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
