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

Remove block argument from `loop` #6026

Merged
merged 1 commit into from Apr 28, 2018

Conversation

@asterite
Contributor

asterite commented Apr 28, 2018

Introducing a block argument here was a mistake. Some people now want the loop index to be a different value. But the worse thing is that the loop index can silently overflow. And if we ever change the language to raise on overflow, this could eventually break some long running programs like servers (and that wouldn't be nice).

loop is intended to be a cute syntax sugar for while true while also allowing you to keep the variable inside the loop local to the block.

Closes #5975

@straight-shoota

This comment has been minimized.

Contributor

straight-shoota commented Apr 28, 2018

Wow, the counter var was never actually used in the compiler and stdlib?

@RX14

RX14 approved these changes Apr 28, 2018

@RX14

This comment has been minimized.

Member

RX14 commented Apr 28, 2018

@straight-shoota i didn't even realise loop do had an index lol

@RX14 RX14 merged commit dc2b8f0 into crystal-lang:master Apr 28, 2018

4 checks passed

ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@RX14 RX14 added this to the Next milestone Apr 29, 2018

@Willamin

This comment has been minimized.

Contributor

Willamin commented Apr 30, 2018

In the event anyone wants this feature back, I've put together a shard to replace this functionality. When the compiler eventually raises an exception on integer overflows, I'll patch my shard to handle that accordingly.

Sidenote: the ability for me to bring a small feature like this back is one of my favorite features of Crystal! 😄

@straight-shoota

This comment has been minimized.

Contributor

straight-shoota commented Apr 30, 2018

@Willamin I don't think this is helpful. That behaviour was removed for a reason. If you want to use a short cut to have looping block with an incrementing counter, it should have a different name at least.
And it hardly makes any sense to require a shard for such a simple six line method.

I'll patch my shard to handle that accordingly.

What would that be? If there was a way to deal with that which would work for every use case, it could've stayed in the stdlib implementation.

@RX14

This comment has been minimized.

Member

RX14 commented Apr 30, 2018

When the compiler eventually raises an exception on integer overflows, I'll patch my shard to handle that accordingly.

and do what? reset back to 0? That's even worse than raising!

chris-huxtable added a commit to chris-huxtable/crystal that referenced this pull request Jun 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment