562448 - ES5 Function.prototype.apply does not require argArray be either Array or arguments

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brendan Eich [:brendan]]

It treats the second parameter, if not null and not undefined, as an object from which to get "length" and index-named properties, i.e. a generic array-like.

/be

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch and tests

I have a query going on es5-discuss about whether it's really intended/desirable to apply ToNumber twice (once via ToUint32) to argsArray.length, but with spec text as it is now, this patch is correct.

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Patch and tests, now with less weird error-checking and double-evaluating

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 459839 [details] [diff] [review]
Patch and tests, now with less weird error-checking and double-evaluating

Nice, much improved and close to the spec.

>+        if (lenval.isInt32()) {
>+            length = jsuint(jsint(lenval.toInt32())); /* jsuint cast does ToUint32 */

Don't need the jsint case here.

r=me with that picked -- thanks.

/be

Comment 4

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/898b20672383

Picked that nit, added comments delineating algorithm steps for greater clarity and readability, and renamed a variable for the same reason -- pushed with those changes.

Comment 5

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/mozilla-central/rev/898b20672383

Comment 6

[Eric Shepherd [:sheppy]]

Added a note to:

https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Function/apply

And to the New in JavaScript 1.8.5 page.