I am not too happy about the assertions. Otherwise, it is looking good.


https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcode4057
src/objects-inl.h:4057: ASSERT(kInstanceSizeOffset % kPointerSize == 0);
STATIC_ASSERT?

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcode4303
src/objects-inl.h:4303: ASSERT(kInstanceSizeOffset % kPointerSize == 0);
STATIC_ASSERT? Tiny nit - since you are using Atomic32, you only really
need kInstanceSizeOffset % sizeof(Atomic32) == 0.

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcode4315
src/objects-inl.h:4315: int difference = (slack << 8) + slack;
Replace 8 with  kInObjectPropertiesByte * kBitsPerByte, also might want
STATIC_ASSERT that kInObjectPropertiesByte < sizeof(Atomic32)

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcode4320
src/objects-inl.h:4320: ((first_word & 0xff00) >> 8) < 256);
The two assertions above do not seem to test any interesting property,
this should hold for any value of first_word, no?

Maybe we want an assertion testing that slack <= object_size and slack
<= in_object_properties. I was imagining something like this: (inserted
before we do first_word -= difference)

ASSERT(slack <= (first_word & 0xff));
ASSERT(slack <= ((first_word >> kInObjectPropertiesByte * kBitsPerByte)
& 0xff));

https://codereview.chromium.org/227133007/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to