patch

patch copied to clipboard

There's a comment at the top of Data.Monoid.DecidablyEmpty which reads: -- TODO upstream somwhere else?

That "somewhere else" already exists: Data.Monoid.Null from monoid-subclasses.

I had a look at the instances it provides, and it is missing the following ones:

instance MonoidNull a => MonoidNull (Identity a)
instance MonoidNull a => MonoidNull (WrappedMonoid a)
instance (Ord a, Bounded a) => MonoidNull (Max a)
instance (Ord a, Bounded a) => MonoidNull (Min a)
instance MonoidNull (Proxy a)
instance MonoidNull a => MonoidNull (Const a b)
instance MonoidNull a => MonoidNull (Down a)
instance MonoidNull p => MonoidNull (Par 1 p)
instance MonoidNull (U1 p)
instance MonoidNull (f p) => MonoidNull (Rec1 f p)
instance MonoidNull (f p) => MonoidNull (M1 i c f p)
instance MonoidNull c => MonoidNull (K1 i c p)
instance (MonoidNull (f p), MonoidNull (g p)) => MonoidNull ((f :*: g) p)
instance (MonoidNull (f p), MonoidNull (g p)) => MonoidNull ((f :.: g) p)
instance (...) => MonoidNull (a, b, c, d, e)

-- Should go to dependent-map
instance GCompare k => MonoidNull (DMap k v)

This would solve #42 but is too much work to hold up GHC 9.2 support. Tasks, as I see them:

• [x] Add the missing instances to monoid-subclasses and release
• [ ] Add the missing instance to dependent-map and release
• [ ] Replace Data.Monoid.DecidablyEmpty.DecidablyEmpty with type alias, set isEmpty = Data.Monoid.Null.null, deprecate both, and release
• [ ] Remove deprecated Data.Monoid.DecidablyEmpty.

None of this seems particularly controversial. We might want monoid-subclasses anyway because of #37. It's quite a light package, so depending on it from dependent-map shouldn't be a problem either.

@Ericson2314 Let me know if you're happy with this plan, and I will begin making PRs.

@endgame this one I have more mixed feelings about because monoid-subclasses is such overkill for our use-case.

Also, if we we had more "non-empty" data types, like https://github.com/haskell/containers/pull/616, we could do this in a more "type directed" way which would be more satisfying.

(If you have spare cycles, I think they would be more usefully spent getting https://github.com/haskell/containers/pull/616 over the finish line. I hope the conflicts are not too bad! The only thing remaining before that I remember was dealing with some perf regressions, which I think is just a matter of tweaking the inlining.)

@endgame this one I have more mixed feelings about because monoid-subclasses is such overkill for our use-case.

It's not that big a package, and its library component has a pretty reasonable dependency footprint. Regardless, I opened https://github.com/blamario/monoid-subclasses/pull/50 .

@endgame This is awesome, thank you!

Since I got the github notification, I thought I'd do dependent-map while I had a couple of minutes.