LGTM

Nice cleanup!


http://codereview.chromium.org/647015/diff/1/6
File src/bootstrapper.cc (right):

http://codereview.chromium.org/647015/diff/1/6#newcode1326
src/bootstrapper.cc:1326:
Not related to your change, but it looks as if this function works
differently if from is in fast case. If from is in fast the to is only
checked for existing property if we are looking at transfering an
accessor, whereas if from is in slow case we check to for the existence
of the property for all properties.

http://codereview.chromium.org/647015/diff/1/4
File src/objects.cc (right):

http://codereview.chromium.org/647015/diff/1/4#newcode1464
src/objects.cc:1464: if (result.IsPropertyOrTransition()) {
I am not sure about NULL_DESCRIPTOR here? SetProperty has code to
"convert" a NULL_DESCRIPTOR to a field the same way it does for
CONSTANT_TRANSITION. I guess AddProperty will will end up overwriting
the NULL_DESCRIPTOR with a MAP_TRANSITION or CONSTANT_TRANSITION.

http://codereview.chromium.org/647015/diff/1/4#newcode1762
src/objects.cc:1762: if (result->IsPropertyOrTransition()) {
Why do we want transitions here?

http://codereview.chromium.org/647015/diff/1/4#newcode1968
src/objects.cc:1968: // ADDED TO CLONE
Please silence this comment :-).

http://codereview.chromium.org/647015/diff/1/11
File src/property.h (right):

http://codereview.chromium.org/647015/diff/1/11#newcode213
src/property.h:213: bool IsTransitionType() {
IsTransitionType -> IsTransition or maybe just remove it, as I think you
have removed the only use of it.

http://codereview.chromium.org/647015

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

Reply via email to