From 680a4db9710b5a178ab1e934b757a253e4f6b560 Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Sat, 16 Mar 2019 09:25:19 -0400
Subject: [PATCH 01/11] move compatible to a type

This makes a O(n^2) loop in the hart of the resolver a O(n) loop,
 but n is small and hashing is not free.
So the main reason to do this is to make the code clearer.
---
 src/cargo/core/resolver/context.rs | 77 ++++++++++++++++++++----------
 src/cargo/core/resolver/mod.rs     | 51 ++++----------------
 2 files changed, 60 insertions(+), 68 deletions(-)

diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs
index 0c8b69ebf6f..7e1b0e1be71 100644
--- a/src/cargo/core/resolver/context.rs
+++ b/src/cargo/core/resolver/context.rs
@@ -1,4 +1,5 @@
 use std::collections::{HashMap, HashSet};
+use std::num::NonZeroU64;
 use std::rc::Rc;
 
 // "ensure" seems to require "bail" be in scope (macro hygiene issue?).
@@ -52,8 +53,36 @@ pub struct Context {
     pub warnings: RcList<String>,
 }
 
-/// list all the activated versions of a particular crate name from a source
-pub type Activations = im_rc::HashMap<(InternedString, SourceId), Rc<Vec<Summary>>>;
+/// find the activated version of a crate based on the name, source, and semver compatibility
+pub type Activations = im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), Summary>;
+
+/// A type that represents when cargo treats two Versions as compatible.
+/// Versions `a` and `b` are compatible if their left-most nonzero digit is the
+/// same.
+#[derive(Clone, Copy, Eq, PartialEq, Hash)]
+pub enum SemverCompatibility {
+    Major(NonZeroU64),
+    Minor(NonZeroU64),
+    Patch(u64),
+}
+
+impl From<&semver::Version> for SemverCompatibility {
+    fn from(ver: &semver::Version) -> Self {
+        if let Some(m) = NonZeroU64::new(ver.major) {
+            return SemverCompatibility::Major(m);
+        }
+        if let Some(m) = NonZeroU64::new(ver.minor) {
+            return SemverCompatibility::Minor(m);
+        }
+        SemverCompatibility::Patch(ver.patch)
+    }
+}
+
+impl PackageId {
+    pub fn as_activations_key(&self) -> (InternedString, SourceId, SemverCompatibility) {
+        (self.name(), self.source_id(), self.version().into())
+    }
+}
 
 impl Context {
     pub fn new(check_public_visible_dependencies: bool) -> Context {
@@ -78,22 +107,27 @@ impl Context {
     /// Returns `true` if this summary with the given method is already activated.
     pub fn flag_activated(&mut self, summary: &Summary, method: &Method<'_>) -> CargoResult<bool> {
         let id = summary.package_id();
-        let prev = self
-            .activations
-            .entry((id.name(), id.source_id()))
-            .or_insert_with(|| Rc::new(Vec::new()));
-        if !prev.iter().any(|c| c == summary) {
-            self.resolve_graph.push(GraphNode::Add(id));
-            if let Some(link) = summary.links() {
-                ensure!(
-                    self.links.insert(link, id).is_none(),
-                    "Attempting to resolve a dependency with more then one crate with the \
-                     links={}.\nThis will not build as is. Consider rebuilding the .lock file.",
-                    &*link
+        match self.activations.entry(id.as_activations_key()) {
+            im_rc::hashmap::Entry::Occupied(o) => {
+                debug_assert_eq!(
+                    o.get(),
+                    summary,
+                    "cargo does not allow two semver compatible versions"
                 );
             }
-            Rc::make_mut(prev).push(summary.clone());
-            return Ok(false);
+            im_rc::hashmap::Entry::Vacant(v) => {
+                self.resolve_graph.push(GraphNode::Add(id));
+                if let Some(link) = summary.links() {
+                    ensure!(
+                        self.links.insert(link, id).is_none(),
+                        "Attempting to resolve a dependency with more then one crate with the \
+                         links={}.\nThis will not build as is. Consider rebuilding the .lock file.",
+                        &*link
+                    );
+                }
+                v.insert(summary.clone());
+                return Ok(false);
+            }
         }
         debug!("checking if {} is already activated", summary.package_id());
         let (features, use_default) = match *method {
@@ -149,17 +183,10 @@ impl Context {
         Ok(deps)
     }
 
-    pub fn prev_active(&self, dep: &Dependency) -> &[Summary] {
-        self.activations
-            .get(&(dep.package_name(), dep.source_id()))
-            .map(|v| &v[..])
-            .unwrap_or(&[])
-    }
-
     pub fn is_active(&self, id: PackageId) -> bool {
         self.activations
-            .get(&(id.name(), id.source_id()))
-            .map(|v| v.iter().any(|s| s.package_id() == id))
+            .get(&id.as_activations_key())
+            .map(|s| s.package_id() == id)
             .unwrap_or(false)
     }
 
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index a0b57173af1..dfebf5e14f3 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -137,7 +137,7 @@ pub fn resolve(
     let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
 
     let mut cksums = HashMap::new();
-    for summary in cx.activations.values().flat_map(|v| v.iter()) {
+    for summary in cx.activations.values() {
         let cksum = summary.checksum().map(|s| s.to_string());
         cksums.insert(summary.package_id(), cksum);
     }
@@ -237,13 +237,6 @@ fn activate_deps_loop(
             dep.package_name(),
             candidates.len()
         );
-        trace!(
-            "{}[{}]>{} {} prev activations",
-            parent.name(),
-            cur,
-            dep.package_name(),
-            cx.prev_active(&dep).len()
-        );
 
         let just_here_for_the_error_messages = just_here_for_the_error_messages
             && past_conflicting_activations
@@ -759,16 +752,15 @@ impl RemainingCandidates {
         dep: &Dependency,
         parent: PackageId,
     ) -> Option<(Candidate, bool)> {
-        let prev_active = cx.prev_active(dep);
-
         'main: for (_, b) in self.remaining.by_ref() {
+            let b_id = b.summary.package_id();
             // The `links` key in the manifest dictates that there's only one
             // package in a dependency graph, globally, with that particular
             // `links` key. If this candidate links to something that's already
             // linked to by a different package then we've gotta skip this.
             if let Some(link) = b.summary.links() {
                 if let Some(&a) = cx.links.get(&link) {
-                    if a != b.summary.package_id() {
+                    if a != b_id {
                         conflicting_prev_active
                             .entry(a)
                             .or_insert_with(|| ConflictReason::Links(link));
@@ -785,10 +777,7 @@ impl RemainingCandidates {
             //
             // Here we throw out our candidate if it's *compatible*, yet not
             // equal, to all previously activated versions.
-            if let Some(a) = prev_active
-                .iter()
-                .find(|a| compatible(a.version(), b.summary.version()))
-            {
+            if let Some(a) = cx.activations.get(&b_id.as_activations_key()) {
                 if *a != b.summary {
                     conflicting_prev_active
                         .entry(a.package_id())
@@ -803,11 +792,11 @@ impl RemainingCandidates {
             // all of witch also need to be checked the same way.
             if let Some(public_dependency) = cx.public_dependency.as_ref() {
                 let existing_public_deps: Vec<PackageId> = public_dependency
-                    .get(&b.summary.package_id())
+                    .get(&b_id)
                     .iter()
                     .flat_map(|x| x.values())
                     .filter_map(|x| if x.1 { Some(&x.0) } else { None })
-                    .chain(&Some(b.summary.package_id()))
+                    .chain(&Some(b_id))
                     .cloned()
                     .collect();
                 for t in existing_public_deps {
@@ -851,27 +840,6 @@ impl RemainingCandidates {
     }
 }
 
-// Returns if `a` and `b` are compatible in the semver sense. This is a
-// commutative operation.
-//
-// Versions `a` and `b` are compatible if their left-most nonzero digit is the
-// same.
-fn compatible(a: &semver::Version, b: &semver::Version) -> bool {
-    if a.major != b.major {
-        return false;
-    }
-    if a.major != 0 {
-        return true;
-    }
-    if a.minor != b.minor {
-        return false;
-    }
-    if a.minor != 0 {
-        return true;
-    }
-    a.patch == b.patch
-}
-
 /// Looks through the states in `backtrack_stack` for dependencies with
 /// remaining candidates. For each one, also checks if rolling back
 /// could change the outcome of the failed resolution that caused backtracking
@@ -937,11 +905,8 @@ fn find_candidate(
 }
 
 fn check_cycles(resolve: &Resolve, activations: &Activations) -> CargoResult<()> {
-    let summaries: HashMap<PackageId, &Summary> = activations
-        .values()
-        .flat_map(|v| v.iter())
-        .map(|s| (s.package_id(), s))
-        .collect();
+    let summaries: HashMap<PackageId, &Summary> =
+        activations.values().map(|s| (s.package_id(), s)).collect();
 
     // Sort packages to produce user friendly deterministic errors.
     let mut all_packages: Vec<_> = resolve.iter().collect();

From 6bd554fb0d365547c0e79bfa0cb951cf932401a7 Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Sat, 16 Mar 2019 13:49:46 -0400
Subject: [PATCH 02/11] pre calculate how far back to back-jump

---
 src/cargo/core/resolver/conflict_cache.rs |  4 +-
 src/cargo/core/resolver/context.rs        | 32 +++++---
 src/cargo/core/resolver/mod.rs            | 90 +++++++++++++++--------
 3 files changed, 81 insertions(+), 45 deletions(-)

diff --git a/src/cargo/core/resolver/conflict_cache.rs b/src/cargo/core/resolver/conflict_cache.rs
index 58beb6da17f..63fdca344fa 100644
--- a/src/cargo/core/resolver/conflict_cache.rs
+++ b/src/cargo/core/resolver/conflict_cache.rs
@@ -29,7 +29,7 @@ impl ConflictStoreTrie {
                 if must_contain.is_none() {
                     // `is_conflicting` checks that all the elements are active,
                     // but we have checked each one by the recursion of this function.
-                    debug_assert!(cx.is_conflicting(None, c));
+                    debug_assert!(cx.is_conflicting(None, c).is_some());
                     Some(c)
                 } else {
                     // We did not find `must_contain`, so we need to keep looking.
@@ -42,7 +42,7 @@ impl ConflictStoreTrie {
                     .unwrap_or_else(|| m.range(..))
                 {
                     // If the key is active, then we need to check all of the corresponding subtrie.
-                    if cx.is_active(pid) {
+                    if cx.is_active(pid).is_some() {
                         if let Some(o) =
                             store.find_conflicting(cx, must_contain.filter(|&f| f != pid))
                         {
diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs
index 7e1b0e1be71..13ed2087173 100644
--- a/src/cargo/core/resolver/context.rs
+++ b/src/cargo/core/resolver/context.rs
@@ -54,7 +54,10 @@ pub struct Context {
 }
 
 /// find the activated version of a crate based on the name, source, and semver compatibility
-pub type Activations = im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), Summary>;
+/// This all so stores the size of `Activations` when that version was add as an "age".
+/// This is used to speed up backtracking.
+pub type Activations =
+    im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), (Summary, usize)>;
 
 /// A type that represents when cargo treats two Versions as compatible.
 /// Versions `a` and `b` are compatible if their left-most nonzero digit is the
@@ -107,10 +110,11 @@ impl Context {
     /// Returns `true` if this summary with the given method is already activated.
     pub fn flag_activated(&mut self, summary: &Summary, method: &Method<'_>) -> CargoResult<bool> {
         let id = summary.package_id();
+        let activations_len = self.activations.len();
         match self.activations.entry(id.as_activations_key()) {
             im_rc::hashmap::Entry::Occupied(o) => {
                 debug_assert_eq!(
-                    o.get(),
+                    &o.get().0,
                     summary,
                     "cargo does not allow two semver compatible versions"
                 );
@@ -125,7 +129,7 @@ impl Context {
                         &*link
                     );
                 }
-                v.insert(summary.clone());
+                v.insert((summary.clone(), activations_len));
                 return Ok(false);
             }
         }
@@ -183,24 +187,30 @@ impl Context {
         Ok(deps)
     }
 
-    pub fn is_active(&self, id: PackageId) -> bool {
+    /// If the package is active returns the "age" (len of activations) when it was added
+    pub fn is_active(&self, id: PackageId) -> Option<usize> {
         self.activations
             .get(&id.as_activations_key())
-            .map(|s| s.package_id() == id)
-            .unwrap_or(false)
+            .and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None })
     }
 
     /// Checks whether all of `parent` and the keys of `conflicting activations`
     /// are still active.
+    /// If so returns the "age" (len of activations) when the newest one was added.
     pub fn is_conflicting(
         &self,
         parent: Option<PackageId>,
         conflicting_activations: &ConflictMap,
-    ) -> bool {
-        conflicting_activations
-            .keys()
-            .chain(parent.as_ref())
-            .all(|&id| self.is_active(id))
+    ) -> Option<usize> {
+        let mut max = 0;
+        for &id in conflicting_activations.keys().chain(parent.as_ref()) {
+            if let Some(age) = self.is_active(id) {
+                max = std::cmp::max(max, age);
+            } else {
+                return None;
+            }
+        }
+        Some(max)
     }
 
     /// Returns all dependencies and the features we want from them.
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index dfebf5e14f3..d02f6342665 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -137,7 +137,7 @@ pub fn resolve(
     let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
 
     let mut cksums = HashMap::new();
-    for summary in cx.activations.values() {
+    for (summary, _) in cx.activations.values() {
         let cksum = summary.checksum().map(|s| s.to_string());
         cksums.insert(summary.package_id(), cksum);
     }
@@ -302,6 +302,7 @@ fn activate_deps_loop(
                 }
 
                 match find_candidate(
+                    &cx,
                     &mut backtrack_stack,
                     &parent,
                     backtracked,
@@ -493,6 +494,7 @@ fn activate_deps_loop(
                     let activate_for_error_message = has_past_conflicting_dep && !has_another && {
                         just_here_for_the_error_messages || {
                             find_candidate(
+                                &cx,
                                 &mut backtrack_stack.clone(),
                                 &parent,
                                 backtracked,
@@ -777,7 +779,7 @@ impl RemainingCandidates {
             //
             // Here we throw out our candidate if it's *compatible*, yet not
             // equal, to all previously activated versions.
-            if let Some(a) = cx.activations.get(&b_id.as_activations_key()) {
+            if let Some((a, _)) = cx.activations.get(&b_id.as_activations_key()) {
                 if *a != b.summary {
                     conflicting_prev_active
                         .entry(a.package_id())
@@ -850,11 +852,33 @@ impl RemainingCandidates {
 /// Read <https://github.com/rust-lang/cargo/pull/4834>
 /// For several more detailed explanations of the logic here.
 fn find_candidate(
+    cx: &Context,
     backtrack_stack: &mut Vec<BacktrackFrame>,
     parent: &Summary,
     backtracked: bool,
     conflicting_activations: &ConflictMap,
 ) -> Option<(Candidate, bool, BacktrackFrame)> {
+    // When we're calling this method we know that `parent` failed to
+    // activate. That means that some dependency failed to get resolved for
+    // whatever reason. Normally, that means that all of those reasons
+    // (plus maybe some extras) are listed in `conflicting_activations`.
+    //
+    // The abnormal situations are things that do not put all of the reasons in `conflicting_activations`:
+    // If we backtracked we do not know how our `conflicting_activations` related to
+    // the cause of that backtrack, so we do not update it.
+    // If we had a PublicDependency conflict, then we do not yet have a compact way to
+    // represent all the parts of the problem, so `conflicting_activations` is incomplete.
+    let age = if !backtracked
+        && !conflicting_activations
+            .values()
+            .any(|c| *c == ConflictReason::PublicDependency)
+    {
+        // we dont have abnormal situations. So we can ask `cx` for how far back we need to go.
+        cx.is_conflicting(Some(parent.package_id()), conflicting_activations)
+    } else {
+        None
+    };
+
     while let Some(mut frame) = backtrack_stack.pop() {
         let next = frame.remaining_candidates.next(
             &mut frame.conflicting_activations,
@@ -866,37 +890,37 @@ fn find_candidate(
             Some(pair) => pair,
             None => continue,
         };
-        // When we're calling this method we know that `parent` failed to
-        // activate. That means that some dependency failed to get resolved for
-        // whatever reason. Normally, that means that all of those reasons
-        // (plus maybe some extras) are listed in `conflicting_activations`.
-        //
-        // This means that if all members of `conflicting_activations` are still
+
+        // If all members of `conflicting_activations` are still
         // active in this back up we know that we're guaranteed to not actually
         // make any progress. As a result if we hit this condition we can
         // completely skip this backtrack frame and move on to the next.
-        //
-        // The abnormal situations are things that do not put all of the reasons in `conflicting_activations`:
-        // If we backtracked we do not know how our `conflicting_activations` related to
-        // the cause of that backtrack, so we do not update it.
-        // If we had a PublicDependency conflict, then we do not yet have a compact way to
-        // represent all the parts of the problem, so `conflicting_activations` is incomplete.
-        if !backtracked
-            && !conflicting_activations
-                .values()
-                .any(|c| *c == ConflictReason::PublicDependency)
-            && frame
-                .context
-                .is_conflicting(Some(parent.package_id()), conflicting_activations)
-        {
-            trace!(
-                "{} = \"{}\" skip as not solving {}: {:?}",
-                frame.dep.package_name(),
-                frame.dep.version_req(),
-                parent.package_id(),
-                conflicting_activations
-            );
-            continue;
+        if let Some(age) = age {
+            if frame.context.activations.len() > age {
+                trace!(
+                    "{} = \"{}\" skip as not solving {}: {:?}",
+                    frame.dep.package_name(),
+                    frame.dep.version_req(),
+                    parent.package_id(),
+                    conflicting_activations
+                );
+                // above we use `cx` to determine that this is still going to be conflicting.
+                // but lets just double check.
+                debug_assert!(
+                    frame
+                        .context
+                        .is_conflicting(Some(parent.package_id()), conflicting_activations)
+                        == Some(age)
+                );
+                continue;
+            } else {
+                // above we use `cx` to determine that this is not going to be conflicting.
+                // but lets just double check.
+                debug_assert!(frame
+                    .context
+                    .is_conflicting(Some(parent.package_id()), conflicting_activations)
+                    .is_none());
+            }
         }
 
         return Some((candidate, has_another, frame));
@@ -905,8 +929,10 @@ fn find_candidate(
 }
 
 fn check_cycles(resolve: &Resolve, activations: &Activations) -> CargoResult<()> {
-    let summaries: HashMap<PackageId, &Summary> =
-        activations.values().map(|s| (s.package_id(), s)).collect();
+    let summaries: HashMap<PackageId, &Summary> = activations
+        .values()
+        .map(|(s, _)| (s.package_id(), s))
+        .collect();
 
     // Sort packages to produce user friendly deterministic errors.
     let mut all_packages: Vec<_> = resolve.iter().collect();

From 0f1791be089e8865c58c7e52cadafd9d26965b39 Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Sat, 16 Mar 2019 15:14:27 -0400
Subject: [PATCH 03/11] proptest, do we have more then one conflict that
 matches?

Yes, apparently we do. So I can't do optimizations based on that being unique.
---
 src/cargo/core/resolver/conflict_cache.rs |  5 ++--
 tests/testsuite/resolve.rs                | 28 +++++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/cargo/core/resolver/conflict_cache.rs b/src/cargo/core/resolver/conflict_cache.rs
index 63fdca344fa..269fd255fcc 100644
--- a/src/cargo/core/resolver/conflict_cache.rs
+++ b/src/cargo/core/resolver/conflict_cache.rs
@@ -37,6 +37,7 @@ impl ConflictStoreTrie {
                 }
             }
             ConflictStoreTrie::Node(m) => {
+                let mut out = None;
                 for (&pid, store) in must_contain
                     .map(|f| m.range(..=f))
                     .unwrap_or_else(|| m.range(..))
@@ -46,13 +47,13 @@ impl ConflictStoreTrie {
                         if let Some(o) =
                             store.find_conflicting(cx, must_contain.filter(|&f| f != pid))
                         {
-                            return Some(o);
+                            assert!(out.replace(o).is_none());
                         }
                     }
                     // Else, if it is not active then there is no way any of the corresponding
                     // subtrie will be conflicting.
                 }
-                None
+                out
             }
         }
     }
diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs
index 6fea4a69406..d187c6478ab 100644
--- a/tests/testsuite/resolve.rs
+++ b/tests/testsuite/resolve.rs
@@ -1407,3 +1407,31 @@ fn conflict_store_bug() {
     let reg = registry(input);
     let _ = resolve_and_validated(pkg_id("root"), vec![dep("j")], &reg);
 }
+
+#[test]
+fn conflict_store_more_then_one_match() {
+    let input = vec![
+        pkg!(("A", "0.0.0")),
+        pkg!(("A", "0.0.1")),
+        pkg!(("A-sys", "0.0.0")),
+        pkg!(("A-sys", "0.0.1")),
+        pkg!(("A-sys", "0.0.2")),
+        pkg!(("A-sys", "0.0.3")),
+        pkg!(("A-sys", "0.0.12")),
+        pkg!(("A-sys", "0.0.16")),
+        pkg!(("B-sys", "0.0.0")),
+        pkg!(("B-sys", "0.0.1")),
+        pkg!(("B-sys", "0.0.2") => [dep_req("A-sys", "= 0.0.12"),]),
+        pkg!(("BA-sys", "0.0.0") => [dep_req("A-sys","= 0.0.16"),]),
+        pkg!(("BA-sys", "0.0.1") => [dep("bad"),]),
+        pkg!(("BA-sys", "0.0.2") => [dep("bad"),]),
+        pkg!("nA" => [
+            dep("A"),
+            dep_req("A-sys", "<= 0.0.3"),
+            dep("B-sys"),
+            dep("BA-sys"),
+        ]),
+    ];
+    let reg = registry(input);
+    let _ = resolve_and_validated(pkg_id("root"), vec![dep("nA")], &reg);
+}

From 70c59ef64b9aa97309d900a0956a1a619c4f4ff9 Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Sat, 16 Mar 2019 17:41:32 -0400
Subject: [PATCH 04/11] We have more then one conflict that matches, so pick
 one with the max back-jumping.

---
 src/cargo/core/resolver/conflict_cache.rs | 32 +++++++++++++++++------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/src/cargo/core/resolver/conflict_cache.rs b/src/cargo/core/resolver/conflict_cache.rs
index 269fd255fcc..5f1895a0a97 100644
--- a/src/cargo/core/resolver/conflict_cache.rs
+++ b/src/cargo/core/resolver/conflict_cache.rs
@@ -18,19 +18,21 @@ enum ConflictStoreTrie {
 
 impl ConflictStoreTrie {
     /// Finds any known set of conflicts, if any,
-    /// which are activated in `cx` and pass the `filter` specified?
+    /// which are activated in `cx` and contain `PackageId` specified.
+    /// If more then one are activated, then it will return
+    /// one that will allow for the most jump-back.
     fn find_conflicting(
         &self,
         cx: &Context,
         must_contain: Option<PackageId>,
-    ) -> Option<&ConflictMap> {
+    ) -> Option<(&ConflictMap, usize)> {
         match self {
             ConflictStoreTrie::Leaf(c) => {
                 if must_contain.is_none() {
                     // `is_conflicting` checks that all the elements are active,
                     // but we have checked each one by the recursion of this function.
                     debug_assert!(cx.is_conflicting(None, c).is_some());
-                    Some(c)
+                    Some((c, 0))
                 } else {
                     // We did not find `must_contain`, so we need to keep looking.
                     None
@@ -43,11 +45,22 @@ impl ConflictStoreTrie {
                     .unwrap_or_else(|| m.range(..))
                 {
                     // If the key is active, then we need to check all of the corresponding subtrie.
-                    if cx.is_active(pid).is_some() {
-                        if let Some(o) =
+                    if let Some(age_this) = cx.is_active(pid) {
+                        if let Some((o, age_o)) =
                             store.find_conflicting(cx, must_contain.filter(|&f| f != pid))
                         {
-                            assert!(out.replace(o).is_none());
+                            let age = if must_contain == Some(pid) {
+                                // all the results will include `must_contain`
+                                // so the age of must_contain is not relevant to find the best result.
+                                age_o
+                            } else {
+                                std::cmp::max(age_this, age_o)
+                            };
+                            let out_age = out.get_or_insert((o, age)).1;
+                            if out_age > age {
+                                // we found one that can jump-back further so replace the out.
+                                out = Some((o, age));
+                            }
                         }
                     }
                     // Else, if it is not active then there is no way any of the corresponding
@@ -138,7 +151,9 @@ impl ConflictCache {
         }
     }
     /// Finds any known set of conflicts, if any,
-    /// which are activated in `cx` and pass the `filter` specified?
+    /// which are activated in `cx` and contain `PackageId` specified.
+    /// If more then one are activated, then it will return
+    /// one that will allow for the most jump-back.
     pub fn find_conflicting(
         &self,
         cx: &Context,
@@ -148,7 +163,8 @@ impl ConflictCache {
         let out = self
             .con_from_dep
             .get(dep)?
-            .find_conflicting(cx, must_contain);
+            .find_conflicting(cx, must_contain)
+            .map(|(c, _)| c);
         if cfg!(debug_assertions) {
             if let Some(f) = must_contain {
                 if let Some(c) = &out {

From fd27ee7bd738c1c1d949693f870f2db88d1f969f Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Mon, 18 Mar 2019 17:40:23 -0400
Subject: [PATCH 05/11] A dep is equivalent to one of the things it can resolve
 to. Thus, if all the things it can resolve to have already ben determined to
 be conflicting, then we can just say that we conflict with the parent.

---
 src/cargo/core/resolver/conflict_cache.rs | 34 ++++++++++++
 src/cargo/core/resolver/mod.rs            | 65 +++++++++++++++++++++++
 tests/testsuite/resolve.rs                |  2 +-
 3 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/src/cargo/core/resolver/conflict_cache.rs b/src/cargo/core/resolver/conflict_cache.rs
index 5f1895a0a97..e2ae54e9bd0 100644
--- a/src/cargo/core/resolver/conflict_cache.rs
+++ b/src/cargo/core/resolver/conflict_cache.rs
@@ -102,6 +102,28 @@ impl ConflictStoreTrie {
             *self = ConflictStoreTrie::Leaf(con)
         }
     }
+
+    fn contains(&self, mut iter: impl Iterator<Item = PackageId>, con: &ConflictMap) -> bool {
+        match (self, iter.next()) {
+            (ConflictStoreTrie::Leaf(c), None) => {
+                if cfg!(debug_assertions) {
+                    let a: Vec<_> = con.keys().collect();
+                    let b: Vec<_> = c.keys().collect();
+                    assert_eq!(a, b);
+                }
+                true
+            }
+            (ConflictStoreTrie::Leaf(_), Some(_)) => false,
+            (ConflictStoreTrie::Node(_), None) => false,
+            (ConflictStoreTrie::Node(m), Some(n)) => {
+                if let Some(next) = m.get(&n) {
+                    next.contains(iter, con)
+                } else {
+                    false
+                }
+            }
+        }
+    }
 }
 
 pub(super) struct ConflictCache {
@@ -206,6 +228,18 @@ impl ConflictCache {
                 .insert(dep.clone());
         }
     }
+
+    /// Check if a conflict was previously added of the form:
+    /// `dep` is known to be unresolvable if
+    /// all the `PackageId` entries are activated.
+    pub fn contains(&self, dep: &Dependency, con: &ConflictMap) -> bool {
+        if let Some(cst) = self.con_from_dep.get(dep) {
+            cst.contains(con.keys().cloned(), &con)
+        } else {
+            false
+        }
+    }
+
     pub fn dependencies_conflicting_with(&self, pid: PackageId) -> Option<&HashSet<Dependency>> {
         self.dep_from_pid.get(&pid)
     }
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index d02f6342665..ea23650539b 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -299,6 +299,15 @@ fn activate_deps_loop(
                 // not be right, so we can't push into our global cache.
                 if !just_here_for_the_error_messages && !backtracked {
                     past_conflicting_activations.insert(&dep, &conflicting_activations);
+                    if let Some(c) = generalize_conflicting(
+                        &cx,
+                        registry,
+                        &mut past_conflicting_activations,
+                        &dep,
+                        &conflicting_activations,
+                    ) {
+                        conflicting_activations = c;
+                    }
                 }
 
                 match find_candidate(
@@ -842,6 +851,62 @@ impl RemainingCandidates {
     }
 }
 
+/// Attempts to find a new conflict that allows a bigger backjump then the input one.
+/// It will add the new conflict to the cache if one is found.
+///
+/// Panics if the input conflict is not all active in `cx`.
+fn generalize_conflicting(
+    cx: &Context,
+    registry: &mut RegistryQueryer<'_>,
+    past_conflicting_activations: &mut conflict_cache::ConflictCache,
+    dep: &Dependency,
+    conflicting_activations: &ConflictMap,
+) -> Option<ConflictMap> {
+    if conflicting_activations.is_empty() {
+        return None;
+    }
+    // We need to determine the "age" that this `conflicting_activations` will jump to, and why.
+    let (jumpback_critical_age, jumpback_critical_id) = conflicting_activations
+        .keys()
+        .map(|&c| (cx.is_active(c).expect("not currently active!?"), c))
+        .max()
+        .unwrap();
+    let jumpback_critical_reason: ConflictReason =
+        conflicting_activations[&jumpback_critical_id].clone();
+    // What parents dose that critical activation have
+    for (critical_parent, critical_parents_deps) in
+        cx.parents.edges(&jumpback_critical_id).filter(|(p, _)| {
+            // it will only help backjump further if it is older then the critical_age
+            cx.is_active(*p).expect("parent not currently active!?") < jumpback_critical_age
+        })
+    {
+        for critical_parents_dep in critical_parents_deps.iter() {
+            // A dep is equivalent to one of the things it can resolve to.
+            // Thus, if all the things it can resolve to have already ben determined
+            // to be conflicting, then we can just say that we conflict with the parent.
+            if registry
+                .query(&critical_parents_dep)
+                .expect("an already used dep now error!?")
+                .iter()
+                .rev() // the last one to be tried is the least likely to be in the cache, so start with that.
+                .all(|other| {
+                    let mut con = conflicting_activations.clone();
+                    con.remove(&jumpback_critical_id);
+                    con.insert(other.summary.package_id(), jumpback_critical_reason.clone());
+                    past_conflicting_activations.contains(&dep, &con)
+                })
+            {
+                let mut con = conflicting_activations.clone();
+                con.remove(&jumpback_critical_id);
+                con.insert(*critical_parent, jumpback_critical_reason);
+                past_conflicting_activations.insert(&dep, &con);
+                return Some(con);
+            }
+        }
+    }
+    None
+}
+
 /// Looks through the states in `backtrack_stack` for dependencies with
 /// remaining candidates. For each one, also checks if rolling back
 /// could change the outcome of the failed resolution that caused backtracking
diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs
index d187c6478ab..ca32ba45fcd 100644
--- a/tests/testsuite/resolve.rs
+++ b/tests/testsuite/resolve.rs
@@ -1345,7 +1345,7 @@ fn large_conflict_cache() {
         pkg!(("last", "0.0.0") => [dep("bad")]), // just to make sure last is less constrained
     ];
     let mut root_deps = vec![dep("last")];
-    const NUM_VERSIONS: u8 = 3;
+    const NUM_VERSIONS: u8 = 20;
     for name in 0..=NUM_VERSIONS {
         // a large number of conflicts can easily be generated by a sys crate.
         let sys_name = format!("{}-sys", (b'a' + name) as char);

From a516d5b67a75b5d44f8cda12045505c2ff7dcecb Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Fri, 22 Mar 2019 22:30:06 -0400
Subject: [PATCH 06/11] if we are a descendant of the trigger of the problem.
 The best generalization of this is to let things bubble up and let
 `jumpback_critical_id` figure this out.

---
 src/cargo/core/resolver/mod.rs | 12 ++++++++++++
 src/cargo/util/graph.rs        | 18 ++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index ea23650539b..158f30553ad 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -303,6 +303,7 @@ fn activate_deps_loop(
                         &cx,
                         registry,
                         &mut past_conflicting_activations,
+                        &parent,
                         &dep,
                         &conflicting_activations,
                     ) {
@@ -859,6 +860,7 @@ fn generalize_conflicting(
     cx: &Context,
     registry: &mut RegistryQueryer<'_>,
     past_conflicting_activations: &mut conflict_cache::ConflictCache,
+    parent: &Summary,
     dep: &Dependency,
     conflicting_activations: &ConflictMap,
 ) -> Option<ConflictMap> {
@@ -873,6 +875,16 @@ fn generalize_conflicting(
         .unwrap();
     let jumpback_critical_reason: ConflictReason =
         conflicting_activations[&jumpback_critical_id].clone();
+
+    if cx
+        .parents
+        .is_path_from_to(&parent.package_id(), &jumpback_critical_id)
+    {
+        // We are a descendant of the trigger of the problem.
+        // The best generalization of this is to let things bubble up
+        // and let `jumpback_critical_id` figure this out.
+        return None;
+    }
     // What parents dose that critical activation have
     for (critical_parent, critical_parents_deps) in
         cx.parents.edges(&jumpback_critical_id).filter(|(p, _)| {
diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs
index 603edf14f9c..00f58a292fb 100644
--- a/src/cargo/util/graph.rs
+++ b/src/cargo/util/graph.rs
@@ -71,6 +71,24 @@ impl<N: Eq + Ord + Clone, E: Default + Clone> Graph<N, E> {
         self.nodes.keys()
     }
 
+    /// Checks if there is a path from `from` to `to`.
+    pub fn is_path_from_to<'a>(&'a self, from: &'a N, to: &'a N) -> bool {
+        let mut stack = vec![from];
+        let mut seen = BTreeSet::new();
+        seen.insert(from);
+        while let Some(iter) = stack.pop().and_then(|p| self.nodes.get(p)) {
+            for p in iter.keys() {
+                if p == to {
+                    return true;
+                }
+                if seen.insert(p) {
+                    stack.push(p);
+                }
+            }
+        }
+        false
+    }
+
     /// Resolves one of the paths from the given dependent package down to
     /// a leaf.
     pub fn path_to_bottom<'a>(&'a self, mut pkg: &'a N) -> Vec<&'a N> {

From 3da71e8040636aafb14ff0001908a97edf09d521 Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Fri, 22 Mar 2019 22:51:42 -0400
Subject: [PATCH 07/11] rename fuzz tests so they can be run easily

---
 tests/testsuite/resolve.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs
index ca32ba45fcd..a1b3277748d 100644
--- a/tests/testsuite/resolve.rs
+++ b/tests/testsuite/resolve.rs
@@ -37,7 +37,7 @@ proptest! {
 
     /// NOTE: if you think this test has failed spuriously see the note at the top of this macro.
     #[test]
-    fn passes_validation(
+    fn prop_passes_validation(
         PrettyPrintRegistry(input) in registry_strategy(50, 20, 60)
     )  {
         let reg = registry(input.clone());
@@ -55,7 +55,7 @@ proptest! {
 
     /// NOTE: if you think this test has failed spuriously see the note at the top of this macro.
     #[test]
-    fn minimum_version_errors_the_same(
+    fn prop_minimum_version_errors_the_same(
             PrettyPrintRegistry(input) in registry_strategy(50, 20, 60)
     ) {
         enable_nightly_features();
@@ -105,7 +105,7 @@ proptest! {
 
     /// NOTE: if you think this test has failed spuriously see the note at the top of this macro.
     #[test]
-    fn removing_a_dep_cant_break(
+    fn prop_removing_a_dep_cant_break(
             PrettyPrintRegistry(input) in registry_strategy(50, 20, 60),
             indexes_to_remove in prop::collection::vec((any::<prop::sample::Index>(), any::<prop::sample::Index>()), ..10)
     ) {
@@ -147,7 +147,7 @@ proptest! {
 
     /// NOTE: if you think this test has failed spuriously see the note at the top of this macro.
     #[test]
-    fn limited_independence_of_irrelevant_alternatives(
+    fn prop_limited_independence_of_irrelevant_alternatives(
         PrettyPrintRegistry(input) in registry_strategy(50, 20, 60),
         indexes_to_unpublish in prop::collection::vec(any::<prop::sample::Index>(), ..10)
     )  {

From 5fea76b2ae36111abbf0baa40605c7944095d78e Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Tue, 26 Mar 2019 15:02:52 -0400
Subject: [PATCH 08/11] never use a generalized in an error message

---
 src/cargo/core/resolver/mod.rs | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index 158f30553ad..fe582cb2147 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -297,6 +297,7 @@ fn activate_deps_loop(
                 // As we mentioned above with the `backtracked` variable if this
                 // local is set to `true` then our `conflicting_activations` may
                 // not be right, so we can't push into our global cache.
+                let mut generalize_conflicting_activations = None;
                 if !just_here_for_the_error_messages && !backtracked {
                     past_conflicting_activations.insert(&dep, &conflicting_activations);
                     if let Some(c) = generalize_conflicting(
@@ -307,7 +308,7 @@ fn activate_deps_loop(
                         &dep,
                         &conflicting_activations,
                     ) {
-                        conflicting_activations = c;
+                        generalize_conflicting_activations = Some(c);
                     }
                 }
 
@@ -316,7 +317,9 @@ fn activate_deps_loop(
                     &mut backtrack_stack,
                     &parent,
                     backtracked,
-                    &conflicting_activations,
+                    generalize_conflicting_activations
+                        .as_ref()
+                        .unwrap_or(&conflicting_activations),
                 ) {
                     Some((candidate, has_another, frame)) => {
                         // Reset all of our local variables used with the
@@ -885,7 +888,7 @@ fn generalize_conflicting(
         // and let `jumpback_critical_id` figure this out.
         return None;
     }
-    // What parents dose that critical activation have
+    // What parents does that critical activation have
     for (critical_parent, critical_parents_deps) in
         cx.parents.edges(&jumpback_critical_id).filter(|(p, _)| {
             // it will only help backjump further if it is older then the critical_age

From a979c9b3b2bde60172a2bb029fe452cdf2846d47 Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Wed, 27 Mar 2019 20:07:24 -0400
Subject: [PATCH 09/11] add a type alias for Age and add some comments

---
 src/cargo/core/resolver/context.rs | 26 +++++++++++++++++++-------
 src/cargo/core/resolver/mod.rs     |  2 +-
 src/cargo/core/resolver/types.rs   |  4 ++++
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs
index 13ed2087173..cea11a2667e 100644
--- a/src/cargo/core/resolver/context.rs
+++ b/src/cargo/core/resolver/context.rs
@@ -53,11 +53,16 @@ pub struct Context {
     pub warnings: RcList<String>,
 }
 
+/// When backtracking it can be useful to know how far back to go.
+/// The `ContextAge` of a `Context` is a monotonically increasing counter of the number
+/// of decisions made to get to this state.
+/// Several structures store the `ContextAge` when it was added, this lets use jump back.
+pub type ContextAge = usize;
+
 /// find the activated version of a crate based on the name, source, and semver compatibility
-/// This all so stores the size of `Activations` when that version was add as an "age".
-/// This is used to speed up backtracking.
+/// This all so stores the `ContextAge`.
 pub type Activations =
-    im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), (Summary, usize)>;
+    im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), (Summary, ContextAge)>;
 
 /// A type that represents when cargo treats two Versions as compatible.
 /// Versions `a` and `b` are compatible if their left-most nonzero digit is the
@@ -110,7 +115,7 @@ impl Context {
     /// Returns `true` if this summary with the given method is already activated.
     pub fn flag_activated(&mut self, summary: &Summary, method: &Method<'_>) -> CargoResult<bool> {
         let id = summary.package_id();
-        let activations_len = self.activations.len();
+        let age: ContextAge = self.age();
         match self.activations.entry(id.as_activations_key()) {
             im_rc::hashmap::Entry::Occupied(o) => {
                 debug_assert_eq!(
@@ -129,7 +134,7 @@ impl Context {
                         &*link
                     );
                 }
-                v.insert((summary.clone(), activations_len));
+                v.insert((summary.clone(), age));
                 return Ok(false);
             }
         }
@@ -187,8 +192,15 @@ impl Context {
         Ok(deps)
     }
 
-    /// If the package is active returns the "age" (len of activations) when it was added
-    pub fn is_active(&self, id: PackageId) -> Option<usize> {
+    /// Returns the `ContextAge` of this `Context`.
+    /// For now we use (len of activations) as the age.
+    /// See the `ContextAge` docs for more details.
+    pub fn age(&self) -> ContextAge {
+        self.activations.len()
+    }
+
+    /// If the package is active returns the `ContextAge` when it was added
+    pub fn is_active(&self, id: PackageId) -> Option<ContextAge> {
         self.activations
             .get(&id.as_activations_key())
             .and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None })
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index fe582cb2147..aa7b4d903a5 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -976,7 +976,7 @@ fn find_candidate(
         // make any progress. As a result if we hit this condition we can
         // completely skip this backtrack frame and move on to the next.
         if let Some(age) = age {
-            if frame.context.activations.len() > age {
+            if frame.context.age() > age {
                 trace!(
                     "{} = \"{}\" skip as not solving {}: {:?}",
                     frame.dep.package_name(),
diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs
index 452032597c2..7c7c477cda6 100644
--- a/src/cargo/core/resolver/types.rs
+++ b/src/cargo/core/resolver/types.rs
@@ -425,6 +425,10 @@ impl ConflictReason {
     }
 }
 
+/// A list of packages that have gotten in the way of resolving a dependency.
+/// If resolving a dependency fails then this represents an incompatibility,
+/// that dependency will never be resolve while all of these packages are active.
+/// This is useless if the packages can't be simultaneously activated for other reasons.
 pub type ConflictMap = BTreeMap<PackageId, ConflictReason>;
 
 pub struct RcVecIter<T> {

From 1dd8e56c5d2861349ae5f9f414760f36c9d29a7d Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Wed, 27 Mar 2019 20:10:57 -0400
Subject: [PATCH 10/11] one more comment

---
 src/cargo/core/resolver/context.rs | 8 +++++---
 src/cargo/core/resolver/mod.rs     | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs
index cea11a2667e..8ae41851287 100644
--- a/src/cargo/core/resolver/context.rs
+++ b/src/cargo/core/resolver/context.rs
@@ -56,10 +56,12 @@ pub struct Context {
 /// When backtracking it can be useful to know how far back to go.
 /// The `ContextAge` of a `Context` is a monotonically increasing counter of the number
 /// of decisions made to get to this state.
-/// Several structures store the `ContextAge` when it was added, this lets use jump back.
+/// Several structures store the `ContextAge` when it was added, that gets use in jump back.
 pub type ContextAge = usize;
 
-/// find the activated version of a crate based on the name, source, and semver compatibility
+/// Find the activated version of a crate based on the name, source, and semver compatibility.
+/// By storing this in a hash map we ensure that there is only one
+/// semver compatible version of each crate.
 /// This all so stores the `ContextAge`.
 pub type Activations =
     im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), (Summary, ContextAge)>;
@@ -208,7 +210,7 @@ impl Context {
 
     /// Checks whether all of `parent` and the keys of `conflicting activations`
     /// are still active.
-    /// If so returns the "age" (len of activations) when the newest one was added.
+    /// If so returns the `ContextAge` when the newest one was added.
     pub fn is_conflicting(
         &self,
         parent: Option<PackageId>,
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index aa7b4d903a5..5a4517bb0bd 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -870,7 +870,7 @@ fn generalize_conflicting(
     if conflicting_activations.is_empty() {
         return None;
     }
-    // We need to determine the "age" that this `conflicting_activations` will jump to, and why.
+    // We need to determine the `ContextAge` that this `conflicting_activations` will jump to, and why.
     let (jumpback_critical_age, jumpback_critical_id) = conflicting_activations
         .keys()
         .map(|&c| (cx.is_active(c).expect("not currently active!?"), c))

From 91b5a9d323145e7c5787f1fa4562b91e2a0acb43 Mon Sep 17 00:00:00 2001
From: Eh2406 <YeomanYaacov@gmail.com>
Date: Sat, 30 Mar 2019 15:08:53 -0400
Subject: [PATCH 11/11] consistently refer to `backtracking` instead of `jump
 back`, `backjump`, or `jumpback`

---
 src/cargo/core/resolver/context.rs |  3 ++-
 src/cargo/core/resolver/mod.rs     | 27 +++++++++++++++------------
 src/cargo/core/resolver/types.rs   |  6 +++---
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs
index 8ae41851287..8a704d864cc 100644
--- a/src/cargo/core/resolver/context.rs
+++ b/src/cargo/core/resolver/context.rs
@@ -56,7 +56,8 @@ pub struct Context {
 /// When backtracking it can be useful to know how far back to go.
 /// The `ContextAge` of a `Context` is a monotonically increasing counter of the number
 /// of decisions made to get to this state.
-/// Several structures store the `ContextAge` when it was added, that gets use in jump back.
+/// Several structures store the `ContextAge` when it was added,
+/// to be used in `find_candidate` for backtracking.
 pub type ContextAge = usize;
 
 /// Find the activated version of a crate based on the name, source, and semver compatibility.
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index 5a4517bb0bd..5c6e240328b 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -855,7 +855,7 @@ impl RemainingCandidates {
     }
 }
 
-/// Attempts to find a new conflict that allows a bigger backjump then the input one.
+/// Attempts to find a new conflict that allows a `find_candidate` feather then the input one.
 /// It will add the new conflict to the cache if one is found.
 ///
 /// Panics if the input conflict is not all active in `cx`.
@@ -871,28 +871,28 @@ fn generalize_conflicting(
         return None;
     }
     // We need to determine the `ContextAge` that this `conflicting_activations` will jump to, and why.
-    let (jumpback_critical_age, jumpback_critical_id) = conflicting_activations
+    let (backtrack_critical_age, backtrack_critical_id) = conflicting_activations
         .keys()
         .map(|&c| (cx.is_active(c).expect("not currently active!?"), c))
         .max()
         .unwrap();
-    let jumpback_critical_reason: ConflictReason =
-        conflicting_activations[&jumpback_critical_id].clone();
+    let backtrack_critical_reason: ConflictReason =
+        conflicting_activations[&backtrack_critical_id].clone();
 
     if cx
         .parents
-        .is_path_from_to(&parent.package_id(), &jumpback_critical_id)
+        .is_path_from_to(&parent.package_id(), &backtrack_critical_id)
     {
         // We are a descendant of the trigger of the problem.
         // The best generalization of this is to let things bubble up
-        // and let `jumpback_critical_id` figure this out.
+        // and let `backtrack_critical_id` figure this out.
         return None;
     }
     // What parents does that critical activation have
     for (critical_parent, critical_parents_deps) in
-        cx.parents.edges(&jumpback_critical_id).filter(|(p, _)| {
+        cx.parents.edges(&backtrack_critical_id).filter(|(p, _)| {
             // it will only help backjump further if it is older then the critical_age
-            cx.is_active(*p).expect("parent not currently active!?") < jumpback_critical_age
+            cx.is_active(*p).expect("parent not currently active!?") < backtrack_critical_age
         })
     {
         for critical_parents_dep in critical_parents_deps.iter() {
@@ -906,14 +906,17 @@ fn generalize_conflicting(
                 .rev() // the last one to be tried is the least likely to be in the cache, so start with that.
                 .all(|other| {
                     let mut con = conflicting_activations.clone();
-                    con.remove(&jumpback_critical_id);
-                    con.insert(other.summary.package_id(), jumpback_critical_reason.clone());
+                    con.remove(&backtrack_critical_id);
+                    con.insert(
+                        other.summary.package_id(),
+                        backtrack_critical_reason.clone(),
+                    );
                     past_conflicting_activations.contains(&dep, &con)
                 })
             {
                 let mut con = conflicting_activations.clone();
-                con.remove(&jumpback_critical_id);
-                con.insert(*critical_parent, jumpback_critical_reason);
+                con.remove(&backtrack_critical_id);
+                con.insert(*critical_parent, backtrack_critical_reason);
                 past_conflicting_activations.insert(&dep, &con);
                 return Some(con);
             }
diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs
index 7c7c477cda6..b399a51c695 100644
--- a/src/cargo/core/resolver/types.rs
+++ b/src/cargo/core/resolver/types.rs
@@ -403,9 +403,9 @@ pub enum ConflictReason {
     /// candidate we're activating didn't actually have the feature `foo`.
     MissingFeatures(String),
 
-    // TODO: needs more info for errors maneges
-    // TODO: needs more info for back jumping
-    /// pub dep errore
+    // TODO: needs more info for `activation_error`
+    // TODO: needs more info for `find_candidate`
+    /// pub dep error
     PublicDependency,
 }