Skip to content

Commit 2b2d8d4

Browse files
Eh2406konstinzanieb
authored
feat: add a simplify for error messages (#156)
* feat: add a `simplify` for error messages * Fix broken links Co-authored-by: konsti <[email protected]> * Inline locate_versions, to simplify lifetimes While attempting to use this simplification code I got an odd lifetime error with ``` let c = set.complement(); let s = c.simplify(versions); s.complement() ``` By in lining locate_versions the lifetimes could be simplified so that that code works * correct capitalization Co-authored-by: Zanie Blue <[email protected]> * improve comment Co-authored-by: Zanie Blue <[email protected]> --------- Co-authored-by: konsti <[email protected]> Co-authored-by: Zanie Blue <[email protected]>
1 parent acfbe99 commit 2b2d8d4

File tree

1 file changed

+152
-12
lines changed

1 file changed

+152
-12
lines changed

src/range.rs

+152-12
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
//! If we do not see practical bugs, or we get a formal proof that the code cannot lead to error states, then we may remove this warning.
5252
5353
use crate::{internal::small_vec::SmallVec, version_set::VersionSet};
54+
use std::cmp::Ordering;
5455
use std::ops::RangeBounds;
5556
use std::{
5657
fmt::{Debug, Display, Formatter},
@@ -202,23 +203,37 @@ impl<V: Ord> Range<V> {
202203
/// Returns true if the this Range contains the specified value.
203204
pub fn contains(&self, v: &V) -> bool {
204205
for segment in self.segments.iter() {
205-
if match segment {
206-
(Unbounded, Unbounded) => true,
207-
(Unbounded, Included(end)) => v <= end,
208-
(Unbounded, Excluded(end)) => v < end,
209-
(Included(start), Unbounded) => v >= start,
210-
(Included(start), Included(end)) => v >= start && v <= end,
211-
(Included(start), Excluded(end)) => v >= start && v < end,
212-
(Excluded(start), Unbounded) => v > start,
213-
(Excluded(start), Included(end)) => v > start && v <= end,
214-
(Excluded(start), Excluded(end)) => v > start && v < end,
215-
} {
216-
return true;
206+
match within_bounds(v, segment) {
207+
Ordering::Less => return false,
208+
Ordering::Equal => return true,
209+
Ordering::Greater => (),
217210
}
218211
}
219212
false
220213
}
221214

215+
/// Returns true if the this Range contains the specified values.
216+
///
217+
/// The `versions` iterator must be sorted.
218+
/// Functionally equivalent to `versions.map(|v| self.contains(v))`.
219+
/// Except it runs in `O(size_of_range + len_of_versions)` not `O(size_of_range * len_of_versions)`
220+
pub fn contains_many<'s, I>(&'s self, versions: I) -> impl Iterator<Item = bool> + 's
221+
where
222+
I: Iterator<Item = &'s V> + 's,
223+
V: 's,
224+
{
225+
versions.scan(0, move |i, v| {
226+
while let Some(segment) = self.segments.get(*i) {
227+
match within_bounds(v, segment) {
228+
Ordering::Less => return Some(false),
229+
Ordering::Equal => return Some(true),
230+
Ordering::Greater => *i += 1,
231+
}
232+
}
233+
Some(false)
234+
})
235+
}
236+
222237
/// Construct a simple range from anything that impls [RangeBounds] like `v1..v2`.
223238
pub fn from_range_bounds<R, IV>(bounds: R) -> Self
224239
where
@@ -264,6 +279,26 @@ impl<V: Ord> Range<V> {
264279
}
265280
}
266281

282+
fn within_bounds<V: PartialOrd>(v: &V, segment: &Interval<V>) -> Ordering {
283+
let below_lower_bound = match segment {
284+
(Excluded(start), _) => v <= start,
285+
(Included(start), _) => v < start,
286+
(Unbounded, _) => false,
287+
};
288+
if below_lower_bound {
289+
return Ordering::Less;
290+
}
291+
let below_upper_bound = match segment {
292+
(_, Unbounded) => true,
293+
(_, Included(end)) => v <= end,
294+
(_, Excluded(end)) => v < end,
295+
};
296+
if below_upper_bound {
297+
return Ordering::Equal;
298+
}
299+
Ordering::Greater
300+
}
301+
267302
fn valid_segment<T: PartialOrd>(start: &Bound<T>, end: &Bound<T>) -> bool {
268303
match (start, end) {
269304
(Included(s), Included(e)) => s <= e,
@@ -274,6 +309,36 @@ fn valid_segment<T: PartialOrd>(start: &Bound<T>, end: &Bound<T>) -> bool {
274309
}
275310
}
276311

312+
/// Group adjacent versions locations.
313+
///
314+
/// ```text
315+
/// [None, 3, 6, 7, None] -> [(3, 7)]
316+
/// [3, 6, 7, None] -> [(None, 7)]
317+
/// [3, 6, 7] -> [(None, None)]
318+
/// [None, 1, 4, 7, None, None, None, 8, None, 9] -> [(1, 7), (8, 8), (9, None)]
319+
/// ```
320+
fn group_adjacent_locations(
321+
mut locations: impl Iterator<Item = Option<usize>>,
322+
) -> impl Iterator<Item = (Option<usize>, Option<usize>)> {
323+
// If the first version matched, then the lower bound of that segment is not needed
324+
let mut seg = locations.next().flatten().map(|ver| (None, Some(ver)));
325+
std::iter::from_fn(move || {
326+
for ver in locations.by_ref() {
327+
if let Some(ver) = ver {
328+
// As long as were still matching versions, we keep merging into the currently matching segment
329+
seg = Some((seg.map_or(Some(ver), |(s, _)| s), Some(ver)));
330+
} else {
331+
// If we have found a version that doesn't match, then right the merge segment and prepare for a new one.
332+
if seg.is_some() {
333+
return seg.take();
334+
}
335+
}
336+
}
337+
// If the last version matched, then write out the merged segment but the upper bound is not needed.
338+
seg.take().map(|(s, _)| (s, None))
339+
})
340+
}
341+
277342
impl<V: Ord + Clone> Range<V> {
278343
/// Computes the union of this `Range` and another.
279344
pub fn union(&self, other: &Self) -> Self {
@@ -321,6 +386,54 @@ impl<V: Ord + Clone> Range<V> {
321386

322387
Self { segments }.check_invariants()
323388
}
389+
390+
/// Returns a simpler Range that contains the same versions
391+
///
392+
/// For every one of the Versions provided in versions the existing range and
393+
/// the simplified range will agree on whether it is contained.
394+
/// The simplified version may include or exclude versions that are not in versions as the implementation wishes.
395+
/// For example:
396+
/// - If all the versions are contained in the original than the range will be simplified to `full`.
397+
/// - If none of the versions are contained in the original than the range will be simplified to `empty`.
398+
///
399+
/// If versions are not sorted the correctness of this function is not guaranteed.
400+
pub fn simplify<'v, I>(&self, versions: I) -> Self
401+
where
402+
I: Iterator<Item = &'v V> + 'v,
403+
V: 'v,
404+
{
405+
// Return the segment index in the range for each version in the range, None otherwise
406+
let version_locations = versions.scan(0, move |i, v| {
407+
while let Some(segment) = self.segments.get(*i) {
408+
match within_bounds(v, segment) {
409+
Ordering::Less => return Some(None),
410+
Ordering::Equal => return Some(Some(*i)),
411+
Ordering::Greater => *i += 1,
412+
}
413+
}
414+
Some(None)
415+
});
416+
let kept_segments = group_adjacent_locations(version_locations);
417+
self.keep_segments(kept_segments)
418+
}
419+
420+
/// Create a new range with a subset of segments at given location bounds.
421+
///
422+
/// Each new segment is constructed from a pair of segments, taking the
423+
/// start of the first and the end of the second.
424+
fn keep_segments(
425+
&self,
426+
kept_segments: impl Iterator<Item = (Option<usize>, Option<usize>)>,
427+
) -> Range<V> {
428+
let mut segments = SmallVec::Empty;
429+
for (s, e) in kept_segments {
430+
segments.push((
431+
s.map_or(Unbounded, |s| self.segments[s].0.clone()),
432+
e.map_or(Unbounded, |e| self.segments[e].1.clone()),
433+
));
434+
}
435+
Self { segments }.check_invariants()
436+
}
324437
}
325438

326439
impl<T: Debug + Display + Clone + Eq + Ord> VersionSet for Range<T> {
@@ -600,5 +713,32 @@ pub mod tests {
600713
let rv2: Range<u32> = rv.bounding_range().map(Range::from_range_bounds::<_, u32>).unwrap_or_else(Range::empty);
601714
assert_eq!(rv, rv2);
602715
}
716+
717+
#[test]
718+
fn contains(range in strategy(), versions in proptest::collection::vec(version_strat(), ..30)) {
719+
for v in versions {
720+
assert_eq!(range.contains(&v), range.segments.iter().any(|s| RangeBounds::contains(s, &v)));
721+
}
722+
}
723+
724+
#[test]
725+
fn contains_many(range in strategy(), mut versions in proptest::collection::vec(version_strat(), ..30)) {
726+
versions.sort();
727+
assert_eq!(versions.len(), range.contains_many(versions.iter()).count());
728+
for (a, b) in versions.iter().zip(range.contains_many(versions.iter())) {
729+
assert_eq!(range.contains(a), b);
730+
}
731+
}
732+
733+
#[test]
734+
fn simplify(range in strategy(), mut versions in proptest::collection::vec(version_strat(), ..30)) {
735+
versions.sort();
736+
let simp = range.simplify(versions.iter());
737+
738+
for v in versions {
739+
assert_eq!(range.contains(&v), simp.contains(&v));
740+
}
741+
assert!(simp.segments.len() <= range.segments.len())
742+
}
603743
}
604744
}

0 commit comments

Comments
 (0)