LGTM (with nits). Once comments are addressed I'll land this for you.
http://codereview.chromium.org/8467010/diff/1/src/objects.cc File src/objects.cc (right): http://codereview.chromium.org/8467010/diff/1/src/objects.cc#newcode7729 src/objects.cc:7729: VisitExternalReferences(p, p+1); Space before and after operator (i.e. "p + 1"). http://codereview.chromium.org/8467010/diff/1/src/serialize.cc File src/serialize.cc (right): http://codereview.chromium.org/8467010/diff/1/src/serialize.cc#newcode1477 src/serialize.cc:1477: if(!(*current)->IsSmi()) { Does the address actually ever get to be a SMI here? If yes, then I'm fine with the check. Otherwise it should be removed. http://codereview.chromium.org/8467010/diff/1/src/serialize.cc#newcode1478 src/serialize.cc:1478: OutputRawData(reinterpret_cast<Address>(rinfo->target_address_address())); No cast should be needed here. http://codereview.chromium.org/8467010/diff/1/src/serialize.cc#newcode1480 src/serialize.cc:1480: if(rinfo->IsCodedSpecially()) { Using the ternary operator here might make things more readable IMHO. http://codereview.chromium.org/8467010/diff/1/src/serialize.cc#newcode1507 src/serialize.cc:1507: reinterpret_cast<Address>(rinfo->target_address_address()); No cast should be needed here. http://codereview.chromium.org/8467010/diff/1/src/serialize.cc#newcode1510 src/serialize.cc:1510: reinterpret_cast<Address*>(rinfo->target_reference_address()); No cast should be needed here. http://codereview.chromium.org/8467010/diff/1/src/serialize.cc#newcode1512 src/serialize.cc:1512: if(rinfo->IsCodedSpecially()) { Using the ternary operator here might make things more readable IMHO. http://codereview.chromium.org/8467010/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
