Please take another look.
http://codereview.chromium.org/8341008/diff/1/src/ic.cc File src/ic.cc (right): http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1423 src/ic.cc:1423: HandleScope scope(isolate()); On 2011/10/18 11:39:40, Kevin Millikin wrote:
This handle scope is not really necessary because of the outer one.
Let's
remove it.
Done. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1424 src/ic.cc:1424: Handle<Object> result = SetElement(receiver, index, value, strict_mode); On 2011/10/18 11:39:40, Kevin Millikin wrote:
As part of this cleanup, we should probably move this SetElement to a
static
member function of class JSObject.
Can we do this in a separate CL after handlification? To minimize possible conflicts. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1425 src/ic.cc:1425: if (result.is_null()) return Failure::Exception(); On 2011/10/18 11:39:40, Kevin Millikin wrote:
RETURN_IF_EMPTY_HANDLE might be clearer.
Done. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1432 src/ic.cc:1432: && JSArray::cast(*receiver)->AllowsSetElementsLength()) { On 2011/10/18 11:39:40, Kevin Millikin wrote:
I slightly prefer cast then dereference:
Handle<JSArray>::cast(receiver)->Allows....
because it reminds the reader we're in handle code.
Done. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1463 src/ic.cc:1463: if (receiver->IsJSGlobalProxy()) { On 2011/10/18 11:39:40, Kevin Millikin wrote:
I'm not sure why we patch this site even with --no-use-ic. Maybe we
shouldn't? Added TODO. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1519 src/ic.cc:1519: case NORMAL: { On 2011/10/18 11:39:40, Kevin Millikin wrote:
It's minor, but I don't like to have these braces where they're not
necessary,
so I remove them when I find them :)
Done. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1537 src/ic.cc:1537: Handle<AccessorInfo> callback(AccessorInfo::cast(*callback_object)); On 2011/10/18 11:39:40, Kevin Millikin wrote:
Better to have
Handle<AccessorInfo> callback =
Handle<AccessorInfo>::cast(callback_object);
here.
Done. http://codereview.chromium.org/8341008/diff/1/src/stub-cache.cc File src/stub-cache.cc (right): http://codereview.chromium.org/8341008/diff/1/src/stub-cache.cc#newcode512 src/stub-cache.cc:512: if (probe->IsCode()) return Handle<Code>::cast(probe); On 2011/10/18 11:39:40, Kevin Millikin wrote:
Also space after this line (and below).
Done. http://codereview.chromium.org/8341008/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
