wip unitary

src/internal/core.rs

@@ -39,6 +39,11 @@ pub struct State<DP: DependencyProvider> {
     #[allow(clippy::type_complexity)]
     merged_dependencies: Map<(DP::P, DP::P), SmallVec<IncompDpId<DP>>>,
 
+    /// All incompatibilities expressing basic facts,
+    /// with common dependents merged.
+    #[allow(clippy::type_complexity)]
+    merged_unitary_incompats: Map<DP::P, SmallVec<IncompDpId<DP>>>,
+
     /// Partial solution.
     /// TODO: remove pub.
     pub partial_solution: PartialSolution<DP>,
@@ -71,6 +76,7 @@ impl<DP: DependencyProvider> State<DP> {
             incompatibility_store,
             unit_propagation_buffer: SmallVec::Empty,
             merged_dependencies: Map::default(),
+            merged_unitary_incompats: Map::default(),
         }
     }
 
@@ -261,6 +267,26 @@ impl<DP: DependencyProvider> State<DP> {
     /// We could collapse them into { foo (1.0.0 ∪ 1.1.0), not bar ^1.0.0 }
     /// without having to check the existence of other versions though.
     fn merge_incompatibility(&mut self, mut id: IncompDpId<DP>) {
+        if let Some(p) = self.incompatibility_store[id].as_unitary() {
+            // If we are unitary, there's a good chance we can be merged with a previous unitary
+            let lookup = self.merged_unitary_incompats.entry(p.clone()).or_default();
+            if let Some((past, merged)) = lookup.as_mut_slice().iter_mut().find_map(|past| {
+                self.incompatibility_store[id]
+                    .merge_unitary(&self.incompatibility_store[*past])
+                    .map(|m| (past, m))
+            }) {
+                let p = p.clone();
+                let new = self.incompatibility_store.alloc(merged);
+                self.incompatibilities
+                    .entry(p)
+                    .or_default()
+                    .retain(|id| id != past);
+                *past = new;
+                id = new;
+            } else {
+                lookup.push(id);
+            }
+        }
         if let Some((p1, p2)) = self.incompatibility_store[id].as_dependency() {
             // If we are a dependency, there's a good chance we can be merged with a previous dependency
             let deps_lookup = self

src/internal/incompatibility.rs

@@ -116,7 +116,6 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
     }
 
     /// Create an incompatibility for a reason outside pubgrub.
-    #[allow(dead_code)] // Used by uv
     pub fn custom_term(package: P, term: Term<VS>, metadata: M) -> Self {
         let set = match &term {
             Term::Positive(r) => r.clone(),
@@ -200,6 +199,60 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
         ));
     }
 
+    pub fn as_unitary(&self) -> Option<&P> {
+        if self.package_terms.len() == 1 {
+            self.package_terms.iter().next().map(|(p, _)| p)
+        } else {
+            None
+        }
+    }
+
+    /// Merge unavailable versions of the same package.
+    ///
+    /// When multiple versions of a package are unavailable,
+    /// we can merge the two into a single incompatibility.
+    /// For example, if a@1 is unavailable and a@2 is unavailable, we can say instead
+    /// a@1||2 is unavailable.
+    ///
+    /// It is a special case of prior cause computation.
+    pub fn merge_unitary(&self, other: &Self) -> Option<Self> {
+        // It is almost certainly a bug to call this method without checking that self is a unitary
+        debug_assert!(self.as_unitary().is_some());
+        // Check that both incompatibilities are of the shape.
+        let p = self.as_unitary()?;
+        if p != other.as_unitary()? {
+            return None;
+        }
+        match (&self.kind, &other.kind) {
+            (Kind::FromDependencyOf(_, _, _, _), _) | (_, Kind::FromDependencyOf(_, _, _, _)) => {
+                None // Should be covered by merge_dependents
+            }
+            (Kind::DerivedFrom(_, _), _) | (_, Kind::DerivedFrom(_, _)) => {
+                None // The canonical case for prior_cause
+            }
+            (Kind::NotRoot(_, _), _) | (_, Kind::NotRoot(_, _)) => {
+                // There are never two rootes to be merged with each other,
+                // nor can a root emerged with anything else.
+                None
+            }
+            (Kind::NoVersions(_, _), Kind::NoVersions(_, _)) => Some(Self::no_versions(
+                p.clone(),
+                self.get(p).unwrap().union(other.get(p).unwrap()), // It is safe to `simplify` here
+            )),
+            (Kind::Custom(_, _, m1), Kind::Custom(_, _, m2)) => {
+                if m1 != m2 {
+                    return None;
+                }
+                Some(Self::custom_term(
+                    p.clone(),
+                    self.get(p).unwrap().union(other.get(p).unwrap()), // It is safe to `simplify` here
+                    m1.clone(),
+                ))
+            }
+            (_, _) => None, // When they're not the same kind we definitely cannot merge.
+        }
+    }
+
     /// Prior cause of two incompatibilities using the rule of resolution.
     pub fn prior_cause(
         incompat: Id<Self>,

tests/examples.rs

@@ -264,16 +264,16 @@ fn confusing_with_lots_of_holes_from_eq() {
     };
     assert_eq!(
         &DefaultStringReporter::report(&derivation_tree),
-        r#"Because there is no version of bar in 1 and foo 1 depends on bar 1, foo 1 is forbidden.
+        r#"Because there is no version of bar in 1 | 2 | 3 | 4 | 5 and foo 1 depends on bar 1, foo 1 is forbidden.
 And because there is no version of foo in <1 | >1, <2 | >2, <3 | >3, <4 | >4, <5 | >5, foo <2 | >2, <3 | >3, <4 | >4, <5 | >5 is forbidden. (1)
 
-Because there is no version of bar in 2 and foo 2 depends on bar 2, foo 2 is forbidden.
+Because there is no version of bar in 2 | 3 | 4 | 5 and foo 2 depends on bar 2, foo 2 is forbidden.
 And because foo <2 | >2, <3 | >3, <4 | >4, <5 | >5 is forbidden (1), foo <3 | >3, <4 | >4, <5 | >5 is forbidden. (2)
 
-Because there is no version of bar in 3 and foo 3 depends on bar 3, foo 3 is forbidden.
+Because there is no version of bar in 3 | 4 | 5 and foo 3 depends on bar 3, foo 3 is forbidden.
 And because foo <3 | >3, <4 | >4, <5 | >5 is forbidden (2), foo <4 | >4, <5 | >5 is forbidden. (3)
 
-Because there is no version of bar in 4 and foo 4 depends on bar 4, foo 4 is forbidden.
+Because there is no version of bar in 4 | 5 and foo 4 depends on bar 4, foo 4 is forbidden.
 And because foo <4 | >4, <5 | >5 is forbidden (3), foo <5 | >5 is forbidden. (4)
 
 Because there is no version of bar in 5 and foo 5 depends on bar 5, foo 5 is forbidden.
@@ -283,8 +283,8 @@ And because root 1 depends on foo, root 1 is forbidden."#
     derivation_tree.collapse_no_versions();
     assert_eq!(
         &DefaultStringReporter::report(&derivation_tree),
-        r#"Because foo <2 | >2, <3 | >3, <4 | >4, <5 | >5 depends on bar 1 and foo 2 depends on bar 2, foo <3 | >3, <4 | >4, <5 | >5 is forbidden.
-And because foo 3 depends on bar 3 and foo 4 depends on bar 4, foo <5 | >5 is forbidden.
+        r#"Because foo <2 | >2, <3 | >3, <4 | >4, <5 | >5 depends on bar 1 | 2 | 3 | 4 | 5 and foo 2 depends on bar 2 | 3 | 4 | 5, foo <3 | >3, <4 | >4, <5 | >5 is forbidden.
+And because foo 3 depends on bar 3 | 4 | 5 and foo 4 depends on bar 4 | 5, foo <5 | >5 is forbidden.
 And because foo 5 depends on bar 5 and root 1 depends on foo, root 1 is forbidden."#
     );
 }