Skip to content

Commit 2928e32

Browse files
authored
Simplify backtrack (#15150)
### What does this PR try to resolve? Some very small simplifications made while poking around the resolver. I was trying to understand when we would end up hitting a `continue` in the code. The easiest way was to run the tests with a panic instead. Turns out I was right, we cannot/(can no longer) hit that code path. Which allowed us to simplify some other surrounding code. ### How should we test and review this PR? All tests pass on each commit. ### Additional information
2 parents 49e5d24 + 8014091 commit 2928e32

File tree

1 file changed

+37
-39
lines changed

1 file changed

+37
-39
lines changed

src/cargo/core/resolver/mod.rs

+37-39
Original file line numberDiff line numberDiff line change
@@ -958,50 +958,48 @@ fn find_candidate(
958958
} else {
959959
None
960960
};
961-
962-
while let Some(mut frame) = backtrack_stack.pop() {
963-
let next = frame
964-
.remaining_candidates
965-
.next(&mut frame.conflicting_activations, &frame.context);
966-
let Some((candidate, has_another)) = next else {
967-
continue;
968-
};
969-
970-
// If all members of `conflicting_activations` are still
971-
// active in this back up we know that we're guaranteed to not actually
972-
// make any progress. As a result if we hit this condition we can
973-
// completely skip this backtrack frame and move on to the next.
974-
if let Some(age) = age {
975-
if frame.context.age >= age {
976-
trace!(
977-
"{} = \"{}\" skip as not solving {}: {:?}",
978-
frame.dep.package_name(),
979-
frame.dep.version_req(),
980-
parent.package_id(),
981-
conflicting_activations
982-
);
983-
// above we use `cx` to determine that this is still going to be conflicting.
984-
// but lets just double check.
985-
debug_assert!(
986-
frame
987-
.context
988-
.is_conflicting(Some(parent.package_id()), conflicting_activations)
989-
== Some(age)
990-
);
991-
continue;
992-
} else {
993-
// above we use `cx` to determine that this is not going to be conflicting.
994-
// but lets just double check.
995-
debug_assert!(frame
961+
let mut new_frame = None;
962+
if let Some(age) = age {
963+
while let Some(frame) = backtrack_stack.pop() {
964+
// If all members of `conflicting_activations` are still
965+
// active in this back up we know that we're guaranteed to not actually
966+
// make any progress. As a result if we hit this condition we can
967+
// completely skip this backtrack frame and move on to the next.
968+
969+
// Above we use `cx` to determine if this is going to be conflicting.
970+
// But lets just double check if the `pop`ed frame agrees.
971+
let frame_too_new = frame.context.age >= age;
972+
debug_assert!(
973+
frame
996974
.context
997975
.is_conflicting(Some(parent.package_id()), conflicting_activations)
998-
.is_none());
976+
== frame_too_new.then_some(age)
977+
);
978+
979+
if !frame_too_new {
980+
new_frame = Some(frame);
981+
break;
999982
}
983+
trace!(
984+
"{} = \"{}\" skip as not solving {}: {:?}",
985+
frame.dep.package_name(),
986+
frame.dep.version_req(),
987+
parent.package_id(),
988+
conflicting_activations
989+
);
1000990
}
1001-
1002-
return Some((candidate, has_another, frame));
991+
} else {
992+
// If we're here then we are in abnormal situations and need to just go one frame at a time.
993+
new_frame = backtrack_stack.pop();
1003994
}
1004-
None
995+
996+
new_frame.map(|mut frame| {
997+
let (candidate, has_another) = frame
998+
.remaining_candidates
999+
.next(&mut frame.conflicting_activations, &frame.context)
1000+
.expect("why did we save a frame that has no next?");
1001+
(candidate, has_another, frame)
1002+
})
10051003
}
10061004

10071005
fn check_cycles(resolve: &Resolve) -> CargoResult<()> {

0 commit comments

Comments
 (0)