snoyberg / mono-traversable

Type classes for mapping, folding, and traversing monomorphic containers
152 stars 61 forks source link

Redefine `forever` #197

Open parsonsmatt opened 2 years ago

parsonsmatt commented 2 years ago

Currently ClassyPrelude exports forever as defined in base.

forever :: (Applicative f) => f a -> f b
forever action = go
  where
    go = action *> go

The forever space leak is documented here, along with links to discussion and other reports. The issue is that the default implementation of *> is:

class Applicative f where
    -- ...
    (*>) :: f a -> f b -> f b
    a1 *> a2 = (id <$ a1) <*> a2

And, unless you specifically define one of (<$) or (*>), then the implementation of forever will cause a space leak.

resoucet is subject to this, and I suspect that HandlerFor is as well. Those types use DeriveFunctor, which may avoid the issue.

I'm proposing that this library export:

forever :: (Monad m) => m a -> m b
forever action = go
  where
    go = action >> go

foreverA :: (Applicative f) => f a -> f b
foreverA = Control.Monad.forever

This should prevent this footgun from impacting users of this library.

EDIT: HandlerFor does not have this issue

snoyberg commented 2 years ago

I remember a similar issue a while ago with for_ I think.

I'm about -0.5 on this proposal. It's a breaking change, which I'm generally against. It also deviates away from base further and makes the standard function slightly less useful. Given that this is an upstream issue as well, patching it here seems like it's just hiding a footgun, since any library you use may decide to use a forever from base that has this issue. It seems like education would be a better option here.

An alternative I'm slightly less bothered by would be putting a WARNING on forever, and then exporting both foreverM and foreverA with docs to explain why you would want one versus the other.

parsonsmatt commented 2 years ago

The WARNING approach sounds good to me - a great way to do education by API design :) I can work up a PR if you'd like.

snoyberg commented 2 years ago

That would be great, thanks!