707383 - Rename ValueTo{ECMA,}{Ui,I}nt32 to either the spec names or clearly non-spec names

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

Look more like the algorithms.  And make the bad methods clearly *not* be the spec algorithms.  'nuff said.

Interestingly, it looks like the typed array code in the places that get changed uses the wrong method.  I'm going to leave that to a followup bug, because the arguments are supposed to be converted to unsigned long, not to int32, so it's more involved than just four more characters of change.

Comment 1

[Luke Wagner [:luke]]

Comment on attachment 578780 [details] [diff] [review]
Patch

Ah, much nicer.

>-ValueToInt32(JSContext *cx, const js::Value &v, int32_t *out)
>+NonstandardToInt32(JSContext *cx, const js::Value &v, int32_t *out)

There seem to be two use cases of this: one from JSAPI (deserving the name you gave it) and several uses in typed arrays.  Given that, can you name this some name that corresponds with the typed array spec?  Failing that, I would prefer ToNumberRoundedToInt32 or ToInt32ForTypedArray or something more specific.

Comment 2

[Brendan Eich [:brendan]]

Nonstandard as a curse word is silly, the names here pre-date the stnadards -- they are ancient history (and other "non-standard" stuff from that era is now standard, and this cycle repeats; if we had used "Nonstandard" out of preemptive guilt, then what would the name want to be? Not "Standard...").

ToNumberRoundedToInt32 is good, might want to include "Nearest".

/be

Comment 3

[Jeff Walden [:Waldo]]

Luke, see comment 0: the typed array instances are using the wrong method (they want the standard conversions like everyone else), but it would have been too digressive to fix them here.

Comment 4

[Luke Wagner [:luke]]

(In reply to Jeff Walden (remove +bmo to email) from comment #3)
Well then your name choice was right on :)

Comment 5

[Luke Wagner [:luke]]

(In reply to Brendan Eich [:brendan] from comment #2)
> Nonstandard as a curse word is silly,

Not "curse" word, but attention-drawing.  We do that in other contexts when a function has some "you might want to look at me twice" property to it.  Regardless, it sounds like, once the typed array conversions are fixed to use the right conversion, there will be exactly one use so we can just inline it.

Comment 6

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/38e995ede425

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/38e995ede425