LGTM with a few nits.

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.
copy-n-paste typo: should be r1. Consider moving the push before the if.

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;
Consider templatized 'To'

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);
Consider moving the push before the if.

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.
copy-n-paste typo: should be a1. Consider moving the push before the if.

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()) {
Using 'if (!Object::IsHeapObject()) return false;' and having
straight-line code would be clearer IMHO.

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;
Make result a Context*, use "...->To(&result)" and remove the cast
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.
copy-n-paste typo: should be rdi. Consider moving the push before the
if.

http://codereview.chromium.org/10876067/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to