yallop / ocaml-ctypes

Library for binding to C libraries using pure OCaml
MIT License
365 stars 95 forks source link

Cstubs.FOREIGN.fn should always be a function type #279

Open whitequark opened 9 years ago

whitequark commented 9 years ago

When fn is abstract, it becomes very inconvenient to write code like this (example from tgls):

  let vertex_attrib_ipointer =
    foreign (* ~stub *) "glVertexAttribIPointer"
      (int_as_uint @-> int @-> int_as_uint @-> int @-> (ptr void) @->
       returning void)

  let vertex_attrib_ipointer index size type_ stride pointer =
    let pointer = match pointer with
    | `Offset o -> ptr_of_raw_address (Int64.of_int o)
    | `Data b -> to_voidp (bigarray_start array1 b)
    in
    vertex_attrib_ipointer index size type_ stride pointer

I.e. wrap the APIs inside the same module, as opposed to splitting the code between it and some external one.

Currently I add a constraint with type 'a fn = 'a and use the following workaround for the bindgen binary:

module Wrap(S: functor(F: Cstubs.FOREIGN with type 'a fn = 'a) -> sig end)
           (F: Cstubs.FOREIGN with type 'a fn = unit) =
struct
  include S(struct
    type 'a fn = 'a
    let foreign name typ = F.foreign name typ; fun _ -> assert false
  end)
end

let () =
  gen_bindings `Gl3 (module Wrap(Tgl3_bindings.Gl))

This change is safe, because it doesn't change anything at runtime, and at compile-time it would detect the function being called and just assert. Similarly it should not be a problem to plug inside Foreign.

whitequark commented 8 years ago

@yallop Any opinion on this?

yallop commented 8 years ago

I see the problem, and agree that the solution you propose addresses it. Still, I'm reluctant to provide an interface that promises more than it can deliver. Giving fn a non-abstract type encourages behaviour that can only lead to disappointment.

One alternative is to provide an interface that makes it possible to call the bound functions, but prevents access to the results. For example, we could take a monadic approach and provide return and >>= for `fn, like this:

  type 'a fn
  val foreign : string -> ('a -> 'b) Ctypes.fn -> ('a -> 'b) fn
  val return : 'a -> 'a fn
  val (>>=) : 'a fn -> ('a -> 'b fn) -> 'b fn

which would allow you to define vertex_attrib_ipointer by changing the final call to something like this:

    liftM5 vertex_attrib_ipointer index size type_ stride pointer

or perhaps to this:

    ...
    pure vertex_attrib_ipointer <*> index <*> size <*> type_ <*> stride <*> pointer

The absence of accessors/destructors for fn would avoid creating false hopes around the ability to call functions before they were bound, and the final instantiation would set 'a fn to 'a, return to %identity and >>= to |>, so there'd be no overhead.

However, this might be a little heavy in practice.

whitequark commented 8 years ago

I agree that the interface you propose addresses the design constraints you list pretty much perfectly, but I think it's far too heavy to be usable. Personally I can't stand lifters even when there's a good reason to introduce them (Cmdliner is useful, but very irritating still), and introducing a monadic interface here I can personally only characterize as "developer-hostile". Although I'm sure others can disagree.

I think what I propose is ultimately OK because anyone who makes a mistake of calling such a function will be notified immediately; it is never a hidden error. In other words, since a cstubs generator is a part of the compile process, raising an exception in such a way when a function is used inappropriately introduces a compile error, which is not any less descriptive than e.g. a type error. Does this argument make sense to you?

yallop commented 8 years ago

Yes, I see what you mean. The error will usually be detected during an early phase, when the bindings functor is run (although not when it's compiled).

I'm still not convinced by the proposal, though. I have a strong preference for types that guide the user by making it clear which operations are available and how they can be combined. From that perspective, fixing the definition of 'a fn as 'a makes the interface harder to understand, even if it makes some programs shorter.

whitequark commented 8 years ago

Yes. But also, types are not their own end. Types should help the programmer by exposing bugs, not hinder them by requiring to work around interfaces hostile to common cases. And in this case it's the latter.

yallop commented 8 years ago

I think #389 might eventually improve the situation here.

whitequark commented 8 years ago

Hmm, that looks vaguely promising, but I don't really understand how one's supposed to use it. Can you show some example?

whitequark commented 8 years ago

I mean--result is still abstract, this is the same basic problem, just shuffled around.

yallop commented 8 years ago

Yes. But I'm hoping this is a step on the way to eliminating result, leaving fn abstract. At that point a binding like this

let f = foreign "foo"
   (string @-> int @-> returning int)

would be given a type like this:

val foo : string -> int -> int fn

rather than the current type:

val foo : (string -> int -> int) fn

so it could be applied (safely!) within the functor.

whitequark commented 8 years ago

That doesn't help at all. E.g. if we have a get_width : string → int fn and it's used in an expression let is_wide = get_width "window" > 1000 then it won't typecheck. And this is indeed the case in my motivating example.

Nothing short of making fn non-abstract will work.

yallop commented 8 years ago

That doesn't help at all.

Well, it allows the code examples given in this issue to pass type checking, and it addresses the problem reported in the title. So it does appear to resemble an improvement.

whitequark commented 8 years ago

Oh, I see your overall approach now, now that I look at val (>>=) : 'a fn -> ('a -> 'b fn) -> 'b fn.

I concur that this is an improvement if you provide such a monadic bind in #389 and it will address all my concerns, albeit in a slightly awkward way. But I can live with that.