LGTM if it passes ARM/x64 tests

http://codereview.chromium.org/2280007/diff/134001/119012
File src/ic.cc (right):

http://codereview.chromium.org/2280007/diff/134001/119012#newcode605
src/ic.cc:605: } else {
apparently you forgot to remove else-clause here.  not mandatory

http://codereview.chromium.org/2280007/diff/134001/119030
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/2280007/diff/134001/119030#newcode7207
test/cctest/test-api.cc:7207: "  if (i == 5) { o.prop = 1; };"
I'd rather change method itself (to something like function(x) { return
x - 1; }, otherwise this test doesn't prove we're functioning fine.

http://codereview.chromium.org/2280007/diff/134001/119030#newcode7229
test/cctest/test-api.cc:7229: "  if (i == 5) { proto.prop = 1; };"
ditto

http://codereview.chromium.org/2280007/diff/134001/119031
File test/mjsunit/keyed-call-ic.js (right):

http://codereview.chromium.org/2280007/diff/134001/119031#newcode158
test/mjsunit/keyed-call-ic.js:158: function testTypeTransitions() {
you might want to consider adding test cases which turn receiver into
not JSObjects like numbers or strings.

http://codereview.chromium.org/2280007/show

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

Reply via email to