Re: A more useful Monoid instance for Data.Map

Irrespective of the eventual replacement, I propose that we remove the current Monoid instance right now. This would also give proprietary codebases more time to adapt to the semantic change. Christian On 2013-01-05 21:00, Roman Cheplyaka wrote:

Great job!

To be fair, there might be code which is not released on hackage (e.g. proprietary), but the current instance is so bad that we really should take a chance to fix it.

But there's another problem... The "right" instance should really be based on Semigroup, not Monoid, but Semigroup is not currently in the base. And adding a dependency on semigroups to containers is hardly an option.

So maybe we should push the change that was discussed in another thread to move Semigroup into base, and only then change this instance.

Roman

* Christian Sattler [2013-01-05 20:07:02+0100]

...

Hi all,

After repeated frustration over the wrong Monoid (Data.Map.Map k v) instance I finally went ahead and did a practical test concerning its current usage.

After removing the Monoid instance for Map and IntMap, each reverse dependency of containers was separately compiled under a standard setup of GHC 7.6.1 in order to avoid shared dependency problems. Out of 1440 reverse dependencies, I could get 545 to compile. However, only the following packages fail because of Monoid instance issues:

- caledon - data-default - dom-lt - EnumMap - i18n - semigroups - unamb-custom - vacuum - stringtable-atom

EnumMap has containers <0.3, semigroups declares <0.6, unamb-custom appears to be a private abandoned clone with uploads only on 24/12/08, stringtable-atom fails to build because of a previous API change for updateMax, and the rest only use the instance internally for saying mempty instead of Data.Map.empty.

Under these circumstances, fixing the Monoid instance mistake for containers 0.6.0.0 does not seem to introduce any semantic breakage at all. I have CCed the maintainers of the lastly mentioned packages.

Let's do it! Christian

2012/4/28 Daniel Peebles

...

I don't actually think there are any rules/optimizations for fmap of newtype constructors or extractors in general. Luckily, unsafeCoerce is explicitly specified to be safe, in this kind of situation (assuming the Map is actually parametric in its value type, which it is)! ;)

On Sat, Apr 28, 2012 at 12:31 PM, Evan Laforge wrote:

...

...

Why not be explicit about the replacement strategy by injecting your values into First/Last? My point is that in terms of functionality, using mappend on the values is strictly more general than the current instance. It seems unfortunate to be stuck with the current instance for historical reasons, but I guess that's how a lot of this stuff works :/ Yeah, I suppose it would be a bit more regular that way. I'm always reluctant to map newtypes over things other than lists because I don't

On Fri, Apr 27, 2012 at 7:00 PM, Daniel Peebles wrote: trust there to be a RULES that will eliminate it, but I suppose for Map there must be. I guess I wouldn't mind updating my code if the definition changed. It's hard to change a general purpose method though, simply because searching for it in your code will turn up so many false positives.

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries