Deriving Hashable

[fehrenbach]

This implements deriving support for Hashable, as discussed in purescript/purescript-prelude#188.

The generated code is not quite ideal at the moment. For this datatype:

data Foo = Foo Int Boolean

I basically generate this code:

instance hashableFoo :: Hashable Foo where
  hash (Foo x y) =
    Hash ((0*31
           + case hash x of
               Hash x' => x')*31)
           + case hash y of
               Hash y' => y')

which results in the following code:

var hashableFoo = new Data_Hashable.Hashable(function () {
    return eqFoo;
}, function (x) {
    return (((0 * 31 | 0) + (function () {
        var v = Data_Hashable.hash(Data_Hashable.hashableInt)(x.value0);
        return v;
    })() | 0) * 31 | 0) + (function () {
        var v = Data_Hashable.hash(Data_Hashable.hashableBoolean)(x.value1);
        return v;
    })() | 0;
});

I'm slightly bothered by these IIFEs.
I've tried to generate code with let to unwrap, something like this:

hash (Foo x y) = let (Hash hx) = hash x
                     (Hash hy) = hash y
                 in Hash ((0*31 + hx)*31 + hy)

But I can't make it work. I always get errors about my let-bound names not being found.

If anyone has a clue what I need to do to make let work, even without a constructor like in the following, please let me know.

let forty = 40
   two = 2
in forty + two

This does not seem to work:

forty <- freshIdent "forty"
two <- freshIdent "two"
let fortyplustwo = Let FromLet [BoundValueDeclaration (ss, []) (VarBinder ss forty) (Literal ss (NumericLiteral (Left 40)))
                               ,BoundValueDeclaration (ss, []) (VarBinder ss two) (Literal ss (NumericLiteral (Left 2)))]
                   (App (App (Var ss (Qualified (Just dataSemiring) (Ident C.add))) (Var ss (Qualified Nothing forty))) (Var ss (Qualified Nothing two)))
pure fortyplustwo

[natefaubion]

Let destructuring is desgured before type classes are derived. The equivalent of

hash (Foo x y) = let (Hash hx) = hash x
                     (Hash hy) = hash y
                 in Hash ((0*31 + hx)*31 + hy)

Would be

hash (Foo x y) =
  case hash x of
    Hash hx -> case hash y of
      Hash hy -> Hash (...)

Or

hash (Foo x y) = case hash x, hash y of
  Hash hx, Hash hy -> Hash (...)

[fehrenbach]

Ah, that makes sense. Thanks!

[fehrenbach]

It took some doing, but this generates decent code now:

data Test = Test Boolean { a :: Int, b :: Array Number }

derive instance eqTest :: Eq Test
derive instance hashableTest :: Hashable Test

var hashableTest = new Data_Hashable.Hashable(function () {
    return eqTest;
}, function (x) {
    var v = Data_Hashable.hash(Data_Hashable.hashableArray(Data_Hashable.hashableNumber))(x.value1.b);
    var v1 = Data_Hashable.hash(Data_Hashable.hashableInt)(x.value1.a);
    var v2 = Data_Hashable.hash(Data_Hashable.hashableBoolean)(x.value0);
    return (((((0 * 31 | 0) + v2 | 0) * 31 | 0) + v1 | 0) * 31 | 0) + v | 0;
});

Thanks for the help, @natefaubion

[natefaubion]

What is the tradeoff here?

[fehrenbach]

This is the combining function used by Java for lists etc. (see https://docs.oracle.com/javase/7/docs/api/java/util/List.html#hashCode()). It has issues when used with strings, where certain length prefixes lead to collisions. I'm not sure how much of a problem that is in practice with arbitrary datatypes.

The Haskell hashable library uses combine h1 h2 = (h1 * 16777619) xor h2 (FNV-1, see https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function). I think the reason I didn't use this, is that I thought xor is not inlined like * and +, but it appears I'm wrong about that.

A better hash function would probably be something like murmur_32, which is used by Google's Guava library as the default for short hashes. The main problem is that it's quite a bit more code, which is a pain to generate in deriving. It's also slower to compute.

We could probably do even more clever things in the future. For small types we could find (minimal?) perfect hash functions, for example.

Related: I'm not at all certain that the seeds [0..] are the best we can do.

[flip111]

Would love to see this merged :)

[drathier]

What is this blocked on now?

[fehrenbach]

Mostly me, I dropped the ball.. I have an updated branch locally that derives instances using hashWithSalt and is debitrotted. There's some support code that still needs to be written (basically update the library parts to also use hashWithSalt) and ideally a test or two. I'll try to at least upload what I have, if not do the rest, over the next couple of weeks. Also, we certainly don't want to block 0.14 for this. I'm not sure if there's enough interest to get it reviewed before the release even if it was ready. ke 27. tammikuuta 2021 klo 12.00 Filip Haglund <notifications@github.com> kirjoitti:

…

What is this blocked on now? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3446 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABRTICPADGECTDRIBZUJ43S37W57ANCNFSM4F6Z3HRA> .

[JordanMartinez]

My vote is to not include this in the v0.14.0 release. If I recall, this isn't breaking, right? So it could be added in a v0.14.1 release or something? Even if not, the delay has been long enough in my opinion.

[JordanMartinez]

Any updates on this PR?