<div dir="ltr">I actually had this very issue bite me *in an interview*. I would love these instances to be removed, and probably not replaced directly - instead, I think we should take the `Sum`/`Product` approach, and provide instances for newtypes around these type constructors so that there can be no backwards-incompatible-semantics issues.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 13, 2018 at 12:33 PM, David Feuer <span dir="ltr"><<a href="mailto:david.feuer@gmail.com" target="_blank">david.feuer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Many people have recognized for years that the Semigroup and Monoid<br>
instances for Data.Map, Data.IntMap, and Data.HashMap are not so<br>
great. In particular, when the same key is present in both maps, they<br>
simply use the value from the first argument, ignoring the other one.<br>
This somewhat counter-intuitive behavior can lead to bugs. See, for<br>
example, the discussion in Tim Humphries's blog post[*]. I would like<br>
do do the following:<br>
<br>
1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,<br>
Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of<br>
containers and unordered-containers.<br>
<br>
2. Remove the deprecated instances.<br>
<br>
3. After another several years (four or five, perhaps?), make a major<br>
release of each package in which the instances are replaced with the<br>
following:<br>
<br>
  instance (Ord k, Semigroup v) => Semigroup (Map k v) where<br>
    (<>) = Data.Map.Strict.unionWith (<>)<br>
  instance (Ord k, Semigroup v) => Monoid (Map k v) where<br>
    mempty = Data.Map.Strict.empty<br>
<br>
  instance Semigroup v => Semigroup (IntMap v) where<br>
    (<>) = Data.IntMap.Strict.unionWith (<>)<br>
  instance Semigroup v => Monoid (IntMap v) where<br>
    mempty = Data.IntMap.Strict.empty<br>
<br>
  instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k v) where<br>
    (<>) = Data.HashMap.Strict.unionWith (<>)<br>
  instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v) where<br>
    mempty = Data.HashMap.Strict.empty<br>
<br>
Why do I want the strict versions? That choice may seem a bit<br>
surprising, since the data structures are lazy. But the lazy versions<br>
really like to leak memory, making them unsuitable for most practical<br>
purposes.<br>
<br>
The big risk:<br>
<br>
Someone using old code or old tutorial documentation could get subtly<br>
wrong behavior without noticing. That is why I have specified an<br>
extended period between removing the current instances and adding the<br>
desired ones.<br>
<br>
Alternatives:<br>
<br>
1. Remove the instances but don't add the new ones. I fear this may<br>
lead others to write their own orphan instances, which may not even<br>
all do the same thing.<br>
<br>
2. Write separate modules with newtype-wrapped versions of the data<br>
structures implementing the desired instances. Unfortunately, this<br>
would be annoying to maintain, and also annoying to use--packages<br>
using the unwrapped and wrapped versions will use different types.<br>
Manually wrapping and unwrapping to make the different types work with<br>
each other will introduce lots of potential for mistakes and<br>
confusion.<br>
<br>
Discussion period: three weeks.<br>
<br>
[*] <a href="http://teh.id.au/posts/2017/03/03/map-merge/index.html" rel="noreferrer" target="_blank">http://teh.id.au/posts/2017/<wbr>03/03/map-merge/index.html</a><br>
______________________________<wbr>_________________<br>
Libraries mailing list<br>
<a href="mailto:Libraries@haskell.org">Libraries@haskell.org</a><br>
<a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries" rel="noreferrer" target="_blank">http://mail.haskell.org/cgi-<wbr>bin/mailman/listinfo/libraries</a><br>
</blockquote></div><br></div>