Skip to content

Range::step_by(0) loops forever #26239

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

Closed
llogiq opened this issue Jun 12, 2015 · 3 comments
Closed

Range::step_by(0) loops forever #26239

llogiq opened this issue Jun 12, 2015 · 3 comments

Comments

@llogiq
Copy link
Contributor

llogiq commented Jun 12, 2015

I believe that the step_by(_) method of ranges should panic! if the argument is zero. Currently it loops forever, which is a footgun. At the very least, iterating over such a StepBy should panic! on debug builds. However, I think the earlier the error appears, the better.

Note that this may require us to specialize step_by to types whose Add implementation have a known identity element – at the moment that would mean bounding by the num::Zero trait, which is unstable, also it would probably reduce the usefulness of ranges to numeric types. However, special casing numeric ranges could solve this.

I have also filed an issue at rust-clippy to create a lint for cases where it can be statically detected. Still, that lint cannot detect cases where the argument is calculated at runtime.

The documentation of .step_by(_) should at least warn of the possibility of looping forever or panic once it is implemented.

@Thiez
Copy link
Contributor

Thiez commented Jun 12, 2015

There is nothing unsafe about looping forever, so I strongly oppose a panic!(). As a user I would expect (a..b).step_by(c).take(n) to work for all c, without having to special case for c == 0..

@llogiq
Copy link
Contributor Author

llogiq commented Jun 12, 2015

@Thiez: Agreed, this is not inherently unsafe. So maybe panic!() on building the StepBy is not the right option. Still, there are enough options to loop forever already, and as I said, the docs should at least remind the reader that supplying 0 as an argument creates an infinite range.

It may also be useful to be able to issue a warning on debug builds iff a `StepBy { step_by: 0, .. } is actually iterated; perhaps as an additional Debug option.

@alexcrichton
Copy link
Member

This is currently intended behavior, and for now I'm going to close this in favor of the discussion in stabilizing step_by as this is definitely a useful data point, but perhaps not big enough to have its own issue right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants