Return Id<DP::P> over ID<DP::P>

It's more efficient to return a u32 than a reference, and it avoids a borrow when returning it.
pubgrub-rs · Dec 11, 2024 · 1a0c3ff · 1a0c3ff
1 parent 3741e3b
commit 1a0c3ff
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 10 deletions.
diff --git a/src/internal/core.rs b/src/internal/core.rs
@@ -275,7 +275,7 @@ impl<DP: DependencyProvider> State<DP> {
                     .map(|m| (past, m))
             }) {
                 let new = self.incompatibility_store.alloc(merged);
-                for (&pkg, _) in self.incompatibility_store[new].iter() {
+                for (pkg, _) in self.incompatibility_store[new].iter() {
                     self.incompatibilities
                         .entry(pkg)
                         .or_default()
@@ -287,7 +287,7 @@ impl<DP: DependencyProvider> State<DP> {
                 deps_lookup.push(id);
             }
         }
-        for (&pkg, term) in self.incompatibility_store[id].iter() {
+        for (pkg, term) in self.incompatibility_store[id].iter() {
             if cfg!(debug_assertions) {
                 assert_ne!(term, &crate::term::Term::any());
             }

diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs
@@ -12,6 +12,11 @@ use crate::{
     VersionSet,
 };
 
+/// An entry in an incompatibility when iterating over one.
+///
+/// Split out to reduce type complexity.
+pub(crate) type IncompatIterItem<'term, P, VS> = (Id<P>, &'term Term<VS>);
+
 /// An incompatibility is a set of terms for different packages
 /// that should never be satisfied all together.
 /// An incompatibility usually originates from a package dependency.
@@ -246,8 +251,10 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
     }
 
     /// Iterate over packages.
-    pub(crate) fn iter(&self) -> impl Iterator<Item = (&Id<P>, &Term<VS>)> {
-        self.package_terms.iter()
+    pub(crate) fn iter(&self) -> impl Iterator<Item = IncompatIterItem<P, VS>> {
+        self.package_terms
+            .iter()
+            .map(|(package, term)| (*package, term))
     }
 
     // Reporting ###############################################################
@@ -348,25 +355,25 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
             [] => "version solving failed".into(),
             // TODO: special case when that unique package is root.
             [(package, Term::Positive(range))] => {
-                format!("{} {} is forbidden", package_store[**package], range)
+                format!("{} {} is forbidden", package_store[*package], range)
             }
             [(package, Term::Negative(range))] => {
-                format!("{} {} is mandatory", package_store[**package], range)
+                format!("{} {} is mandatory", package_store[*package], range)
             }
             [(p_pos, Term::Positive(r_pos)), (p_neg, Term::Negative(r_neg))]
             | [(p_neg, Term::Negative(r_neg)), (p_pos, Term::Positive(r_pos))] => {
                 External::<_, _, M>::FromDependencyOf(
-                    &package_store[**p_pos],
+                    &package_store[*p_pos],
                     r_pos.clone(),
-                    &package_store[**p_neg],
+                    &package_store[*p_neg],
                     r_neg.clone(),
                 )
                 .to_string()
             }
             slice => {
                 let str_terms: Vec<_> = slice
                     .iter()
-                    .map(|(p, t)| format!("{} {}", package_store[**p], t))
+                    .map(|(p, t)| format!("{} {}", package_store[*p], t))
                     .collect();
                 str_terms.join(", ") + " are incompatible"
             }

diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs
@@ -459,7 +459,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
         package_assignments: &FnvIndexMap<Id<DP::P>, PackageAssignments<DP::P, DP::VS, DP::M>>,
     ) -> SatisfiedMap<DP::P, DP::VS, DP::M> {
         let mut satisfied = SmallMap::Empty;
-        for (&package, incompat_term) in incompat.iter() {
+        for (package, incompat_term) in incompat.iter() {
             let pa = package_assignments.get(&package).expect("Must exist");
             satisfied.insert(package, pa.satisfier(package, &incompat_term.negate()));
         }