Closed vouillon closed 10 months ago
I could use more context:
Z.hash x
should be equal to Hashtbl.hash x
? I'm not sure this is the case even with the proposed patch, as an extra mixing with 0 seems to be missing.Clarifications welcome!
My initial motivation was to harmonize hash results over 32-bits architectures (between zarith_stubs_js and zarith C stubs).
Beside that:
zarith_stubs_js
instead.Z.hash
returns a positive integer, since this is less error-prone, and that it returns the same result on all architectures. For this, anding with 0x3FFFFFFF
should be sufficient.Hashtbl.hash
and Z.hash
coincide. But I got this wrong...My initial motivation was to harmonize hash results over 32-bits architectures
Agreed. Let's do that.
I don't really care about zero hashing to 0. I can special-case zero in zarith_stubs_js instead.
I still don't understand why a special case is needed for zero...
It might be nice that Hashtbl.hash and Z.hash coincide. But I got this wrong...
If we really want this identity, we can just define Z.hash as Hashtbl.hash + a type constraint, like we do in the OCaml stdlib for String.hash, Int32.hash, etc. While we're at it, we could also define Z.seeded_hash. I'm tempted!
After thinking some more about it, I believe Z.hash
should be an alias for the standard hash function Hashtbl.hash
, like we do for other standard library modules that work with integers (Int, Int32, Int64, Nativeint). See #150 for more details.