search

vimus / libmpd-haskell

A client library for MPD, the Music Player Daemon
MIT License
36 stars 26 forks source link

MonadFail instance for Parser #101

Closed matthewleon closed 6 years ago

matthewleon commented 6 years ago

https://prime.haskell.org/wiki/Libraries/Proposals/MonadFail

joachifm commented 6 years ago

https://wiki.haskell.org/MonadFail_Proposal#Adapting_old_code suggests this pattern:

if !MIN_VERSION_base(4,11,0)
-- Control.Monad.Fail import will become redundant in GHC 7.16+
import qualified Control.Monad.Fail as Fail
#endif
import Control.Monad

instance Monad Foo where
 (>>=) = <...bind impl...>
#if !MIN_VERSION_base(4,11,0)
 -- Monad(fail) will be removed in GHC 7.16+
 fail = Fail.fail
#endif

instance MonadFail Foo where
 fail = <...fail implementation...>

Having only briefly compared the two, their version seems more robust, though more elaborate.

joachifm commented 6 years ago

Idea for the future is to add a MonadPlus instance, too, and set fail = mzero if possible. Also, I think it'd be good to eventually use one or more compat packages instead of CPP to do this stuff, but that's not a blocker.

matthewleon commented 6 years ago

I'll modify this according to the adaptation proposal. Missed that, thanks.

Idea for the future is to add a MonadPlus instance, too

I'll try to get to that as well.

matthewleon commented 6 years ago

Just attempted to use the recommended pattern, above. It fails right away when building with stack. MonadFail isn't available on base before 4.9. My guess is that their recommendation is aimed at people compiling under GHC 8+.

joachifm commented 6 years ago

I see. What about using https://hackage.haskell.org/package/fail? In the end I don't care too much, so long as it works. Can always be improved upon later.

matthewleon commented 6 years ago

I see. What about using https://hackage.haskell.org/package/fail?

Seems like a reasonable option, and one adopted elsewhere. I'll return to this soon. It indeed doesn't seem to be pressing.

I am curious, where do you see a lack of robustness in the original solution? The use of CPP is, of course, inelegant. But is there something that could cause a compilation error that I'm not catching?

joachifm commented 6 years ago

The alternate version looks like it will continue working as-is even after fail is eventually removed from Monad, whereas I expect your version would need updating.

matthewleon commented 6 years ago

The alternate version looks like it will continue working as-is even after fail is eventually removed from Monad, whereas I expect your version would need updating.

Got it. Thanks.

joachifm commented 6 years ago

Ach, I completely forgot about this one, sorry ...