Functorize `Ppx_hash_lib.Hashable`

[toots]

It would be great if Ppx_hash_lib.Hashable could be derived from a functor the way that Ppx_hash_lib.Stdlib.Hash can be otherwise it seems like there is no way to define a generic hash that can be exported via mli files like:

type t [@@deriving hash]

without duplicating the current code (which is just some module type signatures).

[aalekseyev]

I'm not seeing sufficient benefit to functorizing this. As you say this is just a few type signatures (~20 lines), and copying this is nothing compared to the rest of the trouble you're getting into by using a custom hash type. In fact having a functor could create a false impression that custom hash types are well supported, whereas in reality that's a thorny path.

What I mean by thorny path is that you'll have to implement wrappers for all the basic types you want to be hashable with the new algorithm.

Please let me know if I misunderstood.

[toots]

The problem with copying the lines is that this makes the code prone to breaking with future update of the library..

I was able to add a custom hash to our project, it wasn't that hard, really, and would be pretty solidly future proof if not for the copy/paste:

module custom_hash.ml

module Specs = struct
  ...
end

module Ppx_hash_lib = struct
  module Std = struct
    module Hash = Ppx_hash_lib.Std.Hash.F (Specs)
  end

  type 'a hash_fold = Std.Hash.state -> 'a -> Std.Hash.state

  module Hashable = struct
    module type S = sig
      type t

      val hash_fold_t : t hash_fold
      val hash : t -> Std.Hash.hash_value
    end

    module type S1 = sig
      type 'a t

      val hash_fold_t : 'a hash_fold -> 'a t hash_fold
    end

    module type S2 = sig
      type ('a, 'b) t

      val hash_fold_t : 'a hash_fold -> 'b hash_fold -> ('a, 'b) t hash_fold
    end

    module type S3 = sig
      type ('a, 'b, 'c) t

      val hash_fold_t :
        'a hash_fold -> 'b hash_fold -> 'c hash_fold -> ('a, 'b, 'c) t hash_fold
    end
  end
end

include Ppx_hash_lib.Std.Hash
include Ppx_hash_lib.Std.Hash.Builtin

Typically, all I have to do after that is:

open Custom_hash

type foo = ... [@@deriving hash]

I'm not sure what you mean about implementing all wrappers.

[toots]

I should add that the need for Hashable only arises when you export a type with [@@deriving hash] in a .mli file.

[aalekseyev]

What I meant is that you'll need to define hashes for all the types that you're using, and can't rely on existing body of types that are already [@@deriving hash].

But ok, I can see where you're coming from. I'll make the change to base (since these are now defined in base) and see if I get pushback for exposing something unused.

[toots]

Thanks, this is much appreciated!

[aalekseyev]

Ok, the change is accepted internally, and should appear in the next release. Here's how you'll be able to define Ppx_hash_lib when that reaches you:

module Ppx_hash_lib = struct
  module Std = struct
    module Hash = Ppx_hash_lib.Std.Hash.F (Specs)
  end

  include Ppx_hash_lib.F (struct
      type hash_state = Std.Hash.state
      type hash_value = Std.Hash.hash_value
    end)
end

[toots]

That's awesome thanks so much! Should I close now or let's y'all do it?

[aalekseyev]

It'll be closed when the fix lands in this repo.