Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Could forever be Applicative f => f a -> Void instead? #327

Closed
aleator opened this issue Aug 19, 2020 · 7 comments · Fixed by #331
Closed

Could forever be Applicative f => f a -> Void instead? #327

aleator opened this issue Aug 19, 2020 · 7 comments · Fixed by #331
Labels
new Bring something new into library (add function or type or interface)

Comments

@aleator
Copy link
Contributor

aleator commented Aug 19, 2020

Just asking, since

foo :: IO (Async ())
foo = forever $ do ...

vs.

bar :: IO (Async ())
bar = async $ forever $ do ...

seems like common error for less attentive guys like me.

@chshersh chshersh added the question Further information is requested label Aug 19, 2020
@chshersh
Copy link
Contributor

Hi @aleator! We reexport forever from base and not much can be done with its type signature. One of relude goals is to keep the number of custom things as small as possible, we really would like to avoid introducing more surprises and differences for the library users.

However, I totally understand your concern. It would be really nice to prevent such situations from happening. I will refer to the following blog post that explains best-practices around specifying types for infinite loops:

Main idea, is that if your function returns infinite loop, its type should be something like IO void (polymorphic variable), and if an action expects an infinite loop, argument type should be something like IO Void (monomorphic type Void). Applying this suggestion to forever, we can see that its type should be:

forever :: Applicative f => f a -> f void

You can notice, that this is the same as the original type signature, just with a slightly different variable name:

forever :: Applicative f => f a -> f b

Thus being said, it is probably a good idea to replace b with void in type signature for forever directly in base to give a hint in the type signature. Or even to change the type to to forever :: Applicative f => f a -> f Void to avoid such problems completely. But this should be raised to the core library committee and it is up to them to decide how to change the type signature. Introducing a custom version of forever just with renamed type variables is not worth it in relude.

But what you can do to solve this problem, is to introduce a custom helper function that expects an infinite action:

asyncInfinite :: IO Void -> IO (Async Void)
asyncInfinite = async

And this would probably be a good function to be added to async as well 🙂 This won't prevent your original error from happening, but will be a hint and a compile-time guarantee on the input action.

@aleator
Copy link
Contributor Author

aleator commented Aug 19, 2020

I concede on the point of not causing surprises and this issue may be closed.

However, doesn't the article mentioned justify using the type variable version only with the convenience of not having to say absurdand not needing to import Data.Void? Relude already solves the latter issue and I disagree on the first one. Not having to write absurd is what causes the issue here and in the case mentioned in the article not having absurd is just plain confusing.

But, how would you guys feel about introducing toInfinityAndBeyond :: f a -> f Voidand then adding a hlint hint to use that instead of forever?

@chshersh
Copy link
Contributor

@aleator I agree with you that if you already have Void in scope, then the specific return type Void provides more guarantees. If the return type is polymorphic (e.g. void type variable), you can accidentally pass such IO void action as an IO Int argument, and the code will hang unexpectedly. I now think that always returning Void and expecting Void for actions that run infinitely is a good practice 👍 If you use absurd explicitly in the code to convert Void to Int, something suspsicious is happening, probably.

I don't mind having a different function besides forever that uses Void in the return type. We only need to come up with some name that doesn't have many conflicts on Hackage, but still has meaning.

@aleator
Copy link
Contributor Author

aleator commented Aug 21, 2020

@chshersh If good name is only thing required before I can make a pull request, then here are some suggestions. Maybe the good name is not on the list, but perhaps it will spark an idea:

  • amaranthinely (Quaranteed not to conflict. My favourite)
  • ceaselessly
  • perpetually
  • permanently
  • whileTrue

None seem to have obvious word conflicts. Any favourites?

@vrom911
Copy link
Member

vrom911 commented Aug 22, 2020

I agree that this function would be helpful to have!
Another thing to consider is to add it into the Extra module?
In that case we would be able to give it a more "clashing" name, that could probably used for some vars in code (but maybe I am overestimating names in here).

My options are:

  • always – there problem that it has the meaning of const in Purescript and Elm
  • infinitely
  • constantly

@aleator
Copy link
Contributor Author

aleator commented Aug 24, 2020

Hi @vrom911!

After thinking about these, I'd think infinitelyand perpetually would be good choices, but for me, the name does not matter so much.

Further, I'd vote for not putting it into an Extra module, but to always export it. How would you feel about adding the hlint to move people away from forever?

@vrom911
Copy link
Member

vrom911 commented Sep 2, 2020

@aleator , sounds good to me. I think we can add it in the Relude with the corresponding support HLint warning for forever 👍🏼
I would prefer infinitely personally from the above options 🙂

aleator added a commit to aleator/relude that referenced this issue Sep 9, 2020
This addresses kowainik#327. However, I couldn't regenerate
.hlint.yaml due to dhall version mismatch.
@aleator aleator mentioned this issue Sep 9, 2020
10 tasks
@chshersh chshersh added new Bring something new into library (add function or type or interface) and removed question Further information is requested labels Sep 16, 2020
@chshersh chshersh added this to the v0.8.0.0: Standard milestone Sep 16, 2020
vrom911 pushed a commit that referenced this issue Oct 7, 2020
* Add 'infinitely' to 'Relude.Monad'

This addresses #327. However, I couldn't regenerate
.hlint.yaml due to dhall version mismatch.

* Drop trailing whitespace

Co-authored-by: hint-man[bot] <44720633+hint-man[bot]@users.noreply.github.com>

* Update src/Relude/Monad.hs

Co-authored-by: Dmitrii Kovanikov <kovanikov@gmail.com>

* Update CHANGELOG.md

Co-authored-by: Dmitrii Kovanikov <kovanikov@gmail.com>

* Update CHANGELOG.md

Co-authored-by: Dmitrii Kovanikov <kovanikov@gmail.com>

Co-authored-by: Dmitrii Kovanikov <kovanikov@gmail.com>
Co-authored-by: hint-man[bot] <44720633+hint-man[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new Bring something new into library (add function or type or interface)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants