Make `Async a` `Hashable`

[kirelagin]

Async a already has instances of Eq and Ord, which makes it possible to store it in a Set, but it has no instance of Hashable, so it is not possible to store it in a HashSet, while the idea of having a HashSet of running threads seems to be quite useful.

Given that ThreadId already has an instance of Hashable and Eq and Ord of Async a delegate to the internal ThreadId, providing such an instance would be trivial, although it would require depending on hashable.

If this is acceptable, I can prepare a PR.

[simonmar]

Yes!

[sol]

I'm not too excited about this change for

• the added dependency on hashable (which e.g. currently does not build with GHC HEAD)
• the transitive dependency on text, which is (a) heavy weight (b) not a boot library with older versions of GHC

While I understand the virtue of this change, it also induces a cost on every user of async.

How I see it, this is a trade of between cost and functionality. Making a decision either way is valid. I just wanted to voice my preference here, which is not adding the additional dependencies and keeping async as lightweight as possible.

[kirelagin]

Yeah, I do realise that it is somewhat of a compromise, that’s why I started by asking whether Simon considers adding a dependency on hashable possible.

And I agree that some users may see this as an additional cost they did not ask for. But, on the other hand, I believe that any non-trivial Haskell project is very likely to depend on text and probably on hashable too.

In the end it should be your decision. Another option (rather weird one, but probably easier on the users) would be to have a separate library (e.g. async-hashable) for this sole purpose.

[simonmar]

The alternative is to have hashable depend on async. I don't feel strongly either way, but if hashable already has a large set of dependencies perhaps this is the better way to go. Putting the instance in a separate library is not good because orphans.

[kirelagin]

if hashable already has a large set of dependencies

It does not :(

  Build-depends:     base >= 4.4 && < 4.11,
                     bytestring >= 0.9 && < 0.11,
                     deepseq >= 1.3
  if impl(ghc)
    Build-depends:   ghc-prim,
                     text >= 0.11.0.5
  if impl(ghc) && flag(integer-gmp)
    Build-depends:   integer-gmp >= 0.2

We can ask what they think nevertheless, but I suspect they won’t be too excited about a transitive dependency on stm either.

[LeifW]

You could add it as an optionally built module + dep via cabal flag?

On Jan 8, 2018 1:07 AM, "Kirill Elagin" notifications@github.com wrote:

if hashable already has a large set of dependencies

It does not :(

Build-depends: base >= 4.4 && < 4.11, bytestring >= 0.9 && < 0.11, deepseq >= 1.3 if impl(ghc) Build-depends: ghc-prim, text >= 0.11.0.5 if impl(ghc) && flag(integer-gmp) Build-depends: integer-gmp >= 0.2

We can ask what they think nevertheless, but I suspect they won’t be too excited about a transitive dependency on stm either.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/simonmar/async/issues/73#issuecomment-355913344, or mute the thread https://github.com/notifications/unsubscribe-auth/AABwJ--fj0EIovZUNtpdbHUMFtdYnLovks5tIdrggaJpZM4RHqPT .

[simonmar]

You could add it as an optionally built module + dep via cabal flag?

No, that's even worse. Packages should have a stable API. How would you express a dependency on the version of async with the flag enabled?

FWIW, GHC will have a dependency on text soon, so I don't think a dependency on text is terrible.

[sol]

For hspec/hspec I now include a "vendored" copy of async without the hashable dependency. It's ok with me to close this issue.