search

ygrek / ocaml-cbor

OCaml CBOR generic decoder/encoder, RFC 7049, http://cbor.io/
MIT License
24 stars 10 forks source link

Support for 64 bits integers #2

Open adrien-n opened 5 years ago

adrien-n commented 5 years ago

Hi,

If I'm not mistaken, the CBOR specification supports 64 bits unsigned integers but they're not supported in ocaml-cbor. Is it possible to add them to the library?

Note that I'm currently using ocaml's signed int64 in my application and it'd maybe make sense that I also switch to ocaml 4.08 and its unsigned 64 bit integers.

ygrek commented 5 years ago

Yes, 64 bit are not supported currently. I am not sure how to proceed though : changing Int of int to Int of int64 looks sad (big overhead for 99% of cases). Two not entirely satisfactory solutions come to mind - functorize over int type or add Int64 of int64 ctor.

adrien-n commented 5 years ago

As a user of the library, I think I prefer a new ctor for Int64. It's the simpler to use and it wouldn't be difficult to spot the ctor and add a tiny bit of documentation.

I concur that forcing Int64 for every integer is too heavy for the general case. As for functor-based APIs, they are much heavier.

ygrek commented 5 years ago

actually I looked at the code ( :) ) and me from 5 years ago was forward-thinking enough to put all functionality in CBOR.SImple, so we can experiment with different representations in different module with separate type.

adrien-n commented 5 years ago

I've started trying to add the corresponding code in CBOR.Simple and extract_number causes an issue: it has code that could handle 64 bit integers but it returns an int. I'm not sure that it's possible to handle int64 without making the code more generic (returning either Int orInt64 and passing them around). A functor would probably be the same complexity and possibly faster at run-time so I'll probably try that but that's a bit of code (and Pervasives and Int64 don't have the same API wrt stuff such as land/logand unfortunately).

edit: I've spent a few minutes on this and it seems easier than I thought: I'm currently making two CBOR.Simple modules, one with Int of int and one with Int of int64, implemented through a functor (which most likely won't be part of the library API). I have very little time for coding right now: even though it's probably very simple, it'll still take me a few days to finish but I'm hopeful I can have it done within a week.

adrien-n commented 5 years ago

How should the testsuite be changed? I've crafted a testcase but integration is not obvious since there are two implementations and moreover each have its own expected failures.

The best approach I can think of is to run two separate testsuites and chose the integer implementation through a command-line parameter each time. The downside is that the testsuite output will probably be less obvious since there will be two separate runs. Is that OK with you?

ygrek commented 5 years ago

Sorry for slow reply

I'm currently making two CBOR.Simple modules, one with Int of int and one with Int of int64, implemented through a functor (which most likely won't be part of the library API)

Yup, that would be my first approach to try.

The best approach I can think of is to run two separate testsuites and chose the integer implementation through a command-line parameter each time. The downside is that the testsuite output will probably be less obvious since there will be two separate runs. Is that OK with you?

I don't really like that, if the implementation is a functor it should be possible to chose in code without command-line flags?

Can you create a draft PR please?

adrien-n commented 5 years ago

Sorry for slow reply

No problem, I'm also slightly slow on my side at the moment. :)

I'm currently making two CBOR.Simple modules, one with Int of int and one with Int of int64, implemented through a functor (which most likely won't be part of the library API)

Yup, that would be my first approach to try.

The best approach I can think of is to run two separate testsuites and chose the integer implementation through a command-line parameter each time. The downside is that the testsuite output will probably be less obvious since there will be two separate runs. Is that OK with you?

I don't really like that, if the implementation is a functor it should be possible to chose in code without command-line flags?

I've spent a bit of time on the testsuite. I've started using dune's diff'ing of testsuite output vs. expected output in order to be able to test the two implementations in a single run while still showing something easy to understand.

Can you create a draft PR please?

I'm readying one. I only have a few small obvious deficiencies to clean I think.