Subj : Re: JSVAL_IS_NUMBER(NaN) == TRUE (!?)
To   : Brendan Eich
From : Rob Swindell
Date : Mon Feb 21 2005 01:06 am

  Re: Re: JSVAL_IS_NUMBER(NaN) == TRUE (!?)
  By: Brendan Eich to Rob Swindell on Sun Feb 20 2005 09:46 am

 > Rob Swindell wrote:
 > >   To: Brendan Eich
 > >   Re: Re: JSVAL_IS_NUMBER(NaN) == TRUE (!?)
 > >   By: Brendan Eich to Rob Swindell on Fri Feb 18 2005 02:55 pm
 > >
 > >  > > I found it odd that if jsval is NaN, a test of JSVAL_IS_NUMBER(jsval)
 > >  > > evaluate as TRUE.
 > >  >
 > >  >
 > >  > Sure, it's of "number" type (see also typeof(0/0)).
 > >
 > > It just seemed illogical (to me) that:
 > >
 > >         is (not a number) a number?
 > >
 > > would evaluate to a true statement.
 > 
 > 
 > As Justin Fletcher pointed out, the type of a NaN must be number, as NaN
 > is an IEEE double value (actually a set of values, all not-a-number, all
 > valid IEEE doubles, none == to any other number including NaN).
 > 
 > NaN is funny that way, but it's still in the domain of double, which is
 > a (not explicitly typed, in the JS1.x language) subtype of number.
 > 
 > 
 > >  > > JSBool
 > >  > > jsval_isNaN(JSContext *cx, jsval v)
 > >  > > {
 > >  > >     jsdouble d;
 > >  > >
 > >  > >     if(JSVAL_IS_DOUBLE(v)) {
 > >  > >             if (!JS_ValueToNumber(cx, v, &d))
 > >  >
 > >  >
 > >  > No need to convert v to number if it's already a double (which is a
 > >  > subtype of number).
 > >
 > > But JSDOUBLE_IS_NaN() requires a double argument. You can't simply typecas
 > > jsval to a jsdouble,
 > 
 > 
 > You shouldn't be casting blindly in any case -- the macros do the right
 > casts, all else is shutting up the compiler as you aim at your foot ;-).
 > 
 > JSDOUBLE_IS_NaN(*JSVAL_TO_DOUBLE(v)) does what you want, and since you
 > know that JSVAL_IS_DOUBLE(v), the JS_ValueToNumber(cx, v, &d) call is
 > useless except as a longer way of doing d = *JSVAL_TO_DOUBLE(v).

Ah, cool thanks. And this rather simplifies the macro too. :-)

Any special reason that JSDOUBLE_IS_NaN() isn't in jsapi.h (with the other
JS*_IS_* macros)? 

 > >  > Yeah, it hasn't come up till now.  Most native methods should let NaNs
 > >  > flow through and produce NaN results.  Why not yours?
 > >
 > > I have some native methods which parse through the passed arguments and us
 > > values passed based on their type as well as their order. Simple stated, m
 > > native functions aren't going to like a NaN value when they expect an numb
 > 
 > 
 > As IEEE 754 has it, a NaN is in the domain of the double-precision type,
 > just so you can represent invalid numbers "in band".
 > 
 > What goes wrong if you let NaNs flow through, as Math.sin and all the
 > rest do, and have done, since 1995?

Well as you saw in this case, a crash. :-(

In the case of newer libjs.so's, it would cause the script to fail with an
exception.
 
 > > These functions attempt to convert a "number" argument to an integer using
 > > JS_ValueToInt32(). The test for a "number" was done using JSVAL_IS_NUMBER,
 > > which if the value is NaN, can't logically produce a valid int32 value. I 
 > > know if this is the reason, but js32.dll v1.5 pre-release 5a (the version 
 > > in the current release of my product) will crash if NaN is passed to
 > > JS_ValueToInt32. Here's an example callstack of one such crash:
 > >
 > > NTDLL! 77f813b1()
 > > Decompile(SprintStack * 0x035dc4b0, unsigned char * 0x03eaf4ce, int 0x0000
 > > line 1860 + 47 bytes
 > > js_DecompileCode(JSPrinter * 0x05622208, JSScript * 0x03eaf400, unsigned c
 > > 0x03eaf4ce, unsigned int 0x00000003) line 2347 + 17 bytes
 > > js_DecompileValueGenerator(JSContext * 0x02f42b70, int 0x00000001, long
 > > 0x030b6322, JSString * 0x00000000) line 2669 + 27 bytes
 > > js_ValueToInt32(JSContext * 0x02f42b70, long 0x030b6322, long * 0x035dc89c
 > > line 765 + 17 bytes
 > > JS_ValueToInt32(JSContext * 0x02f42b70, long 0x030b6322, long * 0x035dc89c
 > > line 552 + 17 bytes
 > > js_get_msg_header(JSContext * 0x02f42b70, JSObject * 0x055f13a0, unsigned 
 > > 0x00000002, long * 0x03ea401c, long * 0x035dcd48) line 643 + 32 bytes
 > > js_Invoke(JSContext * 0x02f42b70, unsigned int 0x00000002, unsigned int
 > > 0x00000000) line 843 + 26 bytes
 > > js_Interpret(JSContext * 0x02f42b70, long * 0x035de164) line 2852 + 15 byt
 > > js_Execute(JSContext * 0x02f42b70, JSObject * 0x030b6340, JSScript *
 > > 0x03ea3700, JSStackFrame * 0x00000000, unsigned int 0x00000000, long *
 > > 0x035de164) line 1055 + 13 bytes
 > > JS_ExecuteScript(JSContext * 0x02f42b70, JSObject * 0x030b6340, JSScript *
 > > 0x03ea3700, long * 0x035de164) line 3373 + 25 bytes
 > > exec_ssjs(http_session_t * 0x035de234, char * 0x035de351) line 2758 + 36 b
 > > respond(http_session_t * 0x035de234) line 2809 + 18 bytes
 > > http_session_thread(void * 0x00000000) line 2943 + 12 bytes
 > > _threadstart(void * 0x050286f0) line 187 + 13 bytes
 > > KERNEL32! 7c57b388()
 > >
 > > I don't know whether it *should* be legal for JS_ValueToInt32() to convert
 > > NaN jsval or not,
 > 
 > 
 > It should be possible, because (see jsnum.c) js_ValueToInt32 checks
 > JSDOUBLE_IS_NaN and reports an error.  In the older version of the
 > engine you are using, that seems to crash, but I don't have a copy of
 > that older version to test.
 > 
 > If you can't upgrade to a newer version of the engine (RC5a is quite
 > old), then mail me the source line and a few lines of surrounding
 > context, and the bad pointer or whatever, at the crash site in
 > Decompile, and I'll see if I can help (first by finding the bug with a
 > fix-patch that corrected this problem).

I can and will upgrade. Thanks for the offer of help, but seeing as the problem
is already fixed in later versions, I don't want to waste your time. :-)

 > > but in any case, it's not what I intended this method to do
 > > if passed such a value. It instead should return the proper error result, 
 > > does now, using the JSVAL_IS_NUM() macro to test for a valid number value 
 > > including NaN).
 > 
 > 
 > What's the proper error result, if not an exception (as you'd get from
 > the error reported by js_ValueToInt32 if the Decompile bug were fixed)?

For this particular example, the method returns null if the parameters are not
of the expected type, quantitiy, or order (a valid object otherwise). 

I prefer this method of error reporting (for "high level" errors like this)
than to generate an exception that the script author most likely will not
catch. This allows for easier (in my opinion) error case handling by the
script author and allows the script to continue executing (or not) at the
author's discretion.

 > > I'm assuming the root cause of the above crash has been fixed in later ver
 > > of SpiderMonkey as tests conducted with newer libjs.so's (for Linux and
 > > FreeBSD) did not crash. But the crash highlighted the fact my methods were
 > > attempting to process the passed value as a valid number, when in fact it 
 > > not.
 > 
 > 
 > No, they were attempting to report an error converting NaN to "Int32",
 > so they were trying to "return the proper error result" (at least in the
 > JS_ValueToInt32 sense).
 > 
 > Doing your own custom error handling may be better, but at first blush
 > it seems to me that you would want to use the same input validation that
 > JS_Convert* and JS_ValueToInt32 use.

Perhaps in some cases. "Error converting NaN to Int32" is certainly more
detailed (and potentially helpful) to the script author that made a typo. In
this particular case, the value is a browser-supplied query value, so a more
generic/pretty "syntax error" is the desired result if the query value contains
an unexpected value/type.

Once I get the latest and greatest js32.dll in place, I'll re-evaluate this
particular scenario and decide which is the preferred approach. Thanks for your
time,

                                                  digital man

Snapple "Real Fact" #104:
There are more then 30,000 diets on public record.

.