haskell-unordered-containers / hashable

A class for types that can be converted to a hash value
BSD 3-Clause "New" or "Revised" License
103 stars 86 forks source link

Hashable1 #114

Closed andrewthad closed 7 years ago

andrewthad commented 8 years ago

It looks like Edward Kmett has already done something like this in the hashable-extras package, but I'd like to propose that the classes Hashable1 and Hashable2 be added to hashable. To be able to provide a lot of more useful instances of these, hashable would also need to depend on transformers (just for the data types in the Data.Functor.* modules, not for any of the monad transformers). What do you think of this?

andrewthad commented 8 years ago

Also, the reason I would like these in hashable itself and not in a companion package is that we could then have instances like:

instance (Hashable1 f, Hashable1 g, Hashable a) => Hashable (Compose f g a)

With the current split between hashable and hashable-extras, this would be an orphan instance.

andrewthad commented 8 years ago

@tibbe I don't know if you ever had a chance to think about this, but I just ran into a situation where I needed this again, so I thought I would bring it back up. Since I first raised this issue, the Data.Functor.* data types from transformers have been moved into base, so the suggested additions no longer require taking a dependency on transformers.

Also, just to clarify, the hashable-extras package has Hashable1 written in the older higher-kinded typeclass style that was present in versions of transformers before 5.0. I would rewrite Hashable1 as:

class Hashable1 t where
    liftHashWithSalt :: (Int -> a -> Int) -> Int -> t a -> Int
    liftHash :: (a -> Int) -> t a -> Int

I will gladly do all of the work needed to make this happen. Let me know if you would accept a PR that implements this.

tibbe commented 8 years ago

Just so I can put this in perspective, can you give an example of how you'd use this?

P.S. We need to support the 3 last releases of GHC, so we would still have to rely on transformers if we went ahead with this.

andrewthad commented 8 years ago

The simplest demonstration of its utility is the Hashable instance for Compose:

class Hashable1 t where
  liftHashWithSalt :: (Int -> a -> Int) -> Int -> t a -> Int
  liftHash :: (a -> Int) -> t a -> Int

newtype Compose f g x = Compose { getCompose :: f (g x)) }

instance (Hashable1 f, Hashable1 g, Hashable x) => Hashable (Compose f g x) where
  hash (Compose v) = liftHash (liftHash hash) v
  hashWithSalt s (Compose v) = liftHashWithSalt (liftHashWithSalt hashWithSalt) s v

But it shows up elsewhere. Many data types that are parameterized over a * -> * type constructor need this to write Hashable instances. For example:

instance Hashable1 f => Hashable (Fix f)
instance (Hashable1 f, Hashable a) => Hashable (Free f a)
instance (Hashable1 f, Hashable a) => Hashable (Cofree f a)

I'm fine with leaving out the Hashable instance for Compose for now. I would rather keep a smaller set of dependencies for hashable. Providing the Hashable1 machinery, along with Hashable1 instances for stuff in base, is what I'm most interested in.

On Tue, Oct 18, 2016 at 01:59:42PM -0700, Johan Tibell wrote:

Just so I can put this in perspective, can you give an example of how you'd use this?

P.S. We need to support the 3 last releases of GHC, so we would still have to rely on transformers if we went ahead with this.

You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/tibbe/hashable/issues/114#issuecomment-254637786

tibbe commented 8 years ago

Perhaps we should add it to a new Data.Hashable.XXX module (XXX is up for bike-shedding, it could be Hashable1 or something else that indicates this is for higher order kinds). This would prevent Data.Hashable from getting too large and move less used functionality out of the way.

andrewthad commented 8 years ago

I'm fine with exposing through a new module instead of through Data.Hashable. The definition itself has to go in Data.Hashable.Class to prevent orphan instances. For the name of XXX, I would be fine with Lifted or something like that (HigherKinded is most descriptive but feels too long).

tibbe commented 8 years ago

No problem putting the implementation in D.H.Class since that's not exposed. Lets go for .Lifted and add top-level Haddock comment to that module explaining what it contains.

andrewthad commented 8 years ago

Cool. I'll PR sometime tomorrow.

andrewthad commented 8 years ago

https://github.com/tibbe/hashable/pull/124