Handlify upper layers of KeyedStoreIC.

Handlify upper layers of KeyedStoreIC.


BUG=
TEST=

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

[ulan, 2011-10-20 09:57:18]

PTAL.

[Kevin Millikin (Chromium), 2011-10-20 13:13:42]

LGTM with a few comments below.

http://codereview.chromium.org/8356039/diff/1/src/ic.cc
File src/ic.cc (left):

http://codereview.chromium.org/8356039/diff/1/src/ic.cc#oldcode1845
src/ic.cc:1845: HandleScope scope(isolate());
Strange that there was a handle scope here before.  That seems kind of random. 
Oh well....

http://codereview.chromium.org/8356039/diff/1/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/8356039/diff/1/src/ic.cc#newcode1739
src/ic.cc:1739: HandleScope scope(isolate());
No need for this handle scope.

http://codereview.chromium.org/8356039/diff/1/src/ic.cc#newcode1741
src/ic.cc:1741: if (result.is_null()) return Failure::Exception();
RETURN_IF_EMPTY_HANDLE(isolate(), result);

http://codereview.chromium.org/8356039/diff/1/src/ic.cc#newcode1742
src/ic.cc:1742: return *value;
It's a little strange not to return *result, but I guess we don't want to change
the existing behavior if it makes a difference.

I also guess the only reason we can't just have this whole thing be

return receiver->SetElement(index, *value, strict_mode)

is that there is some magic about ExternalArrayElements in the handles.cc
version of SetElement.  I wonder why we don't need that generally in SetElement?

http://codereview.chromium.org/8356039/diff/1/src/ic.cc#newcode1768
src/ic.cc:1768: Handle<JSObject> receiver = Handle<JSObject>::cast(object);
The existing code is confusing.  We've already checked that object->IsJSObject()
and we had an early return in that case (and we haven't assigned to object).  So
the check here is unnecessary.

We already have a handle named receiver, that is Handle<JSObject>::cast(object),
so it's unnecessary.

http://codereview.chromium.org/8356039/diff/1/src/ic.cc#newcode1854
src/ic.cc:1854: ASSERT(!code.is_null());
I think operator* has the same assert for handles, so it would be caught if you
tried to use it (but it's OK to have it here, too).

http://codereview.chromium.org/8356039/diff/1/src/ic.cc#newcode1868
src/ic.cc:1868: TraceIC("KeyedStoreIC", name, state, target());
Make you change these sites so that TraceIC ==> TRACE_IC.