[m-rev.] for review: rationalise hash functions across the standard library

Julien Fischer jfischer at opturion.com
Tue Feb 11 12:19:12 AEDT 2020


Hi Peter,

On Tue, 11 Feb 2020, Peter Wang wrote:

> On Tue, 11 Feb 2020 11:42:34 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
>>
>> For review by Peter.
>>
>> We discussed this on the developers list back in August 2019.
>>
>> --------------------------------------------------------------------
>>
>> Rationalise hash functions across the standard library.
>>
>> Currently, the hash functions for (some of) the primitive types are defined in
>> three places:
>>
>>     1. The hash_table module.
>>     2. The version_hash_table module (duplicates of the above).
>>     3. Some (but not all) of the library modules for the primitive types.
>>
>> This change makes the library module for a primitive type provide the hash
>> function for that type and deprecates the versions in the hash table modules.
>>
>> Additionally, deprecate the "generic" has functions in the hash table modules.
>>
>
> Related: should we move towards using hash functions that return uints?

Ultimately, the hash table implementation requires indexing into an
array and some of our target languages only support array indexing with
a signed integer type. (Indeed some of them require the index to be a
32-bit type as well, IIRC, so maybe we should move towards hash funtions
that return an int32?)

> And, if we're breaking things,

Nothing should be broken (yet).

> should the hash table modules take a hash *function* instead of a
> predicate?

I wondered about that too.  Given that Ralph was so keen on using
fuctions everywhere else it's a bit odd that he decided to use a
predicate there ...

We could shift to using a function and provide a wrapper so that
existing code that uses predicates still works.

>> diff --git a/library/char.m b/library/char.m
>> index 04ecac3..59e2459 100644
>> --- a/library/char.m
>> +++ b/library/char.m
>> @@ -397,6 +397,16 @@
>>   :- pred det_int_to_digit(int::in, char::out) is det.
>>
>>   %---------------------------------------------------------------------------%
>> +%
>> +% Computing hashes of charrs.
>> +%
>
> chars

Fixed.

>> diff --git a/library/float.m b/library/float.m
>> index 0d246d8..120ef09 100644
>> --- a/library/float.m
>> +++ b/library/float.m
> ...
>> +uint_hash(Key, Hash) :-
>> +    uint.hash(Key, Hash).
>>
>> -float_hash(F, float.hash(F)).
>> +float_hash(F, Hash) :-
>> +    float.hash(F, Hash).
>>       % There are almost certainly better ones out there...
>>
>>   char_hash(C, H) :-
>> +    char.hash(C, H).
>>       % There are almost certainly better ones out there...
>> -    int_hash(char.to_int(C), H).
>>
>> -string_hash(S, string.hash(S)).
>> +string_hash(S, H) :-
>> +    string.hash(S, H).
>>       % There are almost certainly better ones out there...
>>
>>   %---------------------------------------------------------------------------%
>
> Might as well delete the comments.

Done.

Thanks for that!

Julien.


More information about the reviews mailing list