898664 - BinaryData uint64's are signed

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Brandon Benvie [:benvie]]

> uint64(-100) // -100

Comment 1

[Brandon Benvie [:benvie]]

Also:

> var Foo = new ArrayType(uint64, 1);
> new Foo([-1]) // [-1]

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: proposed fix

uint32_t and uint64_t are reified as int32, as there is no template specialization for these types. However, they can overflow the range of an int32 (which happens here) and thus should be saved in a double if necessary. As far as I can see in Value.setNumber(x), if x can actually fit in an int32, it will be done, so we can just use setNumber.

Note that because of the loss of precision of high double values, uint64(-1) == uint64(-2) == uint64(-3), etc.
There is no other real choice as a JS value can only embed an int32 or a float64 value.

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

749786 will introduce 64 bit types to JS userspace. I believe this bug should block on that.

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 783650 [details] [diff] [review]
proposed fix

Review of attachment 783650 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks good. But lets wait before landing this. I was talking to Niko about the latest spec
and it seems 64 bit numbers might be out of the spec for now.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Nikhil Marathe [:nsm] from comment #3)
> 749786 will introduce 64 bit types to JS userspace. I believe this bug
> should block on that.

Oh! I wasn't aware of that, that's good to know :)

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

As of today:

js> TypedObject.uint32(-1) 
4294967295

As the problem was concerning both uint32 and uint64 (which are not implemented right now) according to comment 2, I tentatively close the bug as wfm, but feel free to reopen it if it's not the case.