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=2cbad0e7ced9de896eb9a1732631786a6adb676a *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 <b...@smart-cactus.org> wrote: > Richard Eisenberg <r...@richarde.dev> 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 >
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
