Skip to content

Commit

Permalink
Fix empty gaps returned by Gaps iterator
Browse files Browse the repository at this point in the history
Fix empty gaps for `RangeMap`, and incorrect gaps returned
by `Gaps` iterator for `RangeInclusiveMap`.
  • Loading branch information
jeffparsons committed Jan 29, 2022
1 parent 1f7e72a commit dfdf5b0
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 10 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.)
Expand Down
61 changes: 55 additions & 6 deletions src/inclusive_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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<u32, bool> = 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.
Expand Down
47 changes: 43 additions & 4 deletions src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
Expand Down Expand Up @@ -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<u32, bool> = 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
///
Expand Down

0 comments on commit dfdf5b0

Please sign in to comment.