From 5a7d59f829b2024b1b2d39662dfb0d7cb069fa58 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Wed, 1 May 2024 21:43:52 +0000 Subject: [PATCH] wip unitary --- src/internal/core.rs | 26 ++++++++++++++++ src/internal/incompatibility.rs | 55 ++++++++++++++++++++++++++++++++- tests/examples.rs | 12 +++---- 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/internal/core.rs b/src/internal/core.rs index fe117ca6..c39b9051 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -39,6 +39,11 @@ pub struct State { #[allow(clippy::type_complexity)] merged_dependencies: Map<(DP::P, DP::P), SmallVec>>, + /// All incompatibilities expressing basic facts, + /// with common dependents merged. + #[allow(clippy::type_complexity)] + merged_unitary_incompats: Map>>, + /// Partial solution. /// TODO: remove pub. pub partial_solution: PartialSolution, @@ -71,6 +76,7 @@ impl State { incompatibility_store, unit_propagation_buffer: SmallVec::Empty, merged_dependencies: Map::default(), + merged_unitary_incompats: Map::default(), } } @@ -261,6 +267,26 @@ impl State { /// 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) { + 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 diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index 168c6809..40e9c336 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -116,7 +116,6 @@ impl Incompatibilit } /// Create an incompatibility for a reason outside pubgrub. - #[allow(dead_code)] // Used by uv pub fn custom_term(package: P, term: Term, metadata: M) -> Self { let set = match &term { Term::Positive(r) => r.clone(), @@ -200,6 +199,60 @@ impl 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 { + // 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, diff --git a/tests/examples.rs b/tests/examples.rs index 0ea09ea3..9d75e346 100644 --- a/tests/examples.rs +++ b/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."# ); }