281537 - ScriptRuntime.toNumber warns on Undefined

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Attila Szegedi]

User-Agent:       Opera/7.54 (Windows NT 5.0; U)  [en]
Build Identifier: 

ScriptRuntime.toNumber() will print a warning 

"RHINO USAGE WARNING: Missed Context.javaToJS() conversion: Rhino runtime 
detected object org.mozilla.javascript.Undefined@165d905 of class org.mozilla.
javascript.Undefined where it expected String, Number, Boolean or Scriptable 
instance. Please check your code for missig Context.javaToJS() call"

Now, according to ECMA 9.3 Undefined is a perfectly convertible to a number, as 
NaN. I don't think the warning should be logged, esp. since other toXxx methods 
don't log a warning on Undefined.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Attila Szegedi]

Attachment: The patch to solve this bug

Comment 2

[Attila Szegedi]

Of course, it's possible that #280047 will also address this.

Comment 3

[Igor Bukanov]

The message is just a bug caused by implementing bug 280047. 

Thanks for spotting this!

Comment 4

[Igor Bukanov]

Attachment: 2-loop pattern in toNumber/toBoolean

The above patch was OK, but I would like toNumber and toBoolean to follow the
same double-loop pattern that is used in few other places in ScriptRuntime to
have smaller and easier to follow code.

Comment 5

[Attila Szegedi]

(In reply to comment #4)
> Created an attachment (id=173775) [edit]
> 2-loop pattern in toNumber/toBoolean
> 
> The above patch was OK, but I would like toNumber and toBoolean to follow the
> same double-loop pattern that is used in few other places in ScriptRuntime to
> have smaller and easier to follow code.

That loop isn't double, it's a plain single loop :-)

Anyway, I get what you're doing in there - it's a clever way to guarantee that 
if the method ever returns, the return value is a number, regardless of what 
does the Scriptable.defaultValue(NumberClass) return. Of course, an offending 
Scriptable returning "this" could force the interpreter into an infinite loop ..
.

Comment 6

[Igor Bukanov]

(In reply to comment #5)
> That loop isn't double, it's a plain single loop :-)

Of cause, it was not "double-loop" but rather no more then 2 times executed loop ;)

> 
> Anyway, I get what you're doing in there 

It is not me but rather the initial creators of Rhino who put such loop as
optimization in some of Rhino methods. 

The loop is not necessary if the code would check for Scriptable first. But then
the overhead of "instanceof Scriptable" check would occur in the majority of
cases. The loop allows to run the most common case of Number for toNumber and
Boolean for toBoolean while avoiding duplicating code for the checks.

Comment 7

[Igor Bukanov]

I committed the fix.