Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 707383 - Rename ValueTo{ECMA,}{Ui,I}nt32 to either the spec names or clearly non-spec names
: Rename ValueTo{ECMA,}{Ui,I}nt32 to either the spec names or clearly non-spec ...
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- minor (vote)
: mozilla11
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
: Jason Orendorff [:jorendorff]
Depends on: 708377
  Show dependency treegraph
Reported: 2011-12-02 17:21 PST by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-12-07 12:38 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (14.41 KB, patch)
2011-12-02 17:21 PST, Jeff Walden [:Waldo] (remove +bmo to email)
luke: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-02 17:21:33 PST
Created attachment 578780 [details] [diff] [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.
Comment 1 Luke Wagner [:luke] 2011-12-03 22:07:05 PST
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.
Comment 2 Brendan Eich [:brendan] 2011-12-03 22:41:48 PST
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".

Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-04 11:11:52 PST
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] 2011-12-04 12:18:04 PST
(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] 2011-12-04 12:21:33 PST
(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] (remove +bmo to email) 2011-12-07 08:58:40 PST
Comment 7 Matt Brubeck (:mbrubeck) 2011-12-07 12:27:36 PST

Note You need to log in before you can comment on or make changes to this bug.