perf: more efficient relation_with

Author: Eh2406

src/internal/core.rs

@@ -179,8 +179,7 @@ impl<P: Package, V: Version> State<P, V> {
     fn find_shared_ids(&self, incompat: &Incompatibility<P, V>) -> Set<usize> {
         let mut all_ids = Set::new();
         let mut shared_ids = Set::new();
-        let mut stack = Vec::new();
-        stack.push(incompat);
+        let mut stack = vec![incompat];
         while let Some(i) = stack.pop() {
             if let Some((id1, id2)) = i.causes() {
                 if all_ids.contains(&i.id) {

src/internal/incompatibility.rs

@@ -8,10 +8,10 @@ use std::fmt;
 
 use crate::internal::small_map::SmallMap;
 use crate::package::Package;
-use crate::range::Range;
+use crate::range::{self, Range};
 use crate::report::{DefaultStringReporter, DerivationTree, Derived, External};
 use crate::solver::DependencyConstraints;
-use crate::term::{self, Term};
+use crate::term::Term;
 use crate::version::Version;
 
 /// An incompatibility is a set of terms for different packages
@@ -193,11 +193,11 @@ impl<P: Package, V: Version> Incompatibility<P, V> {
         let mut relation = Relation::Satisfied;
         for (package, incompat_term) in self.package_terms.iter() {
             match terms(package).map(|term| incompat_term.relation_with(&term)) {
-                Some(term::Relation::Satisfied) => {}
-                Some(term::Relation::Contradicted) => {
+                Some(range::Relation::Satisfied) => {}
+                Some(range::Relation::Contradicted) => {
                     return Relation::Contradicted((package.clone(), incompat_term.clone()));
                 }
-                None | Some(term::Relation::Inconclusive) => {
+                None | Some(range::Relation::Inconclusive) => {
                     // If a package is not present, the intersection is the same as [Term::any].
                     // According to the rules of satisfactions, the relation would be inconclusive.
                     // It could also be satisfied if the incompatibility term was also [Term::any],
@@ -324,7 +324,7 @@ pub mod tests {
 
             let i2 = Incompatibility {
                 id: 0,
-                package_terms: SmallMap::Two([("p2", t2.clone()), ("p3", t3.clone())]),
+                package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]),
                 kind: Kind::DerivedFrom(0,0)
             };
 

src/internal/small_vec.rs

@@ -38,7 +38,7 @@ impl<T> SmallVec<T> {
         }
     }
 
-    pub fn iter(&self) -> impl Iterator<Item = &T> {
+    pub fn iter(&self) -> impl DoubleEndedIterator<Item = &T> {
         self.as_slice().iter()
     }
 }

src/range.rs

@@ -237,6 +237,88 @@ impl<V: Version> Range<V> {
     }
 }
 
+/// Describe a relation between a set of terms S and another term t.
+///
+/// As a shorthand, we say that a term v
+/// satisfies or contradicts a term t if {v} satisfies or contradicts it.
+#[derive(Eq, PartialEq, Debug)]
+pub(crate) enum Relation {
+    /// We say that a set of terms S "satisfies" a term t
+    /// if t must be true whenever every term in S is true.
+    Satisfied,
+    /// Conversely, S "contradicts" t if t must be false
+    /// whenever every term in S is true.
+    Contradicted,
+    /// If neither of these is true we say that S is "inconclusive" for t.
+    Inconclusive,
+}
+
+// Set operations.
+impl<V: Version> Range<V> {
+    /// Compute the relation of two sets of versions.
+    pub(crate) fn relation(&self, other: &Self) -> Relation {
+        let mut state = None;
+        for s in self.segments.iter() {
+            match other.contains_interval(s) {
+                Relation::Satisfied => match state {
+                    None | Some(Relation::Satisfied) => state = Some(Relation::Satisfied),
+                    _ => return Relation::Inconclusive,
+                },
+                Relation::Inconclusive => return Relation::Inconclusive,
+                Relation::Contradicted => match state {
+                    None | Some(Relation::Contradicted) => state = Some(Relation::Contradicted),
+                    _ => return Relation::Inconclusive,
+                },
+            };
+        }
+        state.unwrap_or(Relation::Satisfied)
+    }
+
+    fn contains_interval(&self, other: &Interval<V>) -> Relation {
+        match other {
+            (o1, Some(o2)) => {
+                for seg in self.segments.iter() {
+                    match seg {
+                        (s1, Some(s2)) => {
+                            if o2 < s1 {
+                                break;
+                            }
+                            if s1 <= o1 && o2 <= s2 {
+                                return Relation::Satisfied;
+                            }
+                            if !(s1 >= o2 || o1 >= s2) {
+                                return Relation::Inconclusive;
+                            }
+                        }
+                        (s1, None) => {
+                            if s1 <= o1 {
+                                return Relation::Satisfied;
+                            }
+                            if s1 < o2 {
+                                return Relation::Inconclusive;
+                            }
+                        }
+                    }
+                }
+            }
+            (o1, None) => {
+                if let Some((s1, None)) = self.segments.iter().rev().next() {
+                    if s1 <= o1 {
+                        return Relation::Satisfied;
+                    }
+                }
+                if self.segments.iter().any(|seg| match seg {
+                    (_, Some(s2)) => o1 < s2,
+                    _ => true,
+                }) {
+                    return Relation::Inconclusive;
+                }
+            }
+        };
+        Relation::Contradicted
+    }
+}
+
 // Other useful functions.
 impl<V: Version> Range<V> {
     /// Check if a range contains a given version.
@@ -381,6 +463,53 @@ pub mod tests {
             assert_eq!(r1.intersection(&r2).contains(&version), r1.contains(&version) && r2.contains(&version));
         }
 
+        // Testing relation -----------------------------------
+
+        #[test]
+        fn relation_with_any_is_satisfied(range in strategy()) {
+            prop_assert_eq!(range.relation(&Range::any()), Relation::Satisfied);
+        }
+
+        #[test]
+        fn relation_with_none_is_contradicted(range in strategy()) {
+            prop_assert_eq!(range.relation(&Range::none()), if range == Range::none() {
+                Relation::Satisfied
+            } else {
+                Relation::Contradicted
+            });
+        }
+
+        #[test]
+        fn relation_with_self_is_satisfied(range in strategy()) {
+            prop_assert_eq!(range.relation(&range), Relation::Satisfied);
+        }
+
+        #[test]
+        fn relation_matchs_intersection(r1 in strategy(), r2 in strategy()) {
+            let full_intersection = r1.intersection(&r2);
+            let by_intersection = if full_intersection == r2 {
+                Relation::Satisfied
+            } else if full_intersection == Range::none() {
+                Relation::Contradicted
+            } else {
+                Relation::Inconclusive
+            };
+            prop_assert_eq!(by_intersection, r2.relation(&r1));
+        }
+
+        #[test]
+        fn relation_contains_both(r1 in strategy(), r2 in strategy(), version in version_strat()) {
+            match r1.relation(&r2) {
+                Relation::Satisfied => {
+                    prop_assert!(r2.contains(&version) || !r1.contains(&version));
+                }
+                Relation::Contradicted => {
+                    prop_assert!(!(r1.contains(&version) && r2.contains(&version)));
+                }
+                _ => {}
+            }
+        }
+
         // Testing union -----------------------------------
 
         #[test]

src/term.rs

@@ -3,7 +3,7 @@
 //! A term is the fundamental unit of operation of the PubGrub algorithm.
 //! It is a positive or negative expression regarding a set of versions.
 
-use crate::range::Range;
+use crate::range::{Range, Relation};
 use crate::version::Version;
 use std::fmt;
 
@@ -28,7 +28,7 @@ impl<V: Version> Term<V> {
     }
 
     /// A term that is never true.
-    pub(crate) fn empty() -> Self {
+    pub fn empty() -> Self {
         Self::Positive(Range::none())
     }
 
@@ -104,21 +104,6 @@ impl<V: Version> Term<V> {
     }
 }
 
-/// Describe a relation between a set of terms S and another term t.
-///
-/// As a shorthand, we say that a term v
-/// satisfies or contradicts a term t if {v} satisfies or contradicts it.
-pub(crate) enum Relation {
-    /// We say that a set of terms S "satisfies" a term t
-    /// if t must be true whenever every term in S is true.
-    Satisfied,
-    /// Conversely, S "contradicts" t if t must be false
-    /// whenever every term in S is true.
-    Contradicted,
-    /// If neither of these is true we say that S is "inconclusive" for t.
-    Inconclusive,
-}
-
 /// Relation between terms.
 impl<'a, V: 'a + Version> Term<V> {
     /// Check if a set of terms satisfies this term.
@@ -148,14 +133,24 @@ impl<'a, V: 'a + Version> Term<V> {
 
     /// Check if a set of terms satisfies or contradicts a given term.
     /// Otherwise the relation is inconclusive.
-    pub(crate) fn relation_with(&self, other_terms_intersection: &Term<V>) -> Relation {
-        let full_intersection = self.intersection(other_terms_intersection.as_ref());
-        if &full_intersection == other_terms_intersection {
-            Relation::Satisfied
-        } else if full_intersection == Self::empty() {
-            Relation::Contradicted
-        } else {
-            Relation::Inconclusive
+    pub(crate) fn relation_with(&self, other: &Term<V>) -> Relation {
+        match (self, other) {
+            (Self::Positive(r1), Self::Positive(r2)) => r2.relation(r1),
+            (Self::Positive(r1), Self::Negative(r2)) => match r2.negate().relation(r1) {
+                Relation::Satisfied => {
+                    if r2 == &Range::any() {
+                        Relation::Contradicted
+                    } else {
+                        Relation::Inconclusive
+                    }
+                }
+                x => x,
+            },
+            (Self::Negative(r1), Self::Positive(r2)) => r2.relation(&r1.negate()),
+            (Self::Negative(r1), Self::Negative(r2)) => match r1.relation(r2) {
+                Relation::Contradicted => Relation::Inconclusive,
+                x => x,
+            },
         }
     }
 }