LGTM if comments are addressed.
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()); This handle scope is not really necessary because of the outer one. Let's remove it. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1424 src/ic.cc:1424: Handle<Object> result = SetElement(receiver, index, value, strict_mode); As part of this cleanup, we should probably move this SetElement to a static member function of class JSObject. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1425 src/ic.cc:1425: if (result.is_null()) return Failure::Exception(); RETURN_IF_EMPTY_HANDLE might be clearer. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1432 src/ic.cc:1432: && JSArray::cast(*receiver)->AllowsSetElementsLength()) { I slightly prefer cast then dereference: Handle<JSArray>::cast(receiver)->Allows.... because it reminds the reader we're in handle code. http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1463 src/ic.cc:1463: if (receiver->IsJSGlobalProxy()) { I'm not sure why we patch this site even with --no-use-ic. Maybe we shouldn't? http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1519 src/ic.cc:1519: case NORMAL: { 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 :) http://codereview.chromium.org/8341008/diff/1/src/ic.cc#newcode1537 src/ic.cc:1537: Handle<AccessorInfo> callback(AccessorInfo::cast(*callback_object)); Better to have Handle<AccessorInfo> callback = Handle<AccessorInfo>::cast(callback_object); here. 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); Also space after this line (and below). http://codereview.chromium.org/8341008/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
