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() {
On 2011/05/26 08:26:55, Lasse Reichstein wrote:
Why ES5Native and not just Native?
Is there any native code that isn't ES5Native?

Following naming elsewhere to not have two names for this. I'll rename
them all to remove the es5 part. Having the current spec version in the
name is not such a great idea. :-)

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);
On 2011/05/26 08:26:55, Lasse Reichstein wrote:
Also if undetectable?

No, you are right.

http://codereview.chromium.org/7068009/diff/1/src/v8natives.js#newcode138
src/v8natives.js:138: var global_receiver = %GlobalReceiver(global);
On 2011/05/26 08:26:55, Lasse Reichstein wrote:
Duplicate call to runtime function %GlobalReceiver(global).
Move this to before the previous line and assign receiver =
global_receiver.

Done.

http://codereview.chromium.org/7068009/diff/1/src/v8natives.js#newcode142
src/v8natives.js:142: if (!this_is_global_receiver ||
global_is_detached) {
On 2011/05/26 08:26:55, Lasse Reichstein wrote:
Could we make a comment that this test is jsut to be consistent with
JSC, and is
not required by the specification.

Done.

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.
On 2011/05/26 08:26:55, Lasse Reichstein wrote:
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!)

Yes, we could. Currently, we use a valid smi which means that the marker
is in the high 32 bits. We do that because we have to push the marker on
the stack. We only need the value to be smi tagged though. Let me change
this in a separate change to use a smi tagged non-smi.

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);
On 2011/05/26 08:26:55, Lasse Reichstein wrote:
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) {}

Done.

http://codereview.chromium.org/7068009/

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

Reply via email to