address nits

Author: Eh2406

src/cargo/core/resolver/context.rs

@@ -9,6 +9,7 @@ use log::debug;
 
 use crate::core::interning::InternedString;
 use crate::core::{Dependency, FeatureValue, PackageId, SourceId, Summary};
+use crate::util::config::Config;
 use crate::util::CargoResult;
 use crate::util::Graph;
 
@@ -217,7 +218,7 @@ impl Context {
         parent: Option<&Summary>,
         s: &'b Summary,
         method: &'b Method<'_>,
-        config: Option<&crate::util::config::Config>,
+        config: Option<&Config>,
     ) -> ActivateResult<Vec<(Dependency, Vec<InternedString>)>> {
         let dev_deps = match *method {
             Method::Everything => true,
@@ -329,14 +330,16 @@ impl Context {
     }
 
     pub fn graph(&self) -> Graph<PackageId, Vec<Dependency>> {
-        let mut graph = Graph::new();
+        let mut graph: Graph<PackageId, Vec<Dependency>> = Graph::new();
         self.activations
             .values()
             .for_each(|(r, _)| graph.add(r.package_id()));
         for i in self.parents.iter() {
             graph.add(*i);
             for (o, e) in self.parents.edges(i) {
-                *graph.link(*o, *i) = e.to_vec();
+                let old_link = graph.link(*o, *i);
+                debug_assert!(old_link.is_empty());
+                *old_link = e.to_vec();
             }
         }
         graph

src/cargo/core/resolver/mod.rs

@@ -1088,7 +1088,11 @@ fn check_duplicate_pkgs_in_lockfile(resolve: &Resolve) -> CargoResult<()> {
     Ok(())
 }
 
-/// re-run all used resolve_features so it can print warnings
+/// Re-run all used `resolve_features` so it can print warnings.
+/// Within the resolution loop we do not pass a `config` to `resolve_features`
+/// this tells it not to print warnings. Hear we do pass `config` triggering it to print them.
+/// This is done as the resolution is NP-Hard so the loop can potentially call `resolve features`
+/// an exponential number of times, but this pass is linear in the number of dependencies.
 fn emit_warnings(
     cx: &Context,
     resolve: &Resolve,
@@ -1098,22 +1102,26 @@ fn emit_warnings(
     let mut new_cx = cx.clone();
     new_cx.resolve_features = im_rc::HashMap::new();
     let mut features_from_dep = HashMap::new();
-    for (summery, method) in summaries {
+    for (summary, method) in summaries {
         for (dep, features) in new_cx
-            .resolve_features(None, summery, &method, config)
-            .expect("can not resolve_features for a required summery")
+            .resolve_features(None, summary, &method, config)
+            .expect("can not resolve_features for a required summary")
         {
-            features_from_dep.insert((summery.package_id(), dep), features);
+            features_from_dep.insert((summary.package_id(), dep), features);
         }
     }
-    for summery in resolve.sort().iter().rev().map(|id| {
+    // crates enable features for their dependencies.
+    // So by iterating reverse topologically we process all of the parents before each child.
+    // Then we know all the needed features for each crate.
+    let topological_sort = resolve.sort();
+    for summary in topological_sort.iter().rev().map(|id| {
         cx.activations
             .get(&id.as_activations_key())
             .expect("id in dependency graph but not in activations")
             .0
             .clone()
     }) {
-        for (parent, deps) in cx.parents.edges(&summery.package_id()) {
+        for (parent, deps) in cx.parents.edges(&summary.package_id()) {
             for dep in deps.iter() {
                 let features = features_from_dep
                     .remove(&(*parent, dep.clone()))
@@ -1125,10 +1133,10 @@ fn emit_warnings(
                     uses_default_features: dep.uses_default_features(),
                 };
                 for (dep, features) in new_cx
-                    .resolve_features(None, &summery, &method, config)
+                    .resolve_features(None, &summary, &method, config)
                     .expect("can not resolve_features for a used dep")
                 {
-                    features_from_dep.insert((summery.package_id(), dep), features);
+                    features_from_dep.insert((summary.package_id(), dep), features);
                 }
             }
         }

src/cargo/core/resolver/types.rs

@@ -219,8 +219,21 @@ impl<'a> RegistryQueryer<'a> {
                 debug!("\t{} => {}", dep.package_name(), dep.version_req());
             }
             if let Some(r) = &replace {
-                self.used_replacements
-                    .insert(summary.package_id(), r.package_id());
+                if let Some(old) = self
+                    .used_replacements
+                    .insert(summary.package_id(), r.package_id())
+                {
+                    debug_assert_eq!(
+                        r.package_id(),
+                        old,
+                        "we are inconsistent about witch replacement is used for {:?}, \
+                         one time we used {:?}, \
+                         now we are adding {:?}.",
+                        summary.package_id(),
+                        old,
+                        r.package_id()
+                    )
+                }
             }
 
             candidate.replace = replace;

tests/testsuite/resolve.rs

@@ -111,13 +111,13 @@ proptest! {
     ) {
         let reg = registry(input.clone());
         let mut removed_input = input.clone();
-        for (summery_idx, dep_idx) in indexes_to_remove {
+        for (summary_idx, dep_idx) in indexes_to_remove {
             if !removed_input.is_empty() {
-                let summery_idx = summery_idx.index(removed_input.len());
-                let deps = removed_input[summery_idx].dependencies();
+                let summary_idx = summary_idx.index(removed_input.len());
+                let deps = removed_input[summary_idx].dependencies();
                 if !deps.is_empty() {
-                    let new = remove_dep(&removed_input[summery_idx], dep_idx.index(deps.len()));
-                    removed_input[summery_idx] = new;
+                    let new = remove_dep(&removed_input[summary_idx], dep_idx.index(deps.len()));
+                    removed_input[summary_idx] = new;
                 }
             }
         }