Skip to content

Hashable1 #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
andrewthad opened this issue Mar 24, 2016 · 9 comments
Closed

Hashable1 #114

andrewthad opened this issue Mar 24, 2016 · 9 comments

Comments

@andrewthad
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@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
Copy link
Collaborator

tibbe commented Oct 18, 2016

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
Copy link
Contributor Author

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:
#114 (comment)

@tibbe
Copy link
Collaborator

tibbe commented Oct 19, 2016

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
Copy link
Contributor Author

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
Copy link
Collaborator

tibbe commented Oct 19, 2016

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
Copy link
Contributor Author

Cool. I'll PR sometime tomorrow.

@andrewthad
Copy link
Contributor Author

#124

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants