Minor change to for-in...

Minor change to for-in

Return (smi) 0 instead of object null from the FILTER_KEY builtin.

Add a test which tests keys being deleted during for-in.

Committed: http://code.google.com/p/v8/source/detail?r=5243

[Søren Thygesen Gjesse, 2010-08-11 13:20:01]

Am I right in assuming that a smi 0 on x64 is all zero bits?

[Erik Corry, 2010-08-11 13:27:54]

LGTM, but wait for LRN.

http://codereview.chromium.org/3170004/diff/1/2
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/3170004/diff/1/2#newcode2484
src/arm/codegen-arm.cc:2484: __ mov(r3, Operand(r0));
make this a SetCC operation and omit the tst.

http://codereview.chromium.org/3170004/diff/1/3
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/3170004/diff/1/3#newcode960
src/arm/full-codegen-arm.cc:960: // anymore. If the property has been removed
while iterating, we
anymore -> any more

http://codereview.chromium.org/3170004/diff/1/3#newcode966
src/arm/full-codegen-arm.cc:966: __ tst(r3, r3);
And again.

http://codereview.chromium.org/3170004/diff/1/9
File test/mjsunit/for-in-delete.js (right):

http://codereview.chromium.org/3170004/diff/1/9#newcode28
test/mjsunit/for-in-delete.js:28: // Test that properties deleted during a
for-in iteration does not show up in
properties ... does -> properties ... do

[Lasse Reichstein, 2010-08-11 13:37:36]

X64 LGTM if changed to using SmiCompare.

You are right that smi zero is currently represented by all-zero bits, but you
are wrong in *assuming* it :)

http://codereview.chromium.org/3170004/diff/1/7
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/3170004/diff/1/7#newcode3928
src/x64/codegen-x64.cc:3928: __ testq(rbx, rbx);
To test for Smi zero, use SmiCompare(rbx, Smi::FromInt(0)).
It will generate the same code, but is safe against change of representation.
In general never assume anything about the smi format outside of the
MacroAssembler Smi-macros.

http://codereview.chromium.org/3170004/diff/1/8
File src/x64/full-codegen-x64.cc (right):

http://codereview.chromium.org/3170004/diff/1/8#newcode1056
src/x64/full-codegen-x64.cc:1056: __ testq(rax, rax);
Use SmiCompare(rax, Smi::FromInt(0)).

[Søren Thygesen Gjesse, 2010-08-11 13:45:55]

http://codereview.chromium.org/3170004/diff/1/2
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/3170004/diff/1/2#newcode2484
src/arm/codegen-arm.cc:2484: __ mov(r3, Operand(r0));
On 2010/08/11 13:27:54, Erik Corry wrote:
> make this a SetCC operation and omit the tst.

Of cause - done.

http://codereview.chromium.org/3170004/diff/1/3
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/3170004/diff/1/3#newcode960
src/arm/full-codegen-arm.cc:960: // anymore. If the property has been removed
while iterating, we
On 2010/08/11 13:27:54, Erik Corry wrote:
> anymore -> any more

Done (and changed "null" to "(smi) 0").

http://codereview.chromium.org/3170004/diff/1/3#newcode966
src/arm/full-codegen-arm.cc:966: __ tst(r3, r3);
On 2010/08/11 13:27:54, Erik Corry wrote:
> And again.

Done.

http://codereview.chromium.org/3170004/diff/1/7
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/3170004/diff/1/7#newcode3928
src/x64/codegen-x64.cc:3928: __ testq(rbx, rbx);
On 2010/08/11 13:37:37, Lasse Reichstein wrote:
> To test for Smi zero, use SmiCompare(rbx, Smi::FromInt(0)).
> It will generate the same code, but is safe against change of representation.
> In general never assume anything about the smi format outside of the
> MacroAssembler Smi-macros.

Done.

http://codereview.chromium.org/3170004/diff/1/8
File src/x64/full-codegen-x64.cc (right):

http://codereview.chromium.org/3170004/diff/1/8#newcode1056
src/x64/full-codegen-x64.cc:1056: __ testq(rax, rax);
On 2010/08/11 13:37:37, Lasse Reichstein wrote:
> Use SmiCompare(rax, Smi::FromInt(0)).

Done.

http://codereview.chromium.org/3170004/diff/1/9
File test/mjsunit/for-in-delete.js (right):

http://codereview.chromium.org/3170004/diff/1/9#newcode28
test/mjsunit/for-in-delete.js:28: // Test that properties deleted during a
for-in iteration does not show up in
On 2010/08/11 13:27:54, Erik Corry wrote:
> properties ... does -> properties ... do

Done.