LGTM
http://codereview.chromium.org/7068009/diff/1/src/compiler.h File src/compiler.h (right): http://codereview.chromium.org/7068009/diff/1/src/compiler.h#newcode92 src/compiler.h:92: void MarkAsES5Native() { Why ES5Native and not just Native? Is there any native code that isn't ES5Native? http://codereview.chromium.org/7068009/diff/1/src/v8natives.js File src/v8natives.js (right): http://codereview.chromium.org/7068009/diff/1/src/v8natives.js#newcode136 src/v8natives.js:136: if (receiver == null) receiver = %GlobalReceiver(global); Also if undetectable? http://codereview.chromium.org/7068009/diff/1/src/v8natives.js#newcode138 src/v8natives.js:138: var global_receiver = %GlobalReceiver(global); Duplicate call to runtime function %GlobalReceiver(global). Move this to before the previous line and assign receiver = global_receiver. http://codereview.chromium.org/7068009/diff/1/src/v8natives.js#newcode142 src/v8natives.js:142: if (!this_is_global_receiver || global_is_detached) { Could we make a comment that this test is jsut to be consistent with JSC, and is not required by the specification. http://codereview.chromium.org/7068009/diff/1/src/x64/full-codegen-x64.cc File src/x64/full-codegen-x64.cc (right): http://codereview.chromium.org/7068009/diff/1/src/x64/full-codegen-x64.cc#newcode137 src/x64/full-codegen-x64.cc:137: // function calls. Do we know, or could we make sure we know, that the non-zero part of rcx is in the low 32 bits (i.e., in ecx)? In that case, we can do with a testl(rcx, rcx) below (and save a byte!) http://codereview.chromium.org/7068009/diff/1/test/mjsunit/regress/regress-1365.js File test/mjsunit/regress/regress-1365.js (right): http://codereview.chromium.org/7068009/diff/1/test/mjsunit/regress/regress-1365.js#newcode42 test/mjsunit/regress/regress-1365.js:42: assertTrue(exception); Test for thrown exceptions by either: assertThrows(function() { valueOf(); }) (possibly even assertThrows(valueOf) , but in this case we want to be in total control of how the function is called) or: try { valueOf(); assertUnreachable(); } catch (e) {} http://codereview.chromium.org/7068009/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev
