Skip to content

Commit 7105f8d

Browse files
authored
Return Id<DP::P> over ID<DP::P> (pubgrub-rs#295)
It's more efficient to return a u32 than a reference, and it avoids a borrow when returning it.
1 parent ea28e95 commit 7105f8d

File tree

3 files changed

+12
-10
lines changed

3 files changed

+12
-10
lines changed

src/internal/core.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ impl<DP: DependencyProvider> State<DP> {
275275
.map(|m| (past, m))
276276
}) {
277277
let new = self.incompatibility_store.alloc(merged);
278-
for (&pkg, _) in self.incompatibility_store[new].iter() {
278+
for (pkg, _) in self.incompatibility_store[new].iter() {
279279
self.incompatibilities
280280
.entry(pkg)
281281
.or_default()
@@ -287,7 +287,7 @@ impl<DP: DependencyProvider> State<DP> {
287287
deps_lookup.push(id);
288288
}
289289
}
290-
for (&pkg, term) in self.incompatibility_store[id].iter() {
290+
for (pkg, term) in self.incompatibility_store[id].iter() {
291291
if cfg!(debug_assertions) {
292292
assert_ne!(term, &crate::term::Term::any());
293293
}

src/internal/incompatibility.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,10 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
246246
}
247247

248248
/// Iterate over packages.
249-
pub(crate) fn iter(&self) -> impl Iterator<Item = (&Id<P>, &Term<VS>)> {
250-
self.package_terms.iter()
249+
pub(crate) fn iter(&self) -> impl Iterator<Item = (Id<P>, &Term<VS>)> {
250+
self.package_terms
251+
.iter()
252+
.map(|(package, term)| (*package, term))
251253
}
252254

253255
// Reporting ###############################################################
@@ -348,25 +350,25 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
348350
[] => "version solving failed".into(),
349351
// TODO: special case when that unique package is root.
350352
[(package, Term::Positive(range))] => {
351-
format!("{} {} is forbidden", package_store[**package], range)
353+
format!("{} {} is forbidden", package_store[*package], range)
352354
}
353355
[(package, Term::Negative(range))] => {
354-
format!("{} {} is mandatory", package_store[**package], range)
356+
format!("{} {} is mandatory", package_store[*package], range)
355357
}
356358
[(p_pos, Term::Positive(r_pos)), (p_neg, Term::Negative(r_neg))]
357359
| [(p_neg, Term::Negative(r_neg)), (p_pos, Term::Positive(r_pos))] => {
358360
External::<_, _, M>::FromDependencyOf(
359-
&package_store[**p_pos],
361+
&package_store[*p_pos],
360362
r_pos.clone(),
361-
&package_store[**p_neg],
363+
&package_store[*p_neg],
362364
r_neg.clone(),
363365
)
364366
.to_string()
365367
}
366368
slice => {
367369
let str_terms: Vec<_> = slice
368370
.iter()
369-
.map(|(p, t)| format!("{} {}", package_store[**p], t))
371+
.map(|(p, t)| format!("{} {}", package_store[*p], t))
370372
.collect();
371373
str_terms.join(", ") + " are incompatible"
372374
}

src/internal/partial_solution.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
457457
package_assignments: &FnvIndexMap<Id<DP::P>, PackageAssignments<DP::P, DP::VS, DP::M>>,
458458
) -> SatisfiedMap<DP::P, DP::VS, DP::M> {
459459
let mut satisfied = SmallMap::Empty;
460-
for (&package, incompat_term) in incompat.iter() {
460+
for (package, incompat_term) in incompat.iter() {
461461
let pa = package_assignments.get(&package).expect("Must exist");
462462
satisfied.insert(package, pa.satisfier(package, &incompat_term.negate()));
463463
}

0 commit comments

Comments
 (0)