LGTM

http://codereview.chromium.org/6323002/diff/74001/src/arm/ic-arm.cc
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/6323002/diff/74001/src/arm/ic-arm.cc#newcode548
src/arm/ic-arm.cc:548: __ JumpIfNotSmi(key, key_not_smi);
Either make the entire 'if (...) __ JumpIfNotSmi(...);' a one-liner or
use braces around the body.

http://codereview.chromium.org/6323002/diff/74001/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/6323002/diff/74001/src/ia32/ic-ia32.cc#newcode513
src/ia32/ic-ia32.cc:513: // Key must be a smi..
.. -> .

http://codereview.chromium.org/6323002/diff/74001/src/ia32/ic-ia32.cc#newcode526
src/ia32/ic-ia32.cc:526: Label oor;
Unused label?

http://codereview.chromium.org/6323002/diff/74001/src/x64/ic-x64.cc
File src/x64/ic-x64.cc (right):

http://codereview.chromium.org/6323002/diff/74001/src/x64/ic-x64.cc#newcode538
src/x64/ic-x64.cc:538: if (key_not_smi != NULL)
Braces or one-liner.

http://codereview.chromium.org/6323002/diff/74001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/6323002/diff/74001/test/cctest/test-api.cc#newcode10178
test/cctest/test-api.cc:10178: // CHECK_EQ(1, result->Int32Value());
On 2011/01/28 10:35:36, danno wrote:
i'll fix this, shouldn't be commented out.

Thanks!

http://codereview.chromium.org/6323002/diff/74001/test/cctest/test-api.cc#newcode10356
test/cctest/test-api.cc:10356: "for (var i = 0; i < 10; ++i) { result =
pa_load(just_ints); }"
On 2011/01/28 10:35:36, danno wrote:
I'll fix the 80 cols, too, here and below.

Thanks.

http://codereview.chromium.org/6323002/

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

Reply via email to