VinylRecords / Vinyl

Extensible Records for Haskell. Pull requests welcome! Come visit us on #vinyl on freenode.
http://hackage.haskell.org/package/vinyl
MIT License
260 stars 49 forks source link

Hashable instance? #158

Open adamConnerSax opened 2 years ago

adamConnerSax commented 2 years ago

Would it be possible to add a Hashable instance for Rec ElField rs (with reasonable constraints on the rs)? I've added that here and there but that creates an orphan and I'm not sure I'm doing it right...

What I've got now (I've only ever used the "else" branch but that crashes ghc 9.2.2, hence the other version..which might be better?)

#if MIN_VERSION_base(4,16,0)
import GHC.Generics (Generic, Rep)
import Data.Hashable.Generic as Hash
#endif

-- | This is only here so we can use hash maps for the grouping step.  This should properly be in Frames itself.
#if MIN_VERSION_base(4,16,0)
instance (Generic (V.Rec V.ElField rs), Eq (V.Rec V.ElField rs), Hash.GHashable Hash.Zero (Rep (V.Rec V.ElField rs))) => Hash.Hashable (V.Rec V.ElField rs) where
  hashWithSalt = Hash.genericHashWithSalt
  {-# INLINEABLE hashWithSalt #-}
#else
instance Hash.Hashable (F.Record '[]) where
  hash = const 0
  {-# INLINABLE hash #-}
  hashWithSalt s = const s -- TODO: this seems BAD! Or not?
  {-# INLINABLE hashWithSalt #-}

instance (V.KnownField t, Hash.Hashable (V.Snd t), Hash.Hashable (F.Record rs), rs F.⊆ (t ': rs)) => Hash.Hashable (F.Record (t ': rs)) where
  hashWithSalt s r = s `Hash.hashWithSalt` (F.rgetField @t r) `Hash.hashWithSalt` (F.rcast @rs r)
  {-# INLINABLE hashWithSalt #-}
#endif
acowley commented 2 years ago

Does the first branch of the #if not work for you? It sure looks nicer :smile:

adamConnerSax commented 2 years ago

It might be fine!

But I haven't tested it since I failed to get 9.2.2 up and running because of a ghc bug that affects my vinyl instances in a few places. But if the generic version works, I'd be happier with it as well!

Might be worth testing for correctness and compilation speed? Various vinyl things cause me very slow compilation time--which seems like it's related to GHC issues that are being worked but must be hard because they remain in GHC!

In either case, having an instance in vinyl seems like it would make sense...