http://codereview.chromium.org/10876067/diff/1/src/arm/full-codegen-arm.cc File src/arm/full-codegen-arm.cc (right):
http://codereview.chromium.org/10876067/diff/1/src/arm/full-codegen-arm.cc#newcode189 src/arm/full-codegen-arm.cc:189: // Argument to NewContext is the function, which is still in edi. On 2012/08/27 06:24:54, Sven Panne wrote:
copy-n-paste typo: should be r1. Consider moving the push before the
if. Done. http://codereview.chromium.org/10876067/diff/1/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/10876067/diff/1/src/heap.cc#newcode2442 src/heap.cc:2442: if (!maybe_obj->ToObject(&obj)) return false; On 2012/08/27 06:24:54, Sven Panne wrote:
Consider templatized 'To'
Yeah, none of the other functions around here does, so I'd thought I'd rather be consistent. Cleaning all that up is a separate gardening todo. http://codereview.chromium.org/10876067/diff/1/src/ia32/full-codegen-ia32.cc File src/ia32/full-codegen-ia32.cc (right): http://codereview.chromium.org/10876067/diff/1/src/ia32/full-codegen-ia32.cc#newcode184 src/ia32/full-codegen-ia32.cc:184: __ push(edi); On 2012/08/27 06:24:54, Sven Panne wrote:
Consider moving the push before the if.
Done. http://codereview.chromium.org/10876067/diff/1/src/mips/full-codegen-mips.cc File src/mips/full-codegen-mips.cc (right): http://codereview.chromium.org/10876067/diff/1/src/mips/full-codegen-mips.cc#newcode197 src/mips/full-codegen-mips.cc:197: // Argument to NewContext is the function, which is still in edi. On 2012/08/27 06:24:54, Sven Panne wrote:
copy-n-paste typo: should be a1. Consider moving the push before the
if. Done. http://codereview.chromium.org/10876067/diff/1/src/objects-inl.h File src/objects-inl.h (right): http://codereview.chromium.org/10876067/diff/1/src/objects-inl.h#newcode571 src/objects-inl.h:571: if (Object::IsHeapObject()) { On 2012/08/27 06:24:54, Sven Panne wrote:
Using 'if (!Object::IsHeapObject()) return false;' and having
straight-line code
would be clearer IMHO.
Done. http://codereview.chromium.org/10876067/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/10876067/diff/1/src/runtime.cc#newcode8750 src/runtime.cc:8750: if (!maybe_result->ToObject(&result)) return maybe_result; On 2012/08/27 06:24:54, Sven Panne wrote:
Make result a Context*, use "...->To(&result)" and remove the cast
below. Done (here and for NewFunctionContext below). http://codereview.chromium.org/10876067/diff/1/src/x64/full-codegen-x64.cc File src/x64/full-codegen-x64.cc (right): http://codereview.chromium.org/10876067/diff/1/src/x64/full-codegen-x64.cc#newcode179 src/x64/full-codegen-x64.cc:179: // Argument to NewContext is the function, which is still in edi. On 2012/08/27 06:24:54, Sven Panne wrote:
copy-n-paste typo: should be rdi. Consider moving the push before the
if. Done. http://codereview.chromium.org/10876067/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
