-
Notifications
You must be signed in to change notification settings - Fork 61
Description
The TDCtx type currently has a manually-defined Hashable instance:
swarm/src/swarm-lang/Swarm/Language/Types.hs
Lines 851 to 858 in cd7ab5c
| -- Need to write manual Hashable instance since MonoidMap | |
| -- does not have instances of its own. | |
| instance Hashable TDCtx where | |
| hashWithSalt s (TDCtx ctx versions) = | |
| s | |
| `hashWithSalt` ctx | |
| `hashWithSalt` (getSum <$> MM.toMap versions) |
The release of monoidmap-hashable should make it possible to auto-derive this instance.
However, it turns out that Data.Semigroup.Sum is not (yet!) an instance of Hashable. (See haskell-unordered-containers/hashable#326.)
This means we still need to do a bit of work to auto-derive the Hashable instance for TDCtx.
Potential workarounds
I can think of two workarounds (until hashable is upgraded):
-
Define an orphan instance of
HashableforSum. Of course, an orphan instance is not ideal. However, if thehashablepackage is eventually upgraded to provide such an instance, this would lead to a compile error for our orphan instance, prompting us to remove the instance, which is actually what we want. -
Define a custom
TDVersionSumtype, analogous toSum, but with aHashableinstance. Then we could write:data TDCtx = TDCtx { getTDCtx :: Ctx TDVar TydefInfo , getTDVersions :: MonoidMap Text (TDVersionSum Int) } deriving (Eq, Generic, Hashable, Show, ToJSON) newtype TDVersionSum a = TDVersionSum {getTDVersionSum :: a} deriving (Eq, Generic, Hashable, Show, ToJSON) deriving (Num, Semigroup, Monoid, MonoidNull) via Sum a
However, option 2 has the disadvantage that the compiler would not automatically notify us if the hashable package is eventually upgraded so that Sum becomes an instance of Hashable.