MonadReader instance for ContT has bad semantics

[KingoftheHomeless]

We'd expect local id == id, but this isn't true for ContT.

Let:

localTheCont :: MonadReader Int m => ContT r m ()
localTheCont = ContT $ \c -> local (+1) (c ())

test1 :: MonadReader Int m => ContT r m Int
test1 = localTheCont >> ask

test2 :: MonadReader Int m => ContT r m Int
test2 = local id localTheCont >> ask

then

runReader (evalContT test1) 1 == 2
runReader (evalContT test2) 1 == 1

Breaking apart the instance reveals what goes wrong:

  local id localTheCont
= ContC $ \c ->
    i <- ask
    local id $ runContC localTheCont (local (const i) . c)
= ContC $ \c ->
    i <- ask
    local id $ local (+1) ((local (const i) . c) ())
= ContC $ \c ->
    i <- ask
    local (const i . (+1) . id) (c ())
= ContC $ \c ->
    i <- ask
    local (const i) (c ())

Removing the instance would undoubtedly cause too much breakage, but perhaps a warning in the docs is warranted.

[ekmett]

It is known that Reader for Cont is a bit wonky. For me this seems like another argument for splitting up MonadReader into the ask and local parts. That said that transition will be painful and may have to be part of a large scale mtl 3 shift.