noncanonical monoid instance for mooreT

[cartazio]

src/Data/Machine/MooreT.hs:29:1: warning: [-Wunused-imports]
    The import of ‘Control.Applicative’ is redundant
      except perhaps to import instances from ‘Control.Applicative’
    To import instances alone, use: import Control.Applicative()
   |
29 | import Control.Applicative
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^

src/Data/Machine/MooreT.hs:132:3: warning: [-Wnoncanonical-monoid-instances]
    Noncanonical ‘mappend’ definition detected
    in the instance declaration for ‘Monoid (MooreT m a b)’.
    Define as ‘mappend = (<>)’

cc @RyanGlScott @davean

[RyanGlScott]

Annoyingly, fixing this warning will require a breaking API change. This is because the current Semigroup and Monoid instances for MooreT are:

https://github.com/ekmett/machines/blob/7190a4d89e3b59c1dd72102cd6f2574a054ee7fa/src/Data/Machine/MooreT.hs#L125-L133

If we define mappend = (<>), however, then the Monoid instance will fail to compile on pre-8.4 GHCs, since Semigroup was not a superclass of Monoid back then. We can repair the instance by doing something like this:

instance ( Applicative m, Monoid b
#if !(MIN_VERSION_base(4,11,0))
         , Semigroup b
#endif
         ) => Monoid (MooreT m a b) where
  mempty = pure mempty
  {-# INLINE mempty #-}
  mappend = (<>)
  {-# INLINE mappend #-}

But that constitutes a breaking change on pre-8.4 GHCs. This is probably the right thing to do, but it might be worth waiting until there is another breaking change we can make at the same time to avoid major version churn.

[cartazio]

Sounds reasonable to me

On Sat, Mar 7, 2020 at 8:59 AM Ryan Scott notifications@github.com wrote:

Annoyingly, fixing this warning will require a breaking API change. This is because the current Semigroup and Monoid instances for MooreT are:

https://github.com/ekmett/machines/blob/7190a4d89e3b59c1dd72102cd6f2574a054ee7fa/src/Data/Machine/MooreT.hs#L125-L133

If we define mappend = (<>), however, then the Monoid instance will fail to compile on pre-8.4 GHCs, since Semigroup was not a superclass of Monoid back then. We can repair the instance by doing something like this:

instance ( Applicative m, Monoid b#if !(MIN_VERSION_base(4,11,0)) , Semigroup b#endif ) => Monoid (MooreT m a b) where mempty = pure mempty {-# INLINE mempty #-} mappend = (<>) {-# INLINE mappend #-}

But that constitutes a breaking change on pre-8.4 GHCs. This is probably the right thing to do, but it might be worth waiting until there is another breaking change we can make at the same time to avoid major version churn.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ekmett/machines/issues/109?email_source=notifications&email_token=AAABBQXMJUD2PRUXIML247TRGJHMNA5CNFSM4LDMG7K2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOD2EGY#issuecomment-596091419, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQWXJWQPJN456ZGA6EDRGJHMNANCNFSM4LDMG7KQ .