search

ekmett / machines

Networks of composable stream transducers
Other
339 stars 46 forks source link

Fix GHC 8.6 noncanonical-monoid-instances warnings. #105

Closed patrickt closed 5 years ago

patrickt commented 5 years ago

When compiling with -Wall, GHC warns if a Monoid instance defines mappend as something other than an alias for <>. The Monoid instances for Moore and Mealy machines defined their own mappend implementations, though they did not differ semantically from <>.

This replaces those mappend definitions with <>.

acowley commented 5 years ago

This looks good, but can we address those failures with older GHCs on travis? For example,

src/Data/Machine/Moore.hs:164:13: error:
    • Could not deduce (Semigroup b) arising from a use of ‘<>’
      from the context: Monoid b
        bound by the instance declaration
        at src/Data/Machine/Moore.hs:162:10-39
      Possible fix:
        add (Semigroup b) to the context of the instance declaration
    • In the expression: (<>)
      In an equation for ‘mappend’: mappend = (<>)
      In the instance declaration for ‘Monoid (Moore a b)’
    |
164 |   mappend = (<>)
    |             ^^^^
patrickt commented 5 years ago

@acowley Sorry about that; should be fixed now.

acowley commented 5 years ago

LGTM; thanks for taking care of this. Hopefully @RyanGlScott or @YoEight can take a last look and merge it soon.

patrickt commented 5 years ago

I’ve run into a snag when testing with GHC 8.2.2, which supports the -Wnoncanonical-monoid-instances warning, but does not have a base with the Semigroup/Monoid proposal applied. In this case, it appears not to be possible to suppress this warning without introducing unforced backcompat errors (/hattip @treeowl for pointing that out).

It’s probably just worth scrapping this PR, since this isn’t an error that most people find.