[webkit-reviews] review denied: [Bug 56168] Avoid slow-path for put() in Array.slice() : [Attachment 85455] Proposed patch v2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Andreas Kling <kling at webkit.org>'s
request for review:
Bug 56168: Avoid slow-path for put() in Array.slice()
https://bugs.webkit.org/show_bug.cgi?id=56168

Attachment 85455: Proposed patch v2
https://bugs.webkit.org/attachment.cgi?id=85455&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=85455&action=review

I’m pretty sure this is wrong. This doesn’t handle the case where there are
holes in the object we are slicing. Both arrays and non-array objects can have
empty slots with undefined values. The old code handled this. Are there any
test cases that cover it?

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:484
> +    JSArray* resObj = new (exec)
JSArray(exec->lexicalGlobalObject()->arrayStructure(), sliceCount,
CreateCompact);

Lets go nuts with this patch and call this result. There’s no need for a
separate local variable named result. JSValue::encode should work fine on a
JSArray*.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:489
> +    for (unsigned k = 0; k < sliceCount; k++)
> +	   resObj->uncheckedSetIndex(globalData, k, getProperty(exec, thisObj,
k + begin));

Shouldn’t we also stop on the first exception we see?