LGTM (a few comments, but mainly style issues).

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/objects.cc#newcode7563
src/objects.cc:7563: RightTrimFixedArray<FROM_GC>(heap, t, trim);
Can we use "t->IsSimpleTransition() ? trim : trim *
TransitionArray::kTransitionSize" here?

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/objects.h#newcode179
src/objects.h:179:
Insert second empty newline here.

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/objects.h#newcode180
src/objects.h:180: // Indicates whether the transition is simple: it
either adds a property to a
The comment is slightly confusing to me. Can we rephrase the second
part? What is "it" in this sentence?

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/objects.h#newcode186
src/objects.h:186:
Insert second empty newline here.

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/objects.h#newcode4849
src/objects.h:4849: SimpleTransitionFlag flag = SIMPLE_TRANSITION);
The Google C++ Style Guide doesn't like default arguments. We sometimes
break that rule. But since this method only has two call-sites, I think
it's not worth here.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Default_Arguments

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/objects.h#newcode5001
src/objects.h:5001: int descriptor_index = 0);
Likewise. Although there are more call-sites here. But they are all in
objects.cc, so I also think it's not worth here.

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/transitions-inl.h
File src/transitions-inl.h (right):

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/transitions-inl.h#newcode73
src/transitions-inl.h:73: return get(kElementsTransitionIndex) !=
Smi::FromInt(0);
Can we turn that into ...

return IsFullTransitionArray() &&
    get(kElementsTransitionIndex) != Smi::FromInt(0);

https://chromiumcodereview.appspot.com/10915260/diff/16001/src/transitions-inl.h#newcode128
src/transitions-inl.h:128: return prototype_transitions !=
Smi::FromInt(0);
Can we turn that into ...

return IsFullTransitionArray() &&
    get(kPrototypeTransitionsIndex) != Smi::FromInt(0);

https://chromiumcodereview.appspot.com/10915260/

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

Reply via email to