Re: instance {Semigroup, Monoid} (Bag a) ?

Good morning all, I have now split Richard's commit in two, so that it's explicit in the commit history that we are adding those instances: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/5509/diffs?commit_id=2cb... *Ben*: I have briefly tried to add the Hlint annotations, but it turned out to be trickier than anticipated. I couldn't find any easy way in HLint to add a custom hint on a *typeclass instance*, and adding something like this into `compiler/.hlint.yaml` didn't change anything: - modules: - {name: GHC.Data.Bag, badidents: [mempty], message: "Use emptyBag as it's more descriptive"} - {name: GHC.Data.Bag, badidents: [(<>)], message: "Use unionBags as it's more descriptive"} (I suspect that's not what's the badidents syntax is for). I have also tried to add two `ANN` to the definitions, like this: {-# ANN (<>) ("HLint: use unionBags as it's more descriptive" :: String) #-} instance Semigroup (Bag a) where (<>) = unionBags However, this won't work as (<>) is imported qualified and there is no way in the ANN syntax to specify a fully qualified identifier. However, I suspect this is not the correct syntax either, as `(<>)` is really the *typeclass* method, but we want to target the particular instance implementation. *Having said that*, Richard carefully defined `(<>) = unionBags` and `mempty = emptyBag`, so I agree that using `(<>)` and `mempty` would be more opaque but at the end of the day it should have the asymptotic complexity than using `unionBags` and `emptyBag` (modulo dictionary passing, but I hope that won't each our lunch). A. On Thu, 15 Apr 2021 at 02:20, Ben Gamari wrote:

Richard Eisenberg writes:

...

Hi devs,

In the work on simplifying the error-message infrastructure (heavy lifting by Alfredo, in cc), I've been tempted (twice!) to add

...

instance Semigroup (Bag a) where (<>) = unionBags

instance Monoid (Bag a) where mempty = emptyBag

to GHC.Data.Bag.

The downside to writing these is that users might be tempted to write e.g. mempty instead of emptyBag, while the latter gives more information to readers and induces less manual type inference (to a human reader). The upside is that it means Bags work well with Monoid-oriented functions, like foldMap.

I favor adding them, and slipped them into !5509 (a big commit with lots of other stuff). Alfredo rightly wondered whether this decision deserved more scrutiny, and so I'm asking the question here.

My sense is that adding the instances is a Good Thing. However, I do think that we probably ought to refrain from using (<>) and mempty where more specific functions would do. Adding a lint would be one way to accomplish this. Hiding the functions from GhcPrelude would be another.

Cheers,

- Ben

_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs