LGTM (with a bunch of nits).
http://codereview.chromium.org/11085070/diff/17001/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/11085070/diff/17001/src/heap.cc#newcode382 src/heap.cc:382: Two empty newlines between function implementations. http://codereview.chromium.org/11085070/diff/17001/src/heap.cc#newcode658 src/heap.cc:658: Two empty newlines between function implementations. http://codereview.chromium.org/11085070/diff/17001/src/heap.cc#newcode4705 src/heap.cc:4705: ? new_space_.AllocateRaw(size) Indentation is off. http://codereview.chromium.org/11085070/diff/17001/src/heap.h File src/heap.h (right): http://codereview.chromium.org/11085070/diff/17001/src/heap.h#newcode1272 src/heap.h:1272: Drop one of the two empty newlines. http://codereview.chromium.org/11085070/diff/17001/src/objects-debug.cc File src/objects-debug.cc (right): http://codereview.chromium.org/11085070/diff/17001/src/objects-debug.cc#newcode785 src/objects-debug.cc:785: Two empty newlines between function implementations. http://codereview.chromium.org/11085070/diff/17001/src/objects-debug.cc#newcode825 src/objects-debug.cc:825: Two empty newlines between function implementations. http://codereview.chromium.org/11085070/diff/17001/src/objects-inl.h File src/objects-inl.h (right): http://codereview.chromium.org/11085070/diff/17001/src/objects-inl.h#newcode3632 src/objects-inl.h:3632: if (Heap::ShouldZapGarbage()) { Let's also use the following here, it's equivalent and more concise: if (Heap::ShouldZapGarbage() && HasTransitionArray()) { ZapTransitions(); } http://codereview.chromium.org/11085070/diff/17001/src/objects.h File src/objects.h (right): http://codereview.chromium.org/11085070/diff/17001/src/objects.h#newcode1250 src/objects.h:1250: Dropping these two newlines would be consistent with the rest of the file. http://codereview.chromium.org/11085070/diff/17001/src/objects.h#newcode2452 src/objects.h:2452: Dropping this newline would be consistent with the rest of the file. http://codereview.chromium.org/11085070/diff/17001/src/objects.h#newcode8813 src/objects.h:8813: #undef DECL_ACCESSORS We should also undef the DECLARE_VERIFIER macro here. http://codereview.chromium.org/11085070/diff/17001/src/spaces.cc File src/spaces.cc (right): http://codereview.chromium.org/11085070/diff/17001/src/spaces.cc#newcode2558 src/spaces.cc:2558: // MVSTANTON: this is weird...the compiler can't make a vtable unless there is If you want to leave that comment in, format it like follows: TODO(mvstanton): This is weird ... http://codereview.chromium.org/11085070/diff/17001/src/spaces.cc#newcode2569 src/spaces.cc:2569: // MVSTANTON: same as above Likewise. http://codereview.chromium.org/11085070/diff/17001/src/spaces.cc#newcode2788 src/spaces.cc:2788: Two empty newlines between function implementations. http://codereview.chromium.org/11085070/diff/17001/src/spaces.cc#newcode2838 src/spaces.cc:2838: Two empty newlines between function implementations. http://codereview.chromium.org/11085070/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
