refactor: replace Range with a bounded implementation (#112)

* refactor: replace Range with a bounded implementation * fix: rewrite range proptest strategy * fix: deserialize SmallVec without Vec alloc * fix: remove not_equals * fix: re-add union and remove early out * fix: renamed V to VS in bench * refactor: simpler intersection Co-authored-by: Jacob Finkelman <[email protected]> * test: use deltas for range strategy Co-authored-by: Jacob Finkelman <[email protected]> * docs(range): added comment about discrete values * More restrictive for valid random range generation * Remove duplicate check in check_invariants * Add warning about non-unique ranges representations * Rename start_bounded into start_unbounded Co-authored-by: Jacob Finkelman <[email protected]> Co-authored-by: Matthieu Pizenberg <[email protected]>
pubgrub-rs · Jun 25, 2022 · 15ac3c7 · 15ac3c7
1 parent f9c8238
commit 15ac3c7
Show file tree

Hide file tree

Showing 12 changed files with 529 additions and 345 deletions.
diff --git a/benches/large_case.rs b/benches/large_case.rs
@@ -5,16 +5,19 @@ extern crate criterion;
 use self::criterion::*;
 
 use pubgrub::package::Package;
+use pubgrub::range::Range;
 use pubgrub::solver::{resolve, OfflineDependencyProvider};
-use pubgrub::version::{NumberVersion, SemanticVersion, Version};
+use pubgrub::version::{NumberVersion, SemanticVersion};
+use pubgrub::version_set::VersionSet;
 use serde::de::Deserialize;
-use std::hash::Hash;
 
-fn bench<'a, P: Package + Deserialize<'a>, V: Version + Hash + Deserialize<'a>>(
+fn bench<'a, P: Package + Deserialize<'a>, VS: VersionSet + Deserialize<'a>>(
     b: &mut Bencher,
     case: &'a str,
-) {
-    let dependency_provider: OfflineDependencyProvider<P, V> = ron::de::from_str(&case).unwrap();
+) where
+    <VS as VersionSet>::V: Deserialize<'a>,
+{
+    let dependency_provider: OfflineDependencyProvider<P, VS> = ron::de::from_str(&case).unwrap();
 
     b.iter(|| {
         for p in dependency_provider.packages() {
@@ -35,11 +38,11 @@ fn bench_nested(c: &mut Criterion) {
         let data = std::fs::read_to_string(&case).unwrap();
         if name.ends_with("u16_NumberVersion.ron") {
             group.bench_function(name, |b| {
-                bench::<u16, NumberVersion>(b, &data);
+                bench::<u16, Range<NumberVersion>>(b, &data);
             });
         } else if name.ends_with("str_SemanticVersion.ron") {
             group.bench_function(name, |b| {
-                bench::<&str, SemanticVersion>(b, &data);
+                bench::<&str, Range<SemanticVersion>>(b, &data);
             });
         }
     }

diff --git a/examples/doc_interface.rs b/examples/doc_interface.rs
@@ -14,10 +14,10 @@ type NumVS = Range<NumberVersion>;
 fn main() {
     let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new();
     dependency_provider.add_dependencies(
-        "root", 1, [("menu", Range::any()), ("icons", Range::any())],
+        "root", 1, [("menu", Range::full()), ("icons", Range::full())],
     );
-    dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::any())]);
-    dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::any())]);
+    dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]);
+    dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]);
     dependency_provider.add_dependencies("icons", 1, []);
 
     // Run the algorithm.

diff --git a/examples/doc_interface_error.rs b/examples/doc_interface_error.rs
@@ -20,9 +20,9 @@ fn main() {
     let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new();
     // Direct dependencies: menu and icons.
     dependency_provider.add_dependencies("root", (1, 0, 0), [
-        ("menu", Range::any()),
-        ("icons", Range::exact((1, 0, 0))),
-        ("intl", Range::exact((5, 0, 0))),
+        ("menu", Range::full()),
+        ("icons", Range::singleton((1, 0, 0))),
+        ("intl", Range::singleton((5, 0, 0))),
     ]);
 
     // Dependencies of the menu lib.
@@ -47,19 +47,19 @@ fn main() {
 
     // Dependencies of the dropdown lib.
     dependency_provider.add_dependencies("dropdown", (1, 8, 0), [
-        ("intl", Range::exact((3, 0, 0))),
+        ("intl", Range::singleton((3, 0, 0))),
     ]);
     dependency_provider.add_dependencies("dropdown", (2, 0, 0), [
-        ("icons", Range::exact((2, 0, 0))),
+        ("icons", Range::singleton((2, 0, 0))),
     ]);
     dependency_provider.add_dependencies("dropdown", (2, 1, 0), [
-        ("icons", Range::exact((2, 0, 0))),
+        ("icons", Range::singleton((2, 0, 0))),
     ]);
     dependency_provider.add_dependencies("dropdown", (2, 2, 0), [
-        ("icons", Range::exact((2, 0, 0))),
+        ("icons", Range::singleton((2, 0, 0))),
     ]);
     dependency_provider.add_dependencies("dropdown", (2, 3, 0), [
-        ("icons", Range::exact((2, 0, 0))),
+        ("icons", Range::singleton((2, 0, 0))),
     ]);
 
     // Icons have no dependencies.

diff --git a/examples/doc_interface_semantic.rs b/examples/doc_interface_semantic.rs
@@ -19,8 +19,8 @@ fn main() {
     let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new();
     // Direct dependencies: menu and icons.
     dependency_provider.add_dependencies("root", (1, 0, 0), [
-        ("menu", Range::any()),
-        ("icons", Range::exact((1, 0, 0))),
+        ("menu", Range::full()),
+        ("icons", Range::singleton((1, 0, 0))),
     ]);
 
     // Dependencies of the menu lib.
@@ -46,16 +46,16 @@ fn main() {
     // Dependencies of the dropdown lib.
     dependency_provider.add_dependencies("dropdown", (1, 8, 0), []);
     dependency_provider.add_dependencies("dropdown", (2, 0, 0), [
-        ("icons", Range::exact((2, 0, 0))),
+        ("icons", Range::singleton((2, 0, 0))),
     ]);
     dependency_provider.add_dependencies("dropdown", (2, 1, 0), [
-        ("icons", Range::exact((2, 0, 0))),
+        ("icons", Range::singleton((2, 0, 0))),
     ]);
     dependency_provider.add_dependencies("dropdown", (2, 2, 0), [
-        ("icons", Range::exact((2, 0, 0))),
+        ("icons", Range::singleton((2, 0, 0))),
     ]);
     dependency_provider.add_dependencies("dropdown", (2, 3, 0), [
-        ("icons", Range::exact((2, 0, 0))),
+        ("icons", Range::singleton((2, 0, 0))),
     ]);
 
     // Icons has no dependency.

diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs
@@ -276,12 +276,12 @@ pub mod tests {
             let mut store = Arena::new();
             let i1 = store.alloc(Incompatibility {
                 package_terms: SmallMap::Two([("p1", t1.clone()), ("p2", t2.negate())]),
-                kind: Kind::UnavailableDependencies("0", Range::any())
+                kind: Kind::UnavailableDependencies("0", Range::full())
             });
 
             let i2 = store.alloc(Incompatibility {
                 package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]),
-                kind: Kind::UnavailableDependencies("0", Range::any())
+                kind: Kind::UnavailableDependencies("0", Range::full())
             });
 
             let mut i3 = Map::default();

diff --git a/src/internal/small_vec.rs b/src/internal/small_vec.rs
@@ -1,4 +1,5 @@
 use std::fmt;
+use std::hash::{Hash, Hasher};
 use std::ops::Deref;
 
 #[derive(Clone)]
@@ -108,6 +109,13 @@ impl<T: fmt::Debug> fmt::Debug for SmallVec<T> {
     }
 }
 
+impl<T: Hash> Hash for SmallVec<T> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.len().hash(state);
+        Hash::hash_slice(self.as_slice(), state);
+    }
+}
+
 #[cfg(feature = "serde")]
 impl<T: serde::Serialize> serde::Serialize for SmallVec<T> {
     fn serialize<S: serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
@@ -118,13 +126,70 @@ impl<T: serde::Serialize> serde::Serialize for SmallVec<T> {
 #[cfg(feature = "serde")]
 impl<'de, T: serde::Deserialize<'de>> serde::Deserialize<'de> for SmallVec<T> {
     fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
-        let items: Vec<T> = serde::Deserialize::deserialize(d)?;
+        struct SmallVecVisitor<T> {
+            marker: std::marker::PhantomData<T>,
+        }
+
+        impl<'de, T> serde::de::Visitor<'de> for SmallVecVisitor<T>
+        where
+            T: serde::Deserialize<'de>,
+        {
+            type Value = SmallVec<T>;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+                formatter.write_str("a sequence")
+            }
+
+            fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
+            where
+                A: serde::de::SeqAccess<'de>,
+            {
+                let mut values = SmallVec::empty();
+                while let Some(value) = seq.next_element()? {
+                    values.push(value);
+                }
+                Ok(values)
+            }
+        }
+
+        let visitor = SmallVecVisitor {
+            marker: Default::default(),
+        };
+        d.deserialize_seq(visitor)
+    }
+}
+
+impl<T> IntoIterator for SmallVec<T> {
+    type Item = T;
+    type IntoIter = SmallVecIntoIter<T>;
 
-        let mut v = Self::empty();
-        for item in items {
-            v.push(item);
+    fn into_iter(self) -> Self::IntoIter {
+        match self {
+            SmallVec::Empty => SmallVecIntoIter::Empty,
+            SmallVec::One(a) => SmallVecIntoIter::One(IntoIterator::into_iter(a)),
+            SmallVec::Two(a) => SmallVecIntoIter::Two(IntoIterator::into_iter(a)),
+            SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(IntoIterator::into_iter(v)),
+        }
+    }
+}
+
+pub enum SmallVecIntoIter<T> {
+    Empty,
+    One(<[T; 1] as IntoIterator>::IntoIter),
+    Two(<[T; 2] as IntoIterator>::IntoIter),
+    Flexible(<Vec<T> as IntoIterator>::IntoIter),
+}
+
+impl<T> Iterator for SmallVecIntoIter<T> {
+    type Item = T;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        match self {
+            SmallVecIntoIter::Empty => None,
+            SmallVecIntoIter::One(it) => it.next(),
+            SmallVecIntoIter::Two(it) => it.next(),
+            SmallVecIntoIter::Flexible(it) => it.next(),
         }
-        Ok(v)
     }
 }
 

diff --git a/src/lib.rs b/src/lib.rs
@@ -55,10 +55,10 @@
 //! let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new();
 //!
 //! dependency_provider.add_dependencies(
-//!     "root", 1, [("menu", Range::any()), ("icons", Range::any())],
+//!     "root", 1, [("menu", Range::full()), ("icons", Range::full())],
 //! );
-//! dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::any())]);
-//! dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::any())]);
+//! dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]);
+//! dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]);
 //! dependency_provider.add_dependencies("icons", 1, []);
 //!
 //! // Run the algorithm.