GHC 8.8: to `fail` or to `error`?

[RyanGlScott]

zippers-0.2.5 fails to build with GHC 8.8.1-alpha1:

src/Control/Zipper/Internal.hs:541:17: error:
    • Could not deduce (MonadFail ((->) a))
        arising from a use of ‘fail’
      from the context: Monad m
        bound by the type signature for:
                   jerks :: forall (m :: * -> *) a.
                            Monad m =>
                            (a -> m a) -> Int -> a -> m a
        at src/Control/Zipper/Internal.hs:539:1-49
    • In the expression: fail "jerks: negative jerk count"
      In an equation for ‘jerks’:
          jerks f n0
            | n0 < 0 = fail "jerks: negative jerk count"
            | otherwise = go n0
            where
                go 0 a = return a
                go n a = f a >>= go (n - 1)
    |
541 |   | n0 < 0    = fail "jerks: negative jerk count"
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

At first, I thought this was a simple issue of supplying too much eta-expansion. I made the following change:

diff --git a/src/Control/Zipper/Internal.hs b/src/Control/Zipper/Internal.hs
index 6606c89..34d2e64 100644
--- a/src/Control/Zipper/Internal.hs
+++ b/src/Control/Zipper/Internal.hs
@@ -536,9 +536,9 @@ farthest f = go where
 --
 -- >>> fmap rezip $ zipper "silly" & within traverse >>= jerks rightward 3 <&> focus .~ 'k'
 -- "silky"
-jerks :: Monad m => (a -> m a) -> Int -> a -> m a
+jerks :: MonadFail m => (a -> m a) -> Int -> a -> m a
 jerks f n0
-  | n0 < 0    = fail "jerks: negative jerk count"
+  | n0 < 0    = \_ -> fail "jerks: negative jerk count"
   | otherwise = go n0
   where
     go 0 a = return a

This, however, reveals a more interesting failure:

[1 of 2] Compiling Control.Zipper.Internal ( src/Control/Zipper/Internal.hs, interpreted )

src/Control/Zipper/Internal.hs:594:9: error:
    • Could not deduce (MonadFail m) arising from a use of ‘jerks’
      from the context: MonadPlus m
        bound by the type signature for:
                   jerkTo :: forall (m :: * -> *) h a i.
                             MonadPlus m =>
                             Int -> (h :> (a :@ i)) -> m (h :> (a :@ i))
        at src/Control/Zipper/Internal.hs:592:1-60
      Possible fix:
        add (MonadFail m) to the context of
          the type signature for:
            jerkTo :: forall (m :: * -> *) h a i.
                      MonadPlus m =>
                      Int -> (h :> (a :@ i)) -> m (h :> (a :@ i))
    • In the expression: jerks rightward (n - k) z
      In a case alternative: LT -> jerks rightward (n - k) z
      In the expression:
        case compare k n of
          LT -> jerks rightward (n - k) z
          EQ -> return z
          GT -> jerks leftward (k - n) z
    |
594 |   LT -> jerks rightward (n - k) z
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^

For context, here is the full definition of jerkTo:

jerkTo :: MonadPlus m => Int -> (h :> a:@i) -> m (h :> a:@i)
jerkTo n z = case compare k n of
  LT -> jerks rightward (n - k) z
  EQ -> return z
  GT -> jerks leftward (k - n) z
  where k = tooth z

It's tempting to fix jerkTo by simply adding an MonadFail constraint. But that doesn't smell quite right. If you stare at the call sites of jerks, you'll notice that it is never invoked with a negative number, so there's no way that jerkTo can ever actually invoke fail via jerks. In other words, we'd be incurring a completely unnecessary MonadFail constraint.

The only call site of jerks within zippers itself is the jerkTo function, so it's tempting to replace jerks' use of fail with error to avoid incurring a MonadFail constraint altogether. On the other hand, jerks is itself exported as a part of the public API, so it's entirely possible for someone to invoke jerks elsewhere with a negative number, in which case it would likely be preferable to use fail over error.

Another option is to swap out the use of jerks in jerksTo with an internal copy of jerks that calls error instead of fail.

[RyanGlScott]

I've adopted the "swap out the use of jerks in jerksTo with an internal copy of jerks that calls error instead of fail" approach in commit c5f6ed66d35a90072a726d9395277762ff5a2d63.