120194 - JS toInt32(x) conversion doesn't match ECMAScript definition

Status: VERIFIED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Phil Schwartau]

This is the Rhino version of bug 120083,
"JavaScript toInt32 conversion doesn't match ECMAScript definition"

The problem centers on numbers between 2^31 and 2^32 with fractional parts.
The JS internal function toInt32(x), available to the JS user as x | 0
or  x << 0, is off by one on these. Same for negative numbers between
-2^31 and -2^32 with fractional parts.

Rhino has the same problem as SpiderMonkey had. Tests to cover this
issue were added to 

        mozilla/js/tests/ecma/TypeConversion/9.5-2.js

This test is now failing in the Rhino shell -

Comment 1

[Phil Schwartau]

Here is the failure output from the testcase:

*-* Testcase ecma/TypeConversion/9.5-2.js failed:

POSITIVE NUMBERS
2147483648.25 << 0 = -2147483647 FAILED! expected: -2147483648
2147483648.5 << 0 = -2147483647 FAILED! expected: -2147483648
2147483648.75 << 0 = -2147483647 FAILED! expected: -2147483648
4294967295.25 << 0 = 0 FAILED! expected: -1
4294967295.5 << 0 = 0 FAILED! expected: -1
4294967295.75 << 0 = 0 FAILED! expected: -1
3000000000.25 << 0 = -1294967295 FAILED! expected: -1294967296
3000000000.5 << 0 = -1294967295 FAILED! expected: -1294967296
3000000000.75 << 0 = -1294967295 FAILED! expected: -1294967296

NEGATIVE NUMBERS
-2147483648.25 << 0 = 2147483647 FAILED! expected: -2147483648
-2147483648.5 << 0 = 2147483647 FAILED! expected: -2147483648
-2147483648.75 << 0 = 2147483647 FAILED! expected: -2147483648
-4294967295.25 << 0 = 0 FAILED! expected: 1
-4294967295.5 << 0 = 0 FAILED! expected: 1
-4294967295.75 << 0 = 0 FAILED! expected: 1
-3000000000.25 << 0 = 1294967295 FAILED! expected: 1294967296
-3000000000.5 << 0 = 1294967295 FAILED! expected: 1294967296
-3000000000.75 << 0 = 1294967295 FAILED! expected: 1294967296

Comment 2

[Steven Cole]

Attachment: Fix ala the Spidermonkey change

Same algorithmic change as in bug 120083.  Note: I am not a Java programmer,
and have *not* tested this patch.  Nevertheless, we tested the C-version in the
spidermonkey bug, and I'm pretty sure this will fly.

Comment 3

[Steven Cole]

Attachment: <sigh> Try #2.

Well, that figures: Rhino has two places where this code lives.  (I hate code
duplication!)

Phil, the two different code paths come as conversions from doubles to int32s
and conversions from jsvalues to int32s.  I'm *guessing* that we ought to test
for both the numeric form of these numbers, and the string form of these
numbers in order to hit both sections of the code.  Norris should be able to
shed more light on just the right way to test both pieces, though.

--scole

Comment 4

[Steven Cole]

Attachment: Actually changed the file I uploaded.

Well, it's been quite a morning.  I didn't actually save the last patch before
submitting it to bugzilla.  Here's the right one.

Comment 5

[Igor Bukanov]

Attachment: tested fix and code duplication removal

The previous fix from Steven actually does not work as it turned out it was
necessary to call floor/ceil before calling IEEEremainer. 

The patch also removes code duplication by simply calling toInt32(double) from
toInt32(Object) and adds optimization for the case when number is already exact
int. 

With the patch rhino passes the tests.

Comment 6

[Norris Boyd]

Thanks, Igor, for the patch. I committed it.

Comment 7

[Phil Schwartau]

Marking Verified Fixed.

The above testcase now passes in both the compiled and
interpreted modes of the Rhino shell -

Comment 8

[Igor Bukanov]

Targeting as resolved against 1.5R4