about summary refs log tree commit diff
diff options
context:
space:
mode:
authorRalf Jung <post@ralfj.de>2020-05-30 13:45:02 +0200
committerGitHub <noreply@github.com>2020-05-30 13:45:02 +0200
commit93d45a01e79cf80ed5e4ffa85cdaa096c440dff3 (patch)
tree44849ef9c57e43d4a2e96e650ebedb3f8e92a7a9
parent35db8196f93a717173a8e58b076dd7c6d845629f (diff)
parent406852ae0d92e5dfda890fa75ac522963065f903 (diff)
downloadrust-93d45a01e79cf80ed5e4ffa85cdaa096c440dff3.tar.gz
rust-93d45a01e79cf80ed5e4ffa85cdaa096c440dff3.zip
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.
-rw-r--r--src/libcore/iter/range.rs10
-rw-r--r--src/libcore/ops/range.rs14
2 files changed, 11 insertions, 13 deletions
diff --git a/src/libcore/iter/range.rs b/src/libcore/iter/range.rs
index 57e3e8084dd..bd7e6cfa5a7 100644
--- a/src/libcore/iter/range.rs
+++ b/src/libcore/iter/range.rs
@@ -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)
     }
diff --git a/src/libcore/ops/range.rs b/src/libcore/ops/range.rs
index d4e6048579a..d86f39c4550 100644
--- a/src/libcore/ops/range.rs
+++ b/src/libcore/ops/range.rs
@@ -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
 ///