Rollup merge of #72368 - CAD97:rangeto, r=dtolnay

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 #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 #69659. Closes #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.
This commit is contained in:
Ralf Jung 2020-05-30 13:45:02 +02:00 committed by GitHub
commit 93d45a01e7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 13 deletions

View File

@ -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)
}

View File

@ -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
///