Remove problematic specialization from RangeInclusive

CAD97 · Mark-Simulacrum · commit 3e115b6c9dea · 2020-02-08T18:47:41.000-05:00
diff --git a/src/libcore/iter/range.rs b/src/libcore/iter/range.rs
@@ -341,16 +341,15 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
 
     #[inline]
     fn next(&mut self) -> Option<A> {
-        self.compute_is_empty();
-        if self.is_empty.unwrap_or_default() {
+        if self.is_empty() {
             return None;
         }
         let is_iterating = self.start < self.end;
-        self.is_empty = Some(!is_iterating);
         Some(if is_iterating {
             let n = self.start.add_one();
             mem::replace(&mut self.start, n)
         } else {
+            self.exhausted = true;
             self.start.clone()
         })
     }
@@ -369,8 +368,7 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
 
     #[inline]
     fn nth(&mut self, n: usize) -> Option<A> {
-        self.compute_is_empty();
-        if self.is_empty.unwrap_or_default() {
+        if self.is_empty() {
             return None;
         }
 
@@ -379,21 +377,20 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
 
             match plus_n.partial_cmp(&self.end) {
                 Some(Less) => {
-                    self.is_empty = Some(false);
                     self.start = plus_n.add_one();
                     return Some(plus_n);
                 }
                 Some(Equal) => {
-                    self.is_empty = Some(true);
                     self.start = plus_n.clone();
+                    self.exhausted = true;
                     return Some(plus_n);
                 }
                 _ => {}
             }
         }
 
         self.start = self.end.clone();
-        self.is_empty = Some(true);
+        self.exhausted = true;
         None
     }
 
@@ -404,8 +401,6 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
         F: FnMut(B, Self::Item) -> R,
         R: Try<Ok = B>,
     {
-        self.compute_is_empty();
-
         if self.is_empty() {
             return Try::from_ok(init);
         }
@@ -418,7 +413,7 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
             accum = f(accum, n)?;
         }
 
-        self.is_empty = Some(true);
+        self.exhausted = true;
 
         if self.start == self.end {
             accum = f(accum, self.start.clone())?;
@@ -447,24 +442,22 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
 impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> {
     #[inline]
     fn next_back(&mut self) -> Option<A> {
-        self.compute_is_empty();
-        if self.is_empty.unwrap_or_default() {
+        if self.is_empty() {
             return None;
         }
         let is_iterating = self.start < self.end;
-        self.is_empty = Some(!is_iterating);
         Some(if is_iterating {
             let n = self.end.sub_one();
             mem::replace(&mut self.end, n)
         } else {
+            self.exhausted = true;
             self.end.clone()
         })
     }
 
     #[inline]
     fn nth_back(&mut self, n: usize) -> Option<A> {
-        self.compute_is_empty();
-        if self.is_empty.unwrap_or_default() {
+        if self.is_empty() {
             return None;
         }
 
@@ -473,21 +466,20 @@ impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> {
 
             match minus_n.partial_cmp(&self.start) {
                 Some(Greater) => {
-                    self.is_empty = Some(false);
                     self.end = minus_n.sub_one();
                     return Some(minus_n);
                 }
                 Some(Equal) => {
-                    self.is_empty = Some(true);
                     self.end = minus_n.clone();
+                    self.exhausted = true;
                     return Some(minus_n);
                 }
                 _ => {}
             }
         }
 
         self.end = self.start.clone();
-        self.is_empty = Some(true);
+        self.exhausted = true;
         None
     }
 
@@ -498,8 +490,6 @@ impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> {
         F: FnMut(B, Self::Item) -> R,
         R: Try<Ok = B>,
     {
-        self.compute_is_empty();
-
         if self.is_empty() {
             return Try::from_ok(init);
         }
@@ -512,7 +502,7 @@ impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> {
             accum = f(accum, n)?;
         }
 
-        self.is_empty = Some(true);
+        self.exhausted = true;
 
         if self.start == self.end {
             accum = f(accum, self.start.clone())?;
diff --git a/src/libcore/ops/range.rs b/src/libcore/ops/range.rs
@@ -340,24 +340,21 @@ pub struct RangeInclusive<Idx> {
     // support that mode.
     pub(crate) start: Idx,
     pub(crate) end: Idx,
-    pub(crate) is_empty: Option<bool>,
+
     // This field is:
-    //  - `None` when next() or next_back() was never called
-    //  - `Some(false)` when `start < end`
-    //  - `Some(true)` when `end < start`
-    //  - `Some(false)` when `start == end` and the range hasn't yet completed iteration
-    //  - `Some(true)` when `start == end` and the range has completed iteration
-    // The field cannot be a simple `bool` because the `..=` constructor can
-    // accept non-PartialOrd types, also we want the constructor to be const.
+    //  - `false` upon construction
+    //  - `false` when iteration has yielded an element and the iterator is not exhausted
+    //  - `true` when iteration has been used to exhaust the iterator
+    //
+    // This is required to support PartialEq and Hash without a PartialOrd bound or specialization.
+    pub(crate) exhausted: bool,
 }
 
 #[stable(feature = "inclusive_range", since = "1.26.0")]
 impl<Idx: PartialEq> PartialEq for RangeInclusive<Idx> {
     #[inline]
     fn eq(&self, other: &Self) -> bool {
-        self.start == other.start
-            && self.end == other.end
-            && self.is_exhausted() == other.is_exhausted()
+        self.start == other.start && self.end == other.end && self.exhausted == other.exhausted
     }
 }
 
@@ -369,8 +366,7 @@ impl<Idx: Hash> Hash for RangeInclusive<Idx> {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.start.hash(state);
         self.end.hash(state);
-        // Ideally we would hash `is_exhausted` here as well, but there's no
-        // way for us to call it.
+        self.exhausted.hash(state);
     }
 }
 
@@ -389,7 +385,7 @@ impl<Idx> RangeInclusive<Idx> {
     #[rustc_promotable]
     #[rustc_const_stable(feature = "const_range_new", since = "1.32.0")]
     pub const fn new(start: Idx, end: Idx) -> Self {
-        Self { start, end, is_empty: None }
+        Self { start, end, exhausted: false }
     }
 
     /// Returns the lower bound of the range (inclusive).
@@ -465,18 +461,13 @@ impl<Idx: fmt::Debug> fmt::Debug for RangeInclusive<Idx> {
         self.start.fmt(fmt)?;
         write!(fmt, "..=")?;
         self.end.fmt(fmt)?;
+        if self.exhausted {
+            write!(fmt, " (exhausted)")?;
+        }
         Ok(())
     }
 }
 
-impl<Idx: PartialEq<Idx>> RangeInclusive<Idx> {
-    // Returns true if this is a range that started non-empty, and was iterated
-    // to exhaustion.
-    fn is_exhausted(&self) -> bool {
-        Some(true) == self.is_empty && self.start == self.end
-    }
-}
-
 impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> {
     /// Returns `true` if `item` is contained in the range.
     ///
@@ -544,15 +535,7 @@ impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> {
     #[unstable(feature = "range_is_empty", reason = "recently added", issue = "48111")]
     #[inline]
     pub fn is_empty(&self) -> bool {
-        self.is_empty.unwrap_or_else(|| !(self.start <= self.end))
-    }
-
-    // If this range's `is_empty` is field is unknown (`None`), update it to be a concrete value.
-    #[inline]
-    pub(crate) fn compute_is_empty(&mut self) {
-        if self.is_empty.is_none() {
-            self.is_empty = Some(!(self.start <= self.end));
-        }
+        self.exhausted || !(self.start <= self.end)
     }
 }
 
