Auto merge of #5428 - matklad:graph-with-edges, r=alexcrichton

Store dependencies as edges of the graph

r? @alexcrichton

src/cargo/core/resolver/context.rs

@@ -269,25 +269,19 @@ impl Context {
         replacements
     }
 
-    pub fn graph(&self)
-        -> (Graph<PackageId>, HashMap<(PackageId, PackageId), Vec<Dependency>>)
-    {
-        let mut graph = Graph::new();
-        let mut deps = HashMap::new();
+    pub fn graph(&self) -> Graph<PackageId, Vec<Dependency>> {
+        let mut graph: Graph<PackageId, Vec<Dependency>> = Graph::new();
         let mut cur = &self.resolve_graph;
         while let Some(ref node) = cur.head {
             match node.0 {
                 GraphNode::Add(ref p) => graph.add(p.clone()),
                 GraphNode::Link(ref a, ref b, ref dep) => {
-                    graph.link(a.clone(), b.clone());
-                    deps.entry((a.clone(), b.clone()))
-                        .or_insert(Vec::new())
-                        .push(dep.clone());
+                    graph.link(a.clone(), b.clone()).push(dep.clone());
                 }
             }
             cur = &node.1;
         }
-        (graph, deps)
+        graph
     }
 }
 

src/cargo/core/resolver/encode.rs

@@ -179,7 +179,6 @@ impl EncodableResolve {
 
         Ok(Resolve::new(
             g,
-            HashMap::new(),
             replacements,
             HashMap::new(),
             checksums,

src/cargo/core/resolver/mod.rs

@@ -122,16 +122,13 @@ pub fn resolve(
     let mut registry = RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions);
     let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
 
-    let (graph, deps) = cx.graph();
-
     let mut cksums = HashMap::new();
     for summary in cx.activations.values().flat_map(|v| v.iter()) {
         let cksum = summary.checksum().map(|s| s.to_string());
         cksums.insert(summary.package_id().clone(), cksum);
     }
     let resolve = Resolve::new(
-        graph,
-        deps,
+        cx.graph(),
         cx.resolve_replacements(),
         cx.resolve_features
             .iter()
@@ -867,7 +864,7 @@ fn activation_error(
     candidates: &[Candidate],
     config: Option<&Config>,
 ) -> CargoError {
-    let (graph, _) = cx.graph();
+    let graph = cx.graph();
     if !candidates.is_empty() {
         let mut msg = format!("failed to select a version for `{}`.", dep.name());
         msg.push_str("\n    ... required by ");

src/cargo/core/resolver/resolve.rs

@@ -18,8 +18,7 @@ use super::encode::Metadata;
 /// for each package.
 #[derive(PartialEq)]
 pub struct Resolve {
-    graph: Graph<PackageId>,
-    dependencies: HashMap<(PackageId, PackageId), Vec<Dependency>>,
+    graph: Graph<PackageId, Vec<Dependency>>,
     replacements: HashMap<PackageId, PackageId>,
     reverse_replacements: HashMap<PackageId, PackageId>,
     empty_features: HashSet<String>,
@@ -31,8 +30,7 @@ pub struct Resolve {
 
 impl Resolve {
     pub fn new(
-        graph: Graph<PackageId>,
-        dependencies: HashMap<(PackageId, PackageId), Vec<Dependency>>,
+        graph: Graph<PackageId, Vec<Dependency>>,
         replacements: HashMap<PackageId, PackageId>,
         features: HashMap<PackageId, HashSet<String>>,
         checksums: HashMap<PackageId, Option<String>>,
@@ -45,7 +43,6 @@ impl Resolve {
             .collect();
         Resolve {
             graph,
-            dependencies,
             replacements,
             features,
             checksums,
@@ -162,14 +159,13 @@ unable to verify that `{0}` is the same as when the lockfile was generated
         Ok(())
     }
 
-    pub fn iter(&self) -> Nodes<PackageId> {
+    pub fn iter(&self) -> Nodes<PackageId, Vec<Dependency>> {
         self.graph.iter()
     }
 
     pub fn deps(&self, pkg: &PackageId) -> Deps {
         Deps {
             edges: self.graph.edges(pkg),
-            id: pkg.clone(),
             resolve: self,
         }
     }
@@ -225,11 +221,11 @@ unable to verify that `{0}` is the same as when the lockfile was generated
         // that's where the dependency originates from, and we only replace
         // targets of dependencies not the originator.
         if let Some(replace) = self.reverse_replacements.get(to) {
-            if let Some(deps) = self.dependencies.get(&(from.clone(), replace.clone())) {
+            if let Some(deps) = self.graph.edge(from, replace) {
                 return deps;
             }
         }
-        match self.dependencies.get(&(from.clone(), to.clone())) {
+        match self.graph.edge(from, to) {
             Some(ret) => ret,
             None => panic!("no Dependency listed for `{}` => `{}`", from, to),
         }
@@ -248,18 +244,16 @@ impl fmt::Debug for Resolve {
 }
 
 pub struct Deps<'a> {
-    edges: Option<Edges<'a, PackageId>>,
-    id: PackageId,
+    edges: Option<Edges<'a, PackageId, Vec<Dependency>>>,
     resolve: &'a Resolve,
 }
 
 impl<'a> Iterator for Deps<'a> {
     type Item = (&'a PackageId, &'a [Dependency]);
 
     fn next(&mut self) -> Option<(&'a PackageId, &'a [Dependency])> {
-        let id = self.edges.as_mut()?.next()?;
+        let (id, deps) = self.edges.as_mut()?.next()?;
         let id_ret = self.resolve.replacement(id).unwrap_or(id);
-        let deps = &self.resolve.dependencies[&(self.id.clone(), id.clone())];
         Some((id_ret, deps))
     }
 
@@ -274,14 +268,14 @@ impl<'a> Iterator for Deps<'a> {
 impl<'a> ExactSizeIterator for Deps<'a> {}
 
 pub struct DepsNotReplaced<'a> {
-    edges: Option<Edges<'a, PackageId>>,
+    edges: Option<Edges<'a, PackageId, Vec<Dependency>>>,
 }
 
 impl<'a> Iterator for DepsNotReplaced<'a> {
     type Item = &'a PackageId;
 
     fn next(&mut self) -> Option<&'a PackageId> {
-        self.edges.as_mut().and_then(|e| e.next())
+        Some(self.edges.as_mut()?.next()?.0)
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {

src/cargo/util/graph.rs

@@ -1,46 +1,44 @@
 use std::fmt;
 use std::hash::Hash;
-use std::collections::hash_set::{HashSet, Iter};
-use std::collections::hash_map::{HashMap, Keys};
+use std::collections::hash_map::{HashMap, Iter, Keys};
 
-pub struct Graph<N> {
-    nodes: HashMap<N, HashSet<N>>,
+pub struct Graph<N, E> {
+    nodes: HashMap<N, HashMap<N, E>>,
 }
 
 enum Mark {
     InProgress,
     Done,
 }
 
-pub type Nodes<'a, N> = Keys<'a, N, HashSet<N>>;
-pub type Edges<'a, N> = Iter<'a, N>;
+pub type Nodes<'a, N, E> = Keys<'a, N, HashMap<N, E>>;
+pub type Edges<'a, N, E> = Iter<'a, N, E>;
 
-impl<N: Eq + Hash + Clone> Graph<N> {
-    pub fn new() -> Graph<N> {
+impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
+    pub fn new() -> Graph<N, E> {
         Graph {
             nodes: HashMap::new(),
         }
     }
 
     pub fn add(&mut self, node: N) {
-        self.nodes
-            .entry(node)
-            .or_insert_with(HashSet::new);
+        self.nodes.entry(node).or_insert_with(HashMap::new);
     }
 
-    pub fn link(&mut self, node: N, child: N) {
+    pub fn link(&mut self, node: N, child: N) -> &mut E {
         self.nodes
             .entry(node)
-            .or_insert_with(HashSet::new)
-            .insert(child);
+            .or_insert_with(HashMap::new)
+            .entry(child)
+            .or_insert_with(Default::default)
     }
 
-    pub fn get_nodes(&self) -> &HashMap<N, HashSet<N>> {
-        &self.nodes
+    pub fn edge(&self, from: &N, to: &N) -> Option<&E> {
+        self.nodes.get(from)?.get(to)
     }
 
-    pub fn edges(&self, node: &N) -> Option<Edges<N>> {
-        self.nodes.get(node).map(|set| set.iter())
+    pub fn edges(&self, from: &N) -> Option<Edges<N, E>> {
+        self.nodes.get(from).map(|set| set.iter())
     }
 
     pub fn sort(&self) -> Option<Vec<N>> {
@@ -61,15 +59,15 @@ impl<N: Eq + Hash + Clone> Graph<N> {
 
         marks.insert(node.clone(), Mark::InProgress);
 
-        for child in &self.nodes[node] {
+        for child in self.nodes[node].keys() {
             self.visit(child, dst, marks);
         }
 
         dst.push(node.clone());
         marks.insert(node.clone(), Mark::Done);
     }
 
-    pub fn iter(&self) -> Nodes<N> {
+    pub fn iter(&self) -> Nodes<N, E> {
         self.nodes.keys()
     }
 
@@ -81,12 +79,12 @@ impl<N: Eq + Hash + Clone> Graph<N> {
         // it's used for!
         let mut result = vec![pkg];
         let first_pkg_depending_on = |pkg: &N, res: &[&N]| {
-            self.get_nodes()
+            self.nodes
                 .iter()
-                .filter(|&(_node, adjacent)| adjacent.contains(pkg))
+                .filter(|&(_node, adjacent)| adjacent.contains_key(pkg))
                 // Note that we can have "cycles" introduced through dev-dependency
                 // edges, so make sure we don't loop infinitely.
-                .filter(|&(_node, _)| !res.contains(&_node))
+                .filter(|&(node, _)| !res.contains(&node))
                 .next()
                 .map(|p| p.0)
         };
@@ -98,20 +96,20 @@ impl<N: Eq + Hash + Clone> Graph<N> {
     }
 }
 
-impl<N: Eq + Hash + Clone> Default for Graph<N> {
-    fn default() -> Graph<N> {
+impl<N: Eq + Hash + Clone, E: Default> Default for Graph<N, E> {
+    fn default() -> Graph<N, E> {
         Graph::new()
     }
 }
 
-impl<N: fmt::Display + Eq + Hash> fmt::Debug for Graph<N> {
+impl<N: fmt::Display + Eq + Hash, E> fmt::Debug for Graph<N, E> {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         writeln!(fmt, "Graph {{")?;
 
         for (n, e) in &self.nodes {
             writeln!(fmt, "  - {}", n)?;
 
-            for n in e.iter() {
+            for n in e.keys() {
                 writeln!(fmt, "    - {}", n)?;
             }
         }
@@ -122,15 +120,15 @@ impl<N: fmt::Display + Eq + Hash> fmt::Debug for Graph<N> {
     }
 }
 
-impl<N: Eq + Hash> PartialEq for Graph<N> {
-    fn eq(&self, other: &Graph<N>) -> bool {
+impl<N: Eq + Hash, E: Eq> PartialEq for Graph<N, E> {
+    fn eq(&self, other: &Graph<N, E>) -> bool {
         self.nodes.eq(&other.nodes)
     }
 }
-impl<N: Eq + Hash> Eq for Graph<N> {}
+impl<N: Eq + Hash, E: Eq> Eq for Graph<N, E> {}
 
-impl<N: Eq + Hash + Clone> Clone for Graph<N> {
-    fn clone(&self) -> Graph<N> {
+impl<N: Eq + Hash + Clone, E: Clone> Clone for Graph<N, E> {
+    fn clone(&self) -> Graph<N, E> {
         Graph {
             nodes: self.nodes.clone(),
         }