LGTM, with comments.

You often use
Tagging::HeapObjectToAddress(this) when this is a subtype of HeapObject. Isn't
this just
this->address()
or even
address()?
I think it is much easier to read.

There are also cases
Tagging::HeapObjectToAddress(foo), which could be
foo->address().



http://codereview.chromium.org/3567007/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/3567007/diff/1/2#newcode3255
include/v8.h:3255: static inline uint8_t* HeapObjectToAddress(Object*
object) {
Comment that uint8_t* is the definition of Address in globals.h.

http://codereview.chromium.org/3567007/diff/1/2#newcode3262
include/v8.h:3262: static inline bool HasNoHeapObjectTag(intptr_t
address) {
It seems strange to have both Has and HasNot HOT, taking different types
of arguments.

http://codereview.chromium.org/3567007/diff/1/11
File src/objects-inl.h (right):

http://codereview.chromium.org/3567007/diff/1/11#newcode1970
src/objects-inl.h:1970: return Tagging::HeapObjectToAddress(this) +
kHeaderSize;
this->address() + kHeaderSize?

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

http://codereview.chromium.org/3567007/diff/1/12#newcode4148
src/objects.cc:4148: Tagging::HeapObjectToAddress(this) + kHeaderSize) +
start;
address() + kHeaderSize?

http://codereview.chromium.org/3567007/show

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

Reply via email to