Skip to content

Commit

Permalink
Rollup merge of rust-lang#72368 - CAD97:rangeto, r=dtolnay
Browse files Browse the repository at this point in the history
Resolve overflow behavior for RangeFrom

This specifies a documented unspecified implementation detail of `RangeFrom` and makes it consistently implement the specified behavior.

Specifically, `(u8::MAX).next()` is defined to cause an overflow, and resolve that overflow in the same manner as the `Step::forward` implementation.

The inconsistency that has existed is `<RangeFrom as Iterator>::nth`. The existing behavior should be plain to see after rust-lang#69659: the skipping part previously always panicked if it caused an overflow, but the final step (to set up the state for further iteration) has always been debug-checked.

The inconsistency, then, is that `RangeFrom::nth` does not implement the same behavior as the naive (and default) implementation of just calling `next` multiple times. This PR aligns `RangeFrom::nth` to have identical behavior to the naive implementation. It also lines up with the standard behavior of primitive math in Rust everywhere else in the language: debug checked overflow.

cc @Amanieu

---

Followup to rust-lang#69659. Closes rust-lang#25708 (by documenting the panic as intended).

The documentation wording is preliminary and can probably be improved.

This will probably need an FCP, as it changes observable stable behavior.
  • Loading branch information
RalfJung authored May 30, 2020
2 parents 35db819 + 406852a commit 93d45a0
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 13 deletions.
10 changes: 1 addition & 9 deletions src/libcore/iter/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,15 +619,7 @@ impl<A: Step> Iterator for ops::RangeFrom<A> {

#[inline]
fn nth(&mut self, n: usize) -> Option<A> {
// If we would jump over the maximum value, panic immediately.
// This is consistent with behavior before the Step redesign,
// even though it's inconsistent with n `next` calls.
// To get consistent behavior, change it to use `forward` instead.
// This change should go through FCP separately to the redesign, so is for now left as a
// FIXME: make this consistent
let plus_n =
Step::forward_checked(self.start.clone(), n).expect("overflow in RangeFrom::nth");
// The final step should always be debug-checked.
let plus_n = Step::forward(self.start.clone(), n);
self.start = Step::forward(plus_n.clone(), 1);
Some(plus_n)
}
Expand Down
14 changes: 10 additions & 4 deletions src/libcore/ops/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,16 @@ impl<Idx: PartialOrd<Idx>> Range<Idx> {
///
/// The `RangeFrom` `start..` contains all values with `x >= start`.
///
/// *Note*: Currently, no overflow checking is done for the [`Iterator`]
/// implementation; if you use an integer range and the integer overflows, it
/// might panic in debug mode or create an endless loop in release mode. **This
/// overflow behavior might change in the future.**
/// *Note*: Overflow in the [`Iterator`] implementation (when the contained
/// data type reaches its numerical limit) is allowed to panic, wrap, or
/// saturate. This behavior is defined by the implementation of the [`Step`]
/// trait. For primitive integers, this follows the normal rules, and respects
/// the overflow checks profile (panic in debug, wrap in release). Note also
/// that overflow happens earlier than you might assume: the overflow happens
/// in the call to `next` that yields the maximum value, as the range must be
/// set to a state to yield the next value.
///
/// [`Step`]: crate::iter::Step
///
/// # Examples
///
Expand Down

0 comments on commit 93d45a0

Please sign in to comment.