This results in a silent change of semantics for all current users and _will_ break existing code _silently_.

I am -1 on this change as there is no good way to manage the transition especially due to containers use as a boot package and the modicum of improvement strikes me as not worth the transition price.

Moreover it is the "wrong" constraint.

Maybe for instance is a terrible example to motivate this proposal as it adjoins a unit to a monoid pretending it is a semigroup to get a monoid.

To union two maps we don't need a unit.

Were Map/IntMap being defined from scratch? Reasonable. But given the very very large body of code that sits atop them that would break I have a hard time advocating for the ensuing debugging hell and silent breakages,

Sent from my iPhone

On May 18, 2014, at 5:05 PM, Nick Partridge <nkpart@gmail.com> wrote:

Hi,

Currently the Monoid instance for Data.Map is implemented using union/unions, which are left biased. On key collision, it discards values from the right hand side of `mappend` - https://github.com/ghc/packages-containers/blob/bae098fb0a3994bc2b0ec3313004b40cd097ed8d/Data/Map/Base.hs#L341-L344

If you compare this with the Monoid for Maybe, it's like we're defaulting to First as the monoid instance for maps.

A more useful instance, however very much a breaking change, would be to make the instance depend on a Monoid (or better yet, a Semigroup) for the values in the map:

instance Monoid v => Monoid (Map k v) where
    mappend = unionWith mappend

This lets us build up maps with values in a useful Monoid, and mappend them with gusto.

Thoughts?

- Nick Partridge

Discussion period: 2 weeks.
_______________________________________________
Libraries mailing list
Libraries@haskell.org
http://www.haskell.org/mailman/listinfo/libraries