ppx_spice icon indicating copy to clipboard operation
ppx_spice copied to clipboard

int64 encoding is broken in the OCaml lib for negative numbers

Open reebalazs opened this issue 1 year ago • 2 comments

Spice uses Caml_int64.bits_of_float / Caml_int64.float_of_bits but I've noticed that for negative numbers float_of_bits gives NaN. Therefore I have the suspicion that this library is broken. Consequently, spice is broken with regards to converting negative int64 values.

I've asked this on the rescript forums: https://forum.rescript-lang.org/t/int64-float-of-bits-gives-nan-for-a-negative-integer/4793/1

I don't know what's the best solution, for now I'm avoiding to convert a negative int64, but for the sake of completeness, this should also work in spice.

reebalazs avatar Nov 14 '23 17:11 reebalazs

Hi, Sorry for late response. Frankly, I didn't acknowledge this issue before. Thank you for openning issue. As I shortly dig into this issue, I reach to the consideration of what about the compiler has the bigint primitive such as int32, int64. Let me think about this further, and I'll come back later.

mununki avatar Nov 19 '23 05:11 mununki

Hi, thanks! Let me know if I can help in any way! It would be great to fix this in a good way.

My current standpoint is:

  • The bits_of encoding would be better (storing as a number is more economic that storing in string) but it has to work for negatives too. It's unclear for me if the ocaml library is broken, how come noone has noticed this? Maybe this would also need a fix in the rescript compiler repo, then, but until then, spice could fix it on its own account.

  • Regardless of the previous point, it might be beneficial to support a to_string codec for int64s, as an alternate codec that is usable from spice if needed. In some cases you'd prefer a string codec...

reebalazs avatar Nov 21 '23 12:11 reebalazs