Skip to content

Commit

Permalink
Auto merge of #5132 - Eh2406:less_clones, r=alexcrichton
Browse files Browse the repository at this point in the history
don't clone a BacktrackFrame if we are not going to backtrack

I think this is a fix for #5130.

Currently the only recoverable `activate` error does not corrupt `cx`, so this is definitely ok. We should be careful as we add more recoverable `activate` errors that they don't corrupt `cx` to the point where the error messages are affected.
  • Loading branch information
bors committed Mar 6, 2018
2 parents ae796ad + 8304b17 commit c721937
Showing 1 changed file with 19 additions and 13 deletions.
32 changes: 19 additions & 13 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,15 +892,19 @@ fn activate_deps_loop(
// We have a candidate. Clone a `BacktrackFrame`
// so we can add it to the `backtrack_stack` if activation succeeds.
// We clone now in case `activate` changes `cx` and then fails.
let backtrack = BacktrackFrame {
cur,
context_backup: Context::clone(&cx),
deps_backup: <BinaryHeap<DepsFrame>>::clone(&remaining_deps),
remaining_candidates: remaining_candidates.clone(),
parent: Summary::clone(&parent),
dep: Dependency::clone(&dep),
features: Rc::clone(&features),
conflicting_activations: conflicting_activations.clone(),
let backtrack = if has_another {
Some(BacktrackFrame {
cur,
context_backup: Context::clone(&cx),
deps_backup: <BinaryHeap<DepsFrame>>::clone(&remaining_deps),
remaining_candidates: remaining_candidates.clone(),
parent: Summary::clone(&parent),
dep: Dependency::clone(&dep),
features: Rc::clone(&features),
conflicting_activations: conflicting_activations.clone(),
})
} else {
None
};

let method = Method::Required {
Expand All @@ -925,12 +929,14 @@ fn activate_deps_loop(
// Add an entry to the `backtrack_stack` so
// we can try the next one if this one fails.
if successfully_activated {
if has_another {
backtrack_stack.push(backtrack);
if let Some(b) = backtrack {
backtrack_stack.push(b);
}
} else {
// `activate` changed `cx` and then failed so put things back.
cx = backtrack.context_backup;
if let Some(b) = backtrack {
// `activate` changed `cx` and then failed so put things back.
cx = b.context_backup;
} // else we are just using the cx for the error message so we can live with a corrupted one
}
}
}
Expand Down

0 comments on commit c721937

Please sign in to comment.