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

Take care of boundaries in Step trait #20249

Closed
bluss opened this issue Dec 26, 2014 · 8 comments
Closed

Take care of boundaries in Step trait #20249

bluss opened this issue Dec 26, 2014 · 8 comments
Labels
A-traits Area: Trait system

Comments

@bluss
Copy link
Member

bluss commented Dec 26, 2014

Watch out for min/max boundaries in the Step trait.

It would be more natural for (0i8..) to produce a finite range, ending with the maximum value 127i8, than to overflow and produce an infinite iterator. Overflow is problematic, (and if we let it stay in, users will rely on the wrap around behavior.) and the Range objects and Step traits need to handle this with care.

The current Step trait supposes that every element has a successor, which is not necessarily true.

@bluss
Copy link
Member Author

bluss commented Dec 26, 2014

cc @nick29581

@nrc
Copy link
Member

nrc commented Dec 26, 2014

This was intentional and discussed with a bunch of people on irc. The majority felt that an open ended interval should repeat forever, rather than being bounded by the maximum. I feel this could go either way though. Certainly true open-endedness is more efficient, but if you actually rely on the value produced, then it is probably not what the user expects. OTOH, I expect that such iterators would only ever be used with take so this edge case doesn't matter too much.

@bluss
Copy link
Member Author

bluss commented Dec 26, 2014

But the intervals won't repeat this way -- not with the coming overflow rules and overflow checks. It seems wiser to insert the boundary check manually (and I think to terminate iteration there).

What do you think about the Step trait w.r.t this? http://discuss.rust-lang.org/t/a-tale-of-twos-complement/1062

@nrc
Copy link
Member

nrc commented Dec 27, 2014

I'm not really sure, I think it only makes sense to overflow here (panicking is probably wrong, but then, perhaps not, perhaps it is the user's responsibility not to overflow - that certainly fits with the zero-cost abstraction theme for Rust), which would imply using WrappingInt, or whatever the type will be

The more I think about this, the more I think that letting an open ended range end up overflowing is an error of use - i.e., that they should only be taken or whatever. Anything else, means they are not open ended, and if the user wants a range limited by MAX_INT, then they can use ..MAX_INT.

@rustonaut
Copy link

I agree with @nick29581 that letting a open ended rage overflow is an error of use.

Therefore I think it would be the best (in combination with the coming overflowing rules) to let a overflowing rang panic (in both relate and debug bullies).
If the user want wrapping behaviour he/she can use the coming WrappedInt (if it is supported by the range operator).
And if the user wants a ending range ..INT_MAX can be used like mentioned above.

@bluss
Copy link
Member Author

bluss commented Dec 31, 2014

Since we have exclusive-end ranges, 1i32.. and 1i32..i32::MAX would be different if the enumeration ended at the bound, 1i32.. would be like 1i32...i32::MAX.

@kmcallister kmcallister added A-traits Area: Trait system A-libs labels Jan 16, 2015
@aturon
Copy link
Member

aturon commented Jan 23, 2015

This behavior was changed in a recent stabilization PR to panic on overflow on debug builds (matching the anticipated overflow RFC).

@aturon aturon closed this as completed Jan 23, 2015
@bluss
Copy link
Member Author

bluss commented Jan 23, 2015

As long as the panic checks are elided in bounded loops, the performance fallout won't matter that much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system
Projects
None yet
Development

No branches or pull requests

5 participants