Merge in upstream & rename to vulkano-rangemap

CHANGELOG.md

@@ -1,3 +1,9 @@
+### (unreleased)
+
+- **Fixes**:
+    - Fix `Gaps` iterator for `RangeMap` yielding an empty gap for an empty outer range. Simplified gaps logic and expanded fuzz testing to better cover this and similar cases.
+
+
 ### v1.0.2 (2022-05-17)
 
 - **Fixes**:

fuzz/fuzz_targets/rangemap_gaps.rs

@@ -32,12 +32,13 @@ impl Arbitrary for Input {
     fn arbitrary(u: &mut Unstructured) -> arbitrary::Result<Self> {
         Ok(Self {
             ops: u.arbitrary()?,
-            // Larger margins than that are too
+            // Larger margins than these are too
             // far away from boundary conditions to be interesting.
             // ("Oh, the fools! If only they'd built it with 6,001 hulls." -- Philip J. Fry)
             //
             // NOTE: Not using `int_in_range` because of <https://github.com/rust-fuzz/arbitrary/issues/106>.
-            outer_range: *u.choose(&[0, 1, 2, 3])?..*u.choose(&[252, 253, 254, 255])?,
+            outer_range: *u.choose(&[0, 1, 2, 3, 100, 101, 102, 103])?
+                ..*u.choose(&[100, 101, 102, 103, 252, 253, 254, 255])?,
         })
     }
 }
@@ -66,10 +67,22 @@ fuzz_target!(|input: Input| {
             // Truncate anything straddling either edge.
             u8::max(start, outer_range.start)..u8::min(end, outer_range.end)
         })
+        .filter(|range| {
+            // Reject anything that is now empty after being truncated.
+            !range.is_empty()
+        })
         .collect();
+
     keys.extend(gaps.into_iter());
     keys.sort_by_key(|key| key.start);
 
+    if outer_range.is_empty() {
+        // There should be no gaps or keys returned if the outer range is empty,
+        // because empty ranges cover no values.
+        assert!(keys.is_empty());
+        return;
+    }
+
     // Gaps and keys combined should span whole outer range.
     assert_eq!(keys.first().unwrap().start, outer_range.start);
     assert_eq!(keys.last().unwrap().end, outer_range.end);

fuzz/fuzz_targets/rangemap_inclusive_gaps.rs

@@ -31,12 +31,13 @@ impl Arbitrary for Input {
     fn arbitrary(u: &mut Unstructured) -> arbitrary::Result<Self> {
         Ok(Self {
             ops: u.arbitrary()?,
-            // Larger margins than that are too
+            // Larger margins than these are too
             // far away from boundary conditions to be interesting.
             // ("Oh, the fools! If only they'd built it with 6,001 hulls." -- Philip J. Fry)
             //
             // NOTE: Not using `int_in_range` because of <https://github.com/rust-fuzz/arbitrary/issues/106>.
-            outer_range: *u.choose(&[0, 1, 2, 3])?..=*u.choose(&[252, 253, 254, 255])?,
+            outer_range: *u.choose(&[0, 1, 2, 3, 100, 101, 102, 103])?
+                ..=*u.choose(&[100, 101, 102, 103, 252, 253, 254, 255])?,
         })
     }
 }
@@ -66,10 +67,21 @@ fuzz_target!(|input: Input| {
             u8::max(*range.start(), *outer_range.start())
                 ..=u8::min(*range.end(), *outer_range.end())
         })
+        .filter(|range| {
+            // Reject anything that is now empty after being truncated.
+            !range.is_empty()
+        })
         .collect();
     keys.extend(gaps.into_iter());
     keys.sort_by_key(|key| *key.start());
 
+    if outer_range.is_empty() {
+        // There should be no gaps or keys returned if the outer range is empty,
+        // because empty ranges cover no values.
+        assert!(keys.is_empty());
+        return;
+    }
+
     // Gaps and keys combined should span whole outer range.
     assert_eq!(keys.first().unwrap().start(), outer_range.start());
     assert_eq!(keys.last().unwrap().end(), outer_range.end());

src/inclusive_map.rs

@@ -433,11 +433,10 @@ where
     ///
     /// The iterator element type is `RangeInclusive<K>`.
     pub fn gaps<'a>(&'a self, outer_range: &'a RangeInclusive<K>) -> Gaps<'a, K, V, StepFnsT> {
-        let keys = self.btm.keys().peekable();
         Gaps {
             done: false,
             outer_range,
-            keys,
+            keys: self.btm.keys(),
             // We'll start the candidate range at the start of the outer range
             // without checking what's there. Each time we yield an item,
             // we'll skip any ranges we find before the next gap.
@@ -630,9 +629,7 @@ pub struct Gaps<'a, K, V, StepFnsT> {
     /// All other things here are ignored if `done` is `true`.
     done: bool,
     outer_range: &'a RangeInclusive<K>,
-    keys: core::iter::Peekable<
-        alloc::collections::btree_map::Keys<'a, RangeInclusiveStartWrapper<K>, V>,
-    >,
+    keys: alloc::collections::btree_map::Keys<'a, RangeInclusiveStartWrapper<K>, V>,
     candidate_start: K,
     _phantom: PhantomData<StepFnsT>,
 }
@@ -653,62 +650,47 @@ where
     type Item = RangeInclusive<K>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        if self.done || self.candidate_start > *self.outer_range.end() {
+        if self.done {
             // We've already passed the end of the outer range;
             // there are no more gaps to find.
             return None;
         }
 
-        // Skip past any ranges to find the first value that might
-        // be the start of a gap.
-        while let Some(item) = self.keys.peek() {
-            if *item.range.start() <= self.candidate_start {
-                // Next potential gap starts after this range, and at least as
-                // late as the start of the outer range.
-                self.candidate_start = if item.range.end() < self.outer_range.start() {
-                    self.outer_range.start().clone()
-                } else if item.range.end() >= self.outer_range.end() {
-                    // We're done; there no more room for gaps.
+        for item in &mut self.keys {
+            let range = &item.range;
+            if *range.end() < self.candidate_start {
+                // We're already completely past it; ignore it.
+            } else if *range.start() <= self.candidate_start {
+                // We're inside it; move past it.
+                if *range.end() >= *self.outer_range.end() {
+                    // Special case: it goes all the way to the end so we
+                    // can't safely skip past it. (Might overflow.)
                     self.done = true;
                     return None;
-                } else {
-                    StepFnsT::add_one(item.range.end())
-                };
-                let _ = self.keys.next();
-            } else {
-                // There's a gap before this item!
-                let end = if item.range.start() <= self.outer_range.end() {
-                    // It starts within the outer range; return the gap directly.
-                    StepFnsT::sub_one(item.range.start())
-                } else {
-                    // Next item starts beyond the end of the outer range;
-                    // end our gap at the end of the outer range.
-                    self.outer_range.end().clone()
-                };
-                let gap = self.candidate_start.clone()..=end;
-                // Next potential gap starts after this range.
-                if gap.end() < self.outer_range.end() {
-                    // We can safely add one; there's at least one more element in the domain.
-                    self.candidate_start = StepFnsT::add_one(gap.end());
-                } else {
-                    // The gap went right up to the end of the outer range;
-                    // there can be no more gaps.
+                }
+                self.candidate_start = StepFnsT::add_one(range.end());
+            } else if *range.start() <= *self.outer_range.end() {
+                // It starts before the end of the outer range,
+                // so move past it and then yield a gap.
+                let gap = self.candidate_start.clone()..=StepFnsT::sub_one(range.start());
+                if *range.end() >= *self.outer_range.end() {
+                    // Special case: it goes all the way to the end so we
+                    // can't safely skip past it. (Might overflow.)
                     self.done = true;
+                } else {
+                    self.candidate_start = StepFnsT::add_one(range.end());
                 }
                 return Some(gap);
             }
         }
 
-        // If we got this far, the only other gap that might be
-        // is at the end of the outer range.
+        // Now that we've run out of items, the only other possible
+        // gap is one at the end of the outer range.
+        self.done = true;
         if self.candidate_start <= *self.outer_range.end() {
             // There's a gap at the end!
-            let gap = self.candidate_start.clone()..=self.outer_range.end().clone();
-            // We're done; don't try to find any more gaps.
-            self.done = true;
-            Some(gap)
+            Some(self.candidate_start.clone()..=self.outer_range.end().clone())
         } else {
-            // We got to the end; there can't be any more gaps.
             None
         }
     }

src/map.rs

@@ -363,10 +363,9 @@ where
     ///
     /// The iterator element type is `Range<K>`.
     pub fn gaps<'a>(&'a self, outer_range: &'a Range<K>) -> Gaps<'a, K, V> {
-        let keys = self.btm.keys().peekable();
         Gaps {
             outer_range,
-            keys,
+            keys: self.btm.keys(),
             // We'll start the candidate range at the start of the outer range
             // without checking what's there. Each time we yield an item,
             // we'll skip any ranges we find before the next gap.
@@ -553,7 +552,7 @@ where
 /// [`gaps`]: RangeMap::gaps
 pub struct Gaps<'a, K, V> {
     outer_range: &'a Range<K>,
-    keys: core::iter::Peekable<alloc::collections::btree_map::Keys<'a, RangeStartWrapper<K>, V>>,
+    keys: alloc::collections::btree_map::Keys<'a, RangeStartWrapper<K>, V>,
     candidate_start: &'a K,
 }
 
@@ -567,36 +566,24 @@ where
     type Item = Range<K>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        // Skip past any ranges to find the first value that might
-        // be the start of a gap.
-        while let Some(item) = self.keys.peek() {
-            if item.range.start <= *self.candidate_start {
-                // Next potential gap starts after this range, and at least as
-                // late as the start of the outer range.
-                self.candidate_start = &item.range.end;
-                // Adjust the start to at least the start of the outer range.
-                if *self.candidate_start < self.outer_range.start {
-                    self.candidate_start = &self.outer_range.start;
-                }
-                let _ = self.keys.next();
-            } else {
-                // There's a gap before this item!
-                let gap = if item.range.start <= self.outer_range.end {
-                    // It starts within the outer range; return the gap directly.
-                    self.candidate_start.clone()..item.range.start.clone()
-                } else {
-                    // Next item starts beyond the end of the outer range;
-                    // end our gap at the end of the outer range.
-                    self.candidate_start.clone()..self.outer_range.end.clone()
-                };
-                // Next potential gap starts after this range.
-                self.candidate_start = &item.range.end;
+        for item in &mut self.keys {
+            let range = &item.range;
+            if range.end <= *self.candidate_start {
+                // We're already completely past it; ignore it.
+            } else if range.start <= *self.candidate_start {
+                // We're inside it; move past it.
+                self.candidate_start = &range.end;
+            } else if range.start < self.outer_range.end {
+                // It starts before the end of the outer range,
+                // so move past it and then yield a gap.
+                let gap = self.candidate_start.clone()..range.start.clone();
+                self.candidate_start = &range.end;
                 return Some(gap);
             }
         }
 
-        // If we got this far, the only other gap that might be
-        // is at the end of the outer range.
+        // Now that we've run out of items, the only other possible
+        // gap is at the end of the outer range.
         if *self.candidate_start < self.outer_range.end {
             // There's a gap at the end!
             let gap = self.candidate_start.clone()..self.outer_range.end.clone();
@@ -1131,10 +1118,9 @@ mod tests {
         // ◌ ◌ ◌ ◌ ◆ ◌ ◌ ◌ ◌ ◌
         let outer_range = 4..4;
         let mut gaps = range_map.gaps(&outer_range);
-        // Should yield a gap covering the zero-width outer range.
-        assert_eq!(gaps.next(), Some(4..4));
-        // Gaps iterator should be fused.
+        // Should not yield any gaps, because a zero-width outer range covers no values.
         assert_eq!(gaps.next(), None);
+        // Gaps iterator should be fused.
         assert_eq!(gaps.next(), None);
     }