Annotate integer types with [@@immediate64]

[craigfe]

Since OCaml 4.10.0, the compiler has supported this annotation on types that are immediate on 64-bit platforms only.

This PR adds the annotation in the simplest way I could think of (that still supports the fuzz tests): by using a dune rule to conditionally add it. This is quite hacky; let me know if you'd like a different solution.

[dinosaure]

Hmmhmm, yes the solution is not so good by can we finally use (select ...) for the interface and the implementation finally? optint is a bit old and such design (with an executable which does the choice) seems pretty-old and does properly scale on your request. I will try to propose something else with (select ... and see if it's better or not.

Otherwise, ok to merge 👍.

[craigfe]

@dinosaure: I've pushed a new suggestion that I'm much happier with, but it's a breaking change now.

I've stripped out all the Dune selection logic, got rid of the unwrapped modules, and used functors for selection instead. So we now have Optint.Int63 instead of just Int63 (and Optint remains unchanged). Let me know what you think.

[dinosaure]

😕 I still prefer (select ...) than that... sorry.

[craigfe]

I'm not sure what solution you're imagining w/ (select ...) without first needing a more powerful select stanza.

[craigfe]

Even with the ability to select between multiple interfaces the library would end up needing to expose that conditional choice in an observable way, and this can break downstream code. (This was also true of the Dune rules I had before.)

So I think some sort of Obj.magic is going to be necessary to have this annotation.

[dinosaure]

to expose that conditional choice in an observable way

I'm not sure to understand why we need that. In my mind, the end user should not introspect how optint chosen its implementation - and if the user really need to introspect that, he can do that with Sys.word_size. I suspect that you need such observable value on tests/fuzzers but, again, it's a really limited scope.

More generally, Obj.magic is not the right solution 😕 . It can blows up our stack at a deep level.

[craigfe]

It's precisely that we don't want that that's the problem. With an implementation in the build system, the [@immediate64] annotation is added conditionally according to the platform, and so downstream code that also uses [@immediate64] will break when Optint supplies the abstract type t rather than the annotated one. Generally, it seems to me to be a bad idea to use the build system to select between interfaces: it breaks the abstraction. I'm not a fan of Obj.magic either, but I don't see any other way to avoid this (and I think it's fairly well-scoped here to be safe).

On a not-completely-unrelated note, I think it is important for users to introspect at runtime on the representation, so that we can use int63 for file offsets in C bindings (as in this PR) w/o needing to allocate on 64-bit platforms. That can be done with type equality witnesses though, so is type-safe.

[dinosaure]

It's precisely that we don't want that that's the problem. With an implementation in the build system, the [@immediate64] annotation is added conditionally according to the platform, and so downstream code that also uses [@immediate64] will break when Optint supplies the abstract type t rather than the annotated one.

I understand your point but I was believed that C stubs (in particular) should not widely uses optint. I think, we possibly mix both solution between (select ...) (or whatever do the choice such as our own executable) and your GADT. What do you think about an essentialization of the choice between Immediate and Boxed via a pre-process (as we do now) and a derivation of optint.ml and int63.ml from this generated code-source?

[craigfe]

I think any selection at the build-system level is concerning because it's not abstracting away the selection. e.g. the following would compile on 32-bit platforms and be broken on 64-bit platforms:

module Offset : sig
  type t [@@immediate64]
end = struct
  type t = Int63.t
end

Am I right in thinking you'd rather have this breakage than use Obj.magic to avoid it?

[craigfe]

I understand your point but I was believed that C stubs (in particular) should not widely uses optint.

That would limit the usefulness of this library for systems code quite a bit, I think. e.g. in Index, if we can't use int63 for C stubs then we may as well suffer the allocation upfront and use int64 for file offsets everywhere. I think this problem is orthogonal to the question of the [@immediate64] annotation, since it's a run-time observation of the type.

[craigfe]

@dinosaure: I've updated the implementation to be syntactically identical to the one in Stdlib.Sys and added a comment about that; sorry for not making this clear to begin with :-)

Making the CI happy also required a little manipulating to remove large literals in the _native implementations. I can probably re-add the conditional compilation to avoid needing to do that, if you'd prefer.

[dinosaure]

Making the CI happy also required a little manipulating to remove large literals in the _native implementations. I can probably re-add the conditional compilation to avoid needing to do that, if you'd prefer.

The trick used on the compiler side is to use int_of_string "0xffffffff", then, if we trust on our codebase, it should be fine.

[craigfe]

I gave this a shot, but I'm getting an exception on 32-bit systems (which makes sense). Is the trick to use int_of_string inside the functions and rely on the compiler to inline it?

[dinosaure]

Ok the update was pretty recent on the compiler side: ocaml/ocaml#10244
So your first solution is fine 👍 .

[craigfe]

OK, have dropped that commit 👍

[dinosaure]

Thanks, will do a release next week when we have a breaking changes on this PR.