search

reflex-frp / patch

Data structures for describing changes to other data structures.
https://hackage.haskell.org/package/patch
BSD 3-Clause "New" or "Revised" License
16 stars 13 forks source link

Getting rid of `Data.Monoid.DecidablyEmpty` #44

Open endgame opened 2 years ago

endgame commented 2 years ago

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:

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.

endgame commented 2 years ago

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

Ericson2314 commented 2 years ago

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

Ericson2314 commented 2 years ago

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 commented 8 months ago

@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 .