More to come!
http://codereview.chromium.org/3023033/diff/5001/6002 File src/arm/assembler-arm-inl.h (right): http://codereview.chromium.org/3023033/diff/5001/6002#newcode173 src/arm/assembler-arm-inl.h:173: void RelocInfo::Visit(ObjectVisitor* visitor) { Can't you just replace this entire implementation with a call to the templatized version now? http://codereview.chromium.org/3023033/diff/5001/6004 File src/bootstrapper.cc (right): http://codereview.chromium.org/3023033/diff/5001/6004#newcode42 src/bootstrapper.cc:42: #include "objects-iteration.h" Can't this be with the others in alphabetical order? http://codereview.chromium.org/3023033/diff/5001/6006 File src/heap.cc (right): http://codereview.chromium.org/3023033/diff/5001/6006#newcode48 src/heap.cc:48: #include "objects-iteration.h" Alpha order? http://codereview.chromium.org/3023033/diff/5001/6006#newcode1054 src/heap.cc:1054: class NewSpaceScavenger : public StaticNewSpaceVisitor<NewSpaceScavenger> { Perhaps a comment that this is http://en.wikipedia.org/wiki/Curiously_recurring_template_pattern http://codereview.chromium.org/3023033/diff/5001/6012 File src/objects-inl.h (right): http://codereview.chromium.org/3023033/diff/5001/6012#newcode2094 src/objects-inl.h:2094: int HeapObject::SizeFromMap(Map* map) { Not worth making this into a virtually dispatched getter? http://codereview.chromium.org/3023033/diff/5001/6016 File src/objects.h (left): http://codereview.chromium.org/3023033/diff/5001/6016#oldcode3119 src/objects.h:3119: // Dispatched behavior. this comment was accidentally deleted http://codereview.chromium.org/3023033/diff/5001/6016 File src/objects.h (right): http://codereview.chromium.org/3023033/diff/5001/6016#newcode1114 src/objects.h:1114: class FixedBodyDescriptor { Seems like this can describe any fixed size object where the heap object pointers are in one consecutive group? If so, I think it would be fitting to have a comment saying so. end_offset is not inclusive? http://codereview.chromium.org/3023033/diff/5001/6016#newcode1128 src/objects.h:1128: Should be two blank lines here. http://codereview.chromium.org/3023033/diff/5001/6016#newcode1130 src/objects.h:1130: class FlexibleBodyDescriptor { Seems like this can describe any variable size object where the heap object pointers are in one consecutive group at the end of the object? If so, I think it would be fitting to have a comment saying so. http://codereview.chromium.org/3023033/diff/5001/6016#newcode2519 src/objects.h:2519: class BodyDescriptor : public FlexibleBodyDescriptor<kHeaderSize> { Does a byte array need a flexible body descriptor? http://codereview.chromium.org/3023033/diff/5001/6016#newcode5453 src/objects.h:5453: Missing blank line here. http://codereview.chromium.org/3023033/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
