Naddiseo/dart-sprintf

Error: The integer literal 0xffffffffffffffff can't be represented exactly in JavaScript.

Closed this issue · 10 comments

Hi there!

With the newest dart version (2.0.0-dev.65.0-1) (I updated from 2.0.0-dev.64.1) this problem arise:

Dart2Js finished with:

packages/sprintf/src/formatters/int_formatter.dart:5:30:
Error: The integer literal 0xffffffffffffffff can't be represented exactly in JavaScript.

In order to make it representable in JavaScript, can you change the value to 0x1FFFFFFFFFFFFF (2^53 - 1), because it is the highest safe integer in javascript See here. I think that should fix it.

I haven't been following dart development for a while, but looks like it's this change?

No I think it is this one.

Since JavaScript seems to store all numeral values as double precision floating numbers (even integers), the highest exactly displayable integer will be 2^53 - 1 if I am not mistaken.

I'm not entirely sure what the best way to solve this is. According to the int64 document, dart2 switched from arbitrary precision integers, to fixed length. So, I should be able to change the type of that variable to a BigInt, however, I'm not sure of the performance implications of doing that.

Looking at the code again, it seems I'm only using the MAX_INT to force the input variable to be at most 64bits, and I'm sure there's a better way of accomplishing the same thing. I'll look into it over the next week as my time frees up.

@BullshitPingu, I think I have two options for this: use BigInt, which will keep the behaviour the same at the expense of performance; or change the formatter to only work on JS 52bit ints, which is a backward incompatible change. Is there an obvious choice? I ask because I haven't actually used dart since around the time I wrote this library, so I don't know which of the choices would be more preferred in the dart ecosystem.

@Naddiseo Since you are already using ints as argument for the IntFormatter, you should stick with the JS 53 bit as max int. The application would have a problem with more than 53 bit integers as argument anyway.

I did not find any reports on the performance of BigInt.. But since int is as well an object in dart I don't think that the performance would be drastically worse.

Okay, sounds good to me: I'll change it to use JS ints, and if someone wants to pass in larger numbers, the library can be extended with a BigIntFormatter. Now I have to figure out how to generate the tests...

Same problem here, this is fatal to blocks dart2js builds unfortunately.

Sorry for leaving this so long; I've been sick the past couple weeks and this project fell off my plate.
I'm currently debating how to handle number at the min/max range when using the "%x" and "%o" formatting.
In C/++ (via bash), if I format -1 as hex, it wraps at the 64bit limit:

$ printf "|%x|\n" -1
|ffffffffffffffff|

similarly, with - max int, it wraps to 1

$ printf "|%x|\n" -0xffffffffffffffff
|1|

which seems to indicate twos-complement representation for negative numbers?

(aside)
Confusingly, javascript (-1).toString(16) prints -1, and (Number.MIN_SAFE_INTEGER).toString(16) gives '-1fffffffffffff'. Although, there's also this: (Number.MIN_SAFE_INTEGER|0).toString(16) => '1'

With that as a reference, I think that sprintf("%x", [Number.MIN_SAFE_INTEGER]) would also give 1, and sprintf("%x", [-1]) would return 1fffffffffffff. I'm currently trying to figure out how to generate the tests if I go that route since other implementations (C/python) use wider int representations and it's been a long time since I've had to think through low level details of int representations.

An alternate, route is to ignore (not force) the wrapping/overflow and just say that any number outside of Number.MIN_SAFE_INTEGER and Number.MAX_SAFE_INTEGER is undefined for this library, and we switch over to follow the javascript way of formatting. This would be a very breaking change, but would be a lot easier to implement (I'd just remove the 64bit mask and emulated overflow).

What do you think?

I've created a PR if you want to try it out before I merge it.

I would say it makes sense to align with the new Dart 2 int limitations and favor performance (I think compatibility with stdio is not a goal for this library?). I see you also increased the major version number so all should be fine. Perhaps in the future you can support a BigInt as input for %i and companions, so anyone wanting to sprintf with big integers can do so?