From e2146fcceec238c9bc50d63eff1825d80cd32512 Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 8 Nov 2024 10:25:04 +0100 Subject: [PATCH] Add `FromIter` for `Ranges` Add a method to construct ranges from an iterator of arbitrary segments. This allows to `.collect()` an iterator of tuples of bounds. This is more ergonomic than folding the previous ranges with the next segment each time, and also faster. Split out from https://github.com/pubgrub-rs/pubgrub/pull/273 Closes https://github.com/astral-sh/pubgrub/pull/33 Fixes https://github.com/pubgrub-rs/pubgrub/issues/249 --- version-ranges/src/lib.rs | 96 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/version-ranges/src/lib.rs b/version-ranges/src/lib.rs index 0e33bd5c..2c80fc57 100644 --- a/version-ranges/src/lib.rs +++ b/version-ranges/src/lib.rs @@ -826,6 +826,87 @@ impl Ranges { } } +impl FromIterator<(Bound, Bound)> for Ranges { + /// Constructor from arbitrary, unsorted and potentially overlapping ranges. + /// + /// This is equivalent, but faster, to computing the [`Ranges::union`] of the + /// [`Ranges::from_range_bounds`] of each segment. + fn from_iter, Bound)>>(iter: T) -> Self { + // We have three constraints we need to fulfil: + // 1. The segments are sorted, from lowest to highest (through `Ord`): By sorting. + // 2. Each segment contains at least one version (start < end): By `union`. + // 3. There is at least one version between two segments: By `union`. + let mut segments: SmallVec<[Interval; 1]> = SmallVec::new(); + + for segment in iter { + if !valid_segment(&segment.start_bound(), &segment.end_bound()) { + continue; + } + // Find where to insert the new segment + let insertion_point = segments.partition_point(|elem: &Interval| { + cmp_bounds_start(elem.start_bound(), segment.start_bound()) + .unwrap() + .is_lt() + }); + // Is it overlapping with the previous segment? + let previous_overlapping = insertion_point > 0 + && !end_before_start_with_gap( + &segments[insertion_point - 1].end_bound(), + &segment.start_bound(), + ); + + // Is it overlapping with the following segment? + let next_overlapping = insertion_point < segments.len() + && !end_before_start_with_gap( + &segment.end_bound(), + &segments[insertion_point].start_bound(), + ); + + match (previous_overlapping, next_overlapping) { + (true, true) => { + // previous: |-------| + // segment: |-------| + // following: |-----| + // + // final: |--------------------| + // We merge all three segments into one, which is effectively removing one of + // two previously inserted and changing the bounds on the other. + let following = segments.remove(insertion_point); + segments[insertion_point - 1].1 = following.1; + } + (true, false) => { + // previous: |-----| + // segment: |-----| + // following: |-----| + // + // final: |---------| |-----| + // We can reuse the existing element by extending it. + segments[insertion_point - 1].1 = segment.1; + } + (false, true) => { + // previous: |-----| + // segment: |-----| + // following: |-----| + // + // final: |-----| |---------| + // We can reuse the existing element by extending it. + segments[insertion_point].0 = segment.0; + } + (false, false) => { + // previous: |-----| + // segment: |-----| + // following: |-----| + // + // final: |-----| |-----| |-----| + segments.insert(insertion_point, segment); + } + } + } + + Self { segments }.check_invariants() + } +} + // REPORT ###################################################################### impl Display for Ranges { @@ -1130,6 +1211,12 @@ pub mod tests { } assert!(simp.segments.len() <= range.segments.len()) } + + #[test] + fn from_iter_valid(segments in proptest::collection::vec(any::<(Bound, Bound)>(), ..30)) { + // We check invariants in the method. + Ranges::from_iter(segments.clone()); + } } #[test] @@ -1166,6 +1253,15 @@ pub mod tests { ); } + #[test] + fn from_iter_regression() { + Ranges::from_iter([ + (Included(0), Included(0)), + (Excluded(1u32), Unbounded), + (Included(0), Included(0)), + ]); + } + #[test] fn version_ord() { let versions: &[Ranges] = &[