Closed Bug 707383 Opened 12 years ago Closed 12 years ago

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


(Core :: JavaScript Engine, defect)

Not set





(Reporter: Waldo, Assigned: Waldo)




(1 file)

Attached patch PatchSplinter Review
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.
Attachment #578780 - Flags: review?(luke)
Comment on attachment 578780 [details] [diff] [review]

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.
Attachment #578780 - Flags: review?(luke) → review+
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".

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.
(In reply to Jeff Walden (remove +bmo to email) from comment #3)
Well then your name choice was right on :)
(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.
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 708377
You need to log in before you can comment on or make changes to this bug.