Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small resolver cleanups #15040

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Jan 9, 2025

This is just too small resolver cleanups I found while doing some other work.

@rustbot
Copy link
Collaborator

rustbot commented Jan 9, 2025

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 9, 2025
// Well if we made it this far then we've got a valid dependency. We
// want this iterator to be inherently "peekable" so we don't
// necessarily return the item just yet. Instead we stash it away to
// get returned later, and if we replaced something then that was
// actually the candidate to try first so we return that.
if let Some(r) = mem::replace(&mut self.has_another, Some(b.clone())) {
if let Some(r) = self.has_another.replace(b.clone()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: unused import: `std::mem`
  --> src/cargo/core/resolver/mod.rs:62:5
   |
62 | use std::mem;
   |     ^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

warning: `cargo` (lib) generated 1 warning (run `cargo fix --lib -p cargo` to apply 1 suggestion)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what I get for trying to manually cherry pick.

src/cargo/core/resolver/mod.rs Show resolved Hide resolved
@@ -142,9 +142,7 @@ pub fn resolve(
let mut past_conflicting_activations = conflict_cache::ConflictCache::new();

let resolver_ctx = loop {
let resolver_ctx = ResolverContext::new();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this more, I wonder if this was a (failed) attempt to reuse memory. Perhaps let mut resolver_ctx = ResolverContext::new(); outside the loop and resolver_ctx.clear() inside the loop was the original intent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants