diff --git a/CHANGELOG.md b/CHANGELOG.md index f8db87c..1c2445e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +### v1.0.1 (2022-01-29) + +- **Fixes**: + - Fix empty gaps returned by `Gaps` iterator for `RangeMap`, and incorrect gaps returned by `Gaps` iterator for `RangeInclusiveMap`. + ### v1.0.0 (2022-01-28) It's time. (No functional change.) diff --git a/src/inclusive_map.rs b/src/inclusive_map.rs index d4f9368..c268265 100644 --- a/src/inclusive_map.rs +++ b/src/inclusive_map.rs @@ -449,7 +449,8 @@ where if item.range.end() < outer_range.start() { // This range sits entirely before the start of // the outer range; just skip it. - let _ = keys.next(); + keys.next() + .expect("We just checked that there is a next element"); } else if item.range.start() <= outer_range.start() { // This range overlaps the start of the // outer range, so the first possible candidate @@ -465,7 +466,8 @@ where } else { candidate_start = StepFnsT::add_one(item.range.end()); } - let _ = keys.next(); + keys.next() + .expect("We just checked that there is a next element"); } else { // The rest of the items might contribute to gaps. break; @@ -695,13 +697,13 @@ where } // Figure out where this gap ends. - let (end, next_candidate_start) = if let Some(item) = self.keys.next() { - if item.range.start() <= self.outer_range.end() { + let (end, mut next_candidate_start) = if let Some(next_item) = self.keys.next() { + if next_item.range.start() <= self.outer_range.end() { // The gap goes up until just before the start of the next item, // and the next candidate starts after it. ( - StepFnsT::sub_one(item.range.start()), - StepFnsT::add_one(item.range.end()), + StepFnsT::sub_one(next_item.range.start()), + StepFnsT::add_one(next_item.range.end()), ) } else { // The item sits after the end of the outer range, @@ -726,6 +728,28 @@ where ) }; + if !self.done { + // Find the start of the next gap. + while let Some(next_item) = self.keys.peek() { + if *next_item.range.start() == next_candidate_start { + // There's another item at the start of our candidate range. + // Gaps can't have zero width, so skip to the end of this + // item and try again. + if next_item.range.start() == self.outer_range.end() { + // There are no more gaps; avoid trying to find successor + // so we don't overflow. + self.done = true; + } else { + next_candidate_start = StepFnsT::add_one(next_item.range.end()); + self.keys.next().expect("We just checked that this exists"); + } + } else { + // We found an item that actually has a gap before it. + break; + } + } + } + // Move the next candidate gap start past the end // of this gap, and yield the gap we found. let gap = self.candidate_start.clone()..=end; @@ -1321,6 +1345,31 @@ mod tests { assert_eq!(gaps.next(), None); } + #[test] + fn no_empty_gaps() { + // Make two ranges different values so they don't + // get coalesced. + let mut range_map: RangeInclusiveMap = RangeInclusiveMap::new(); + // 0 1 2 3 4 5 6 7 8 9 + // ◌ ◌ ◌ ◌ ◆-◆ ◌ ◌ ◌ ◌ + range_map.insert(4..=5, true); + // 0 1 2 3 4 5 6 7 8 9 + // ◌ ◌ ◆-◆ ◌ ◌ ◌ ◌ ◌ ◌ + range_map.insert(2..=3, false); + // 0 1 2 3 4 5 6 7 8 9 + // ◌ ●-------------● ◌ + let outer_range = 1..=8; + let mut gaps = range_map.gaps(&outer_range); + // Should yield gaps at start and end, but not between the + // two touching items. + assert_eq!(gaps.next(), Some(1..=1)); + assert_eq!(gaps.next(), Some(6..=8)); + assert_eq!(gaps.next(), None); + // Gaps iterator should be fused. + assert_eq!(gaps.next(), None); + assert_eq!(gaps.next(), None); + } + #[test] fn no_overflow_finding_gaps_at_key_domain_extremes() { // Items and outer range both at extremes. diff --git a/src/map.rs b/src/map.rs index 72eedd5..bfbd154 100644 --- a/src/map.rs +++ b/src/map.rs @@ -590,11 +590,11 @@ where } // Figure out where this gap ends. - let (end, next_candidate_start) = if let Some(item) = self.keys.next() { - if item.range.start < self.outer_range.end { + let (gap_end, mut next_candidate_start) = if let Some(next_item) = self.keys.next() { + if next_item.range.start < self.outer_range.end { // The gap goes up until the start of the next item, // and the next candidate starts after it. - (&item.range.start, &item.range.end) + (&next_item.range.start, &next_item.range.end) } else { // The item sits after the end of the outer range, // so this gap ends at the end of the outer range. @@ -608,9 +608,23 @@ where (&self.outer_range.end, &self.outer_range.end) }; + // Find the start of the next gap. + while let Some(next_item) = self.keys.peek() { + if next_item.range.start == *next_candidate_start { + // There's another item at the start of our candidate range. + // Gaps can't have zero width, so skip to the end of this + // item and try again. + next_candidate_start = &next_item.range.end; + self.keys.next().expect("We just checked that this exists"); + } else { + // We found an item that actually has a gap before it. + break; + } + } + // Move the next candidate gap start past the end // of this gap, and yield the gap we found. - let gap = self.candidate_start.clone()..end.clone(); + let gap = self.candidate_start.clone()..gap_end.clone(); self.candidate_start = next_candidate_start; Some(gap) } @@ -1180,6 +1194,31 @@ mod tests { assert_eq!(gaps.next(), None); } + #[test] + fn no_empty_gaps() { + // Make two ranges different values so they don't + // get coalesced. + let mut range_map: RangeMap = RangeMap::new(); + // 0 1 2 3 4 5 6 7 8 9 + // ◌ ◌ ◌ ◌ ●-◌ ◌ ◌ ◌ ◌ + range_map.insert(4..5, true); + // 0 1 2 3 4 5 6 7 8 9 + // ◌ ◌ ◌ ●-◌ ◌ ◌ ◌ ◌ ◌ + range_map.insert(3..4, false); + // 0 1 2 3 4 5 6 7 8 9 + // ◌ ◆-------------◇ ◌ + let outer_range = 1..8; + let mut gaps = range_map.gaps(&outer_range); + // Should yield gaps at start and end, but not between the + // two touching items. (4 is covered, so there should be no gap.) + assert_eq!(gaps.next(), Some(1..3)); + assert_eq!(gaps.next(), Some(5..8)); + assert_eq!(gaps.next(), None); + // Gaps iterator should be fused. + assert_eq!(gaps.next(), None); + assert_eq!(gaps.next(), None); + } + /// /// impl Debug ///