From 606cd71c381681a3fa744d83065fe254a33f327d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Feb 2025 13:43:05 -0600 Subject: [PATCH 1/6] refactor(tree): Don't look up nodes directly --- src/cargo/ops/tree/graph.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index da2f27708e8..15baa97200a 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -126,8 +126,7 @@ impl<'a> Graph<'a> { let from_index = self.nodes.len(); self.nodes.push(node); self.edges.push(Edges::new()); - self.index - .insert(self.nodes[from_index].clone(), from_index); + self.index.insert(self.node(from_index).clone(), from_index); from_index } @@ -136,7 +135,7 @@ impl<'a> Graph<'a> { let edges = self.edges[from].of_kind(kind); // Created a sorted list for consistent output. let mut edges = edges.to_owned(); - edges.sort_unstable_by(|a, b| self.nodes[a.node()].cmp(&self.nodes[b.node()])); + edges.sort_unstable_by(|a, b| self.node(a.node()).cmp(&self.node(b.node()))); edges } @@ -173,8 +172,8 @@ impl<'a> Graph<'a> { } fn package_id_for_index(&self, index: usize) -> PackageId { - match self.nodes[index] { - Node::Package { package_id, .. } => package_id, + match self.node(index) { + Node::Package { package_id, .. } => *package_id, Node::Feature { .. } => panic!("unexpected feature node"), } } @@ -509,7 +508,7 @@ fn add_feature( to: Edge, ) -> (bool, usize) { // `to` *must* point to a package node. - assert!(matches! {graph.nodes[to.node()], Node::Package{..}}); + assert!(matches! {graph.node(to.node()), Node::Package{..}}); let node = Node::Feature { node_index: to.node(), name, From 9700a89040113ca29799edff35918ce98e2dfb32 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Feb 2025 13:45:05 -0600 Subject: [PATCH 2/6] refactor(tree): Be more specific edges is filtered --- src/cargo/ops/tree/graph.rs | 2 +- src/cargo/ops/tree/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index 15baa97200a..9ca9e080374 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -131,7 +131,7 @@ impl<'a> Graph<'a> { } /// Returns a list of nodes the given node index points to for the given kind. - pub fn edges(&self, from: usize, kind: &EdgeKind) -> Vec { + pub fn edges_of_kind(&self, from: usize, kind: &EdgeKind) -> Vec { let edges = self.edges[from].of_kind(kind); // Created a sorted list for consistent output. let mut edges = edges.to_owned(); diff --git a/src/cargo/ops/tree/mod.rs b/src/cargo/ops/tree/mod.rs index 6488e6a132e..8f5cda5321f 100644 --- a/src/cargo/ops/tree/mod.rs +++ b/src/cargo/ops/tree/mod.rs @@ -383,7 +383,7 @@ fn print_dependencies<'a>( print_stack: &mut Vec, kind: &EdgeKind, ) { - let deps = graph.edges(node_index, kind); + let deps = graph.edges_of_kind(node_index, kind); if deps.is_empty() { return; } From 0659425f380f5db437c0275a59040bb0d38b2fdd Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Feb 2025 13:46:29 -0600 Subject: [PATCH 3/6] refactor(tree): Don't look up edges directly --- src/cargo/ops/tree/graph.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index 9ca9e080374..388c451376b 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -132,16 +132,24 @@ impl<'a> Graph<'a> { /// Returns a list of nodes the given node index points to for the given kind. pub fn edges_of_kind(&self, from: usize, kind: &EdgeKind) -> Vec { - let edges = self.edges[from].of_kind(kind); + let edges = self.edges(from).of_kind(kind); // Created a sorted list for consistent output. let mut edges = edges.to_owned(); edges.sort_unstable_by(|a, b| self.node(a.node()).cmp(&self.node(b.node()))); edges } + fn edges(&self, from: usize) -> &Edges { + &self.edges[from] + } + + fn edges_mut(&mut self, from: usize) -> &mut Edges { + &mut self.edges[from] + } + /// Returns `true` if the given node has any outgoing edges. pub fn has_outgoing_edges(&self, index: usize) -> bool { - !self.edges[index].is_empty() + !self.edges(index).is_empty() } /// Gets a node by index. @@ -207,13 +215,13 @@ impl<'a> Graph<'a> { let new_from = new_graph.add_node(node); remap[index] = Some(new_from); // Visit dependencies. - for edge in graph.edges[index].all() { + for edge in graph.edges(index).all() { let new_to_index = visit(graph, new_graph, remap, edge.node()); let new_edge = Edge { kind: edge.kind(), node: new_to_index, }; - new_graph.edges[new_from].add_edge(new_edge); + new_graph.edges_mut(new_from).add_edge(new_edge); } new_from } @@ -473,10 +481,10 @@ fn add_pkg( } if !dep.uses_default_features() && dep.features().is_empty() { // No features, use a direct connection. - graph.edges[from_index].add_edge(new_edge); + graph.edges_mut(from_index).add_edge(new_edge); } } else { - graph.edges[from_index].add_edge(new_edge); + graph.edges_mut(from_index).add_edge(new_edge); } } } @@ -522,13 +530,13 @@ fn add_feature( kind: to.kind(), node: node_index, }; - graph.edges[from].add_edge(from_edge); + graph.edges_mut(from).add_edge(from_edge); } let to_edge = Edge { kind: EdgeKind::Feature, node: to.node(), }; - graph.edges[node_index].add_edge(to_edge); + graph.edges_mut(node_index).add_edge(to_edge); (missing, node_index) } From 2e007b5584cfb1ff4cedd038468bc8901b58c94e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 25 Feb 2025 11:25:13 -0600 Subject: [PATCH 4/6] refactor(tree): Use a newtype to track node indexes The primary goal is to make the code more type safe / easier to follow. This also can allow tracking debug information. --- src/cargo/ops/tree/format/mod.rs | 6 +- src/cargo/ops/tree/graph.rs | 99 ++++++++++++++++++-------------- src/cargo/ops/tree/mod.rs | 16 +++--- 3 files changed, 66 insertions(+), 55 deletions(-) diff --git a/src/cargo/ops/tree/format/mod.rs b/src/cargo/ops/tree/format/mod.rs index d0b55b74d3e..f9c170634a8 100644 --- a/src/cargo/ops/tree/format/mod.rs +++ b/src/cargo/ops/tree/format/mod.rs @@ -3,7 +3,7 @@ use std::fmt; use anyhow::{bail, Error}; use self::parse::{Parser, RawChunk}; -use super::{Graph, Node}; +use super::{Graph, Node, NodeId}; mod parse; @@ -41,7 +41,7 @@ impl Pattern { Ok(Pattern(chunks)) } - pub fn display<'a>(&'a self, graph: &'a Graph<'a>, node_index: usize) -> Display<'a> { + pub fn display<'a>(&'a self, graph: &'a Graph<'a>, node_index: NodeId) -> Display<'a> { Display { pattern: self, graph, @@ -53,7 +53,7 @@ impl Pattern { pub struct Display<'a> { pattern: &'a Pattern, graph: &'a Graph<'a>, - node_index: usize, + node_index: NodeId, } impl<'a> fmt::Display for Display<'a> { diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index 388c451376b..bdfe5b86652 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -10,6 +10,17 @@ use crate::util::interning::{InternedString, INTERNED_DEFAULT}; use crate::util::CargoResult; use std::collections::{HashMap, HashSet}; +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct NodeId { + index: usize, +} + +impl NodeId { + fn with_index(index: usize) -> Self { + Self { index } + } +} + #[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] pub enum Node { Package { @@ -20,7 +31,7 @@ pub enum Node { }, Feature { /// Index of the package node this feature is for. - node_index: usize, + node_index: NodeId, /// Name of the feature. name: InternedString, }, @@ -29,7 +40,7 @@ pub enum Node { #[derive(Debug, Copy, Hash, Eq, Clone, PartialEq)] pub struct Edge { kind: EdgeKind, - node: usize, + node: NodeId, } impl Edge { @@ -37,7 +48,7 @@ impl Edge { self.kind } - pub fn node(&self) -> usize { + pub fn node(&self) -> NodeId { self.node } } @@ -94,19 +105,19 @@ pub struct Graph<'a> { /// sync. edges: Vec, /// Index maps a node to an index, for fast lookup. - index: HashMap, + index: HashMap, /// Map for looking up packages. package_map: HashMap, /// Set of indexes of feature nodes that were added via the command-line. /// /// For example `--features foo` will mark the "foo" node here. - cli_features: HashSet, + cli_features: HashSet, /// Map of dependency names, used for building internal feature map for /// `dep_name/feat_name` syntax. /// /// Key is the index of a package node, value is a map of `dep_name` to a /// set of `(pkg_node_index, is_optional)`. - dep_name_map: HashMap>>, + dep_name_map: HashMap>>, } impl<'a> Graph<'a> { @@ -122,8 +133,8 @@ impl<'a> Graph<'a> { } /// Adds a new node to the graph, returning its new index. - fn add_node(&mut self, node: Node) -> usize { - let from_index = self.nodes.len(); + fn add_node(&mut self, node: Node) -> NodeId { + let from_index = NodeId::with_index(self.nodes.len()); self.nodes.push(node); self.edges.push(Edges::new()); self.index.insert(self.node(from_index).clone(), from_index); @@ -131,7 +142,7 @@ impl<'a> Graph<'a> { } /// Returns a list of nodes the given node index points to for the given kind. - pub fn edges_of_kind(&self, from: usize, kind: &EdgeKind) -> Vec { + pub fn edges_of_kind(&self, from: NodeId, kind: &EdgeKind) -> Vec { let edges = self.edges(from).of_kind(kind); // Created a sorted list for consistent output. let mut edges = edges.to_owned(); @@ -139,27 +150,27 @@ impl<'a> Graph<'a> { edges } - fn edges(&self, from: usize) -> &Edges { - &self.edges[from] + fn edges(&self, from: NodeId) -> &Edges { + &self.edges[from.index] } - fn edges_mut(&mut self, from: usize) -> &mut Edges { - &mut self.edges[from] + fn edges_mut(&mut self, from: NodeId) -> &mut Edges { + &mut self.edges[from.index] } /// Returns `true` if the given node has any outgoing edges. - pub fn has_outgoing_edges(&self, index: usize) -> bool { + pub fn has_outgoing_edges(&self, index: NodeId) -> bool { !self.edges(index).is_empty() } /// Gets a node by index. - pub fn node(&self, index: usize) -> &Node { - &self.nodes[index] + pub fn node(&self, index: NodeId) -> &Node { + &self.nodes[index.index] } /// Given a slice of `PackageIds`, returns the indexes of all nodes that match. - pub fn indexes_from_ids(&self, package_ids: &[PackageId]) -> Vec { - let mut result: Vec<(&Node, usize)> = self + pub fn indexes_from_ids(&self, package_ids: &[PackageId]) -> Vec { + let mut result: Vec<(&Node, NodeId)> = self .nodes .iter() .enumerate() @@ -167,7 +178,7 @@ impl<'a> Graph<'a> { Node::Package { package_id, .. } => package_ids.contains(package_id), _ => false, }) - .map(|(i, node)| (node, i)) + .map(|(i, node)| (node, NodeId::with_index(i))) .collect(); // Sort for consistent output (the same command should always return // the same output). "unstable" since nodes should always be unique. @@ -179,7 +190,7 @@ impl<'a> Graph<'a> { self.package_map[&id] } - fn package_id_for_index(&self, index: usize) -> PackageId { + fn package_id_for_index(&self, index: NodeId) -> PackageId { match self.node(index) { Node::Package { package_id, .. } => *package_id, Node::Feature { .. } => panic!("unexpected feature node"), @@ -188,32 +199,32 @@ impl<'a> Graph<'a> { /// Returns `true` if the given feature node index is a feature enabled /// via the command-line. - pub fn is_cli_feature(&self, index: usize) -> bool { + pub fn is_cli_feature(&self, index: NodeId) -> bool { self.cli_features.contains(&index) } /// Returns a new graph by removing all nodes not reachable from the /// given nodes. - pub fn from_reachable(&self, roots: &[usize]) -> Graph<'a> { + pub fn from_reachable(&self, roots: &[NodeId]) -> Graph<'a> { // Graph built with features does not (yet) support --duplicates. assert!(self.dep_name_map.is_empty()); let mut new_graph = Graph::new(self.package_map.clone()); // Maps old index to new index. None if not yet visited. - let mut remap: Vec> = vec![None; self.nodes.len()]; + let mut remap: Vec> = vec![None; self.nodes.len()]; fn visit( graph: &Graph<'_>, new_graph: &mut Graph<'_>, - remap: &mut Vec>, - index: usize, - ) -> usize { - if let Some(new_index) = remap[index] { + remap: &mut Vec>, + index: NodeId, + ) -> NodeId { + if let Some(new_index) = remap[index.index] { // Already visited. return new_index; } let node = graph.node(index).clone(); let new_from = new_graph.add_node(node); - remap[index] = Some(new_from); + remap[index.index] = Some(new_from); // Visit dependencies. for edge in graph.edges(index).all() { let new_to_index = visit(graph, new_graph, remap, edge.node()); @@ -241,9 +252,9 @@ impl<'a> Graph<'a> { for edge in node_edges.all() { let new_edge = Edge { kind: edge.kind(), - node: from_idx, + node: NodeId::with_index(from_idx), }; - new_edges[edge.node()].add_edge(new_edge); + new_edges[edge.node().index].add_edge(new_edge); } } self.edges = new_edges; @@ -251,22 +262,22 @@ impl<'a> Graph<'a> { /// Returns a list of nodes that are considered "duplicates" (same package /// name, with different versions/features/source/etc.). - pub fn find_duplicates(&self) -> Vec { + pub fn find_duplicates(&self) -> Vec { // Graph built with features does not (yet) support --duplicates. assert!(self.dep_name_map.is_empty()); - // Collect a map of package name to Vec<(&Node, usize)>. + // Collect a map of package name to Vec<(&Node, NodeId)>. let mut packages = HashMap::new(); for (i, node) in self.nodes.iter().enumerate() { if let Node::Package { package_id, .. } = node { packages .entry(package_id.name()) .or_insert_with(Vec::new) - .push((node, i)); + .push((node, NodeId::with_index(i))); } } - let mut dupes: Vec<(&Node, usize)> = packages + let mut dupes: Vec<(&Node, NodeId)> = packages .into_iter() .filter(|(_name, indexes)| { indexes @@ -356,7 +367,7 @@ fn add_pkg( target_data: &RustcTargetData<'_>, requested_kind: CompileKind, opts: &TreeOptions, -) -> usize { +) -> NodeId { let node_features = resolved_features.activated_features(package_id, features_for); let node_kind = match features_for { FeaturesFor::HostDep => CompileKind::Host, @@ -373,7 +384,7 @@ fn add_pkg( } let from_index = graph.add_node(node); // Compute the dep name map which is later used for foo/bar feature lookups. - let mut dep_name_map: HashMap> = HashMap::new(); + let mut dep_name_map: HashMap> = HashMap::new(); let mut deps: Vec<_> = resolve.deps(package_id).collect(); deps.sort_unstable_by_key(|(dep_id, _)| *dep_id); let show_all_targets = opts.target == super::Target::All; @@ -512,9 +523,9 @@ fn add_pkg( fn add_feature( graph: &mut Graph<'_>, name: InternedString, - from: Option, + from: Option, to: Edge, -) -> (bool, usize) { +) -> (bool, NodeId) { // `to` *must* point to a package node. assert!(matches! {graph.node(to.node()), Node::Package{..}}); let node = Node::Feature { @@ -547,7 +558,7 @@ fn add_feature( /// `--invert`. fn add_cli_features( graph: &mut Graph<'_>, - package_index: usize, + package_index: NodeId, cli_features: &CliFeatures, feature_map: &FeatureMap, ) { @@ -596,7 +607,7 @@ fn add_cli_features( "missing dep graph connection for CLI feature `{}` for member {:?}\n\ Please file a bug report at https://github.com/rust-lang/cargo/issues", fv, - graph.nodes.get(package_index) + graph.nodes.get(package_index.index) ); } }; @@ -626,7 +637,7 @@ fn add_cli_features( /// for every package. fn add_internal_features(graph: &mut Graph<'_>, resolve: &Resolve) { // Collect features already activated by dependencies or command-line. - let feature_nodes: Vec<(PackageId, usize, usize, InternedString)> = graph + let feature_nodes: Vec<(PackageId, NodeId, NodeId, InternedString)> = graph .nodes .iter() .enumerate() @@ -634,7 +645,7 @@ fn add_internal_features(graph: &mut Graph<'_>, resolve: &Resolve) { Node::Package { .. } => None, Node::Feature { node_index, name } => { let package_id = graph.package_id_for_index(*node_index); - Some((package_id, *node_index, i, *name)) + Some((package_id, *node_index, NodeId::with_index(i), *name)) } }) .collect(); @@ -660,8 +671,8 @@ fn add_feature_rec( resolve: &Resolve, feature_name: InternedString, package_id: PackageId, - from: usize, - package_index: usize, + from: NodeId, + package_index: NodeId, ) { let feature_map = resolve.summary(package_id).features(); let Some(fvs) = feature_map.get(&feature_name) else { diff --git a/src/cargo/ops/tree/mod.rs b/src/cargo/ops/tree/mod.rs index 8f5cda5321f..157f46f0fd0 100644 --- a/src/cargo/ops/tree/mod.rs +++ b/src/cargo/ops/tree/mod.rs @@ -16,7 +16,7 @@ use std::str::FromStr; mod format; mod graph; -pub use {graph::EdgeKind, graph::Node}; +pub use {graph::EdgeKind, graph::Node, graph::NodeId}; pub struct TreeOptions { pub cli_features: CliFeatures, @@ -240,7 +240,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<() fn print( ws: &Workspace<'_>, opts: &TreeOptions, - roots: Vec, + roots: Vec, pkgs_to_prune: &[PackageIdSpec], graph: &Graph<'_>, ) -> CargoResult<()> { @@ -292,16 +292,16 @@ fn print( fn print_node<'a>( ws: &Workspace<'_>, graph: &'a Graph<'_>, - node_index: usize, + node_index: NodeId, format: &Pattern, symbols: &Symbols, pkgs_to_prune: &[PackageIdSpec], prefix: Prefix, no_dedupe: bool, display_depth: DisplayDepth, - visited_deps: &mut HashSet, + visited_deps: &mut HashSet, levels_continue: &mut Vec, - print_stack: &mut Vec, + print_stack: &mut Vec, ) { let new = no_dedupe || visited_deps.insert(node_index); @@ -371,16 +371,16 @@ fn print_node<'a>( fn print_dependencies<'a>( ws: &Workspace<'_>, graph: &'a Graph<'_>, - node_index: usize, + node_index: NodeId, format: &Pattern, symbols: &Symbols, pkgs_to_prune: &[PackageIdSpec], prefix: Prefix, no_dedupe: bool, display_depth: DisplayDepth, - visited_deps: &mut HashSet, + visited_deps: &mut HashSet, levels_continue: &mut Vec, - print_stack: &mut Vec, + print_stack: &mut Vec, kind: &EdgeKind, ) { let deps = graph.edges_of_kind(node_index, kind); From 822135d22e58d64147f6a20d6e2922dc2a884228 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Feb 2025 14:16:04 -0600 Subject: [PATCH 5/6] refactor(tree): Manually implement comparison traits --- src/cargo/ops/tree/graph.rs | 40 ++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index bdfe5b86652..47a298d2db9 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -10,17 +10,43 @@ use crate::util::interning::{InternedString, INTERNED_DEFAULT}; use crate::util::CargoResult; use std::collections::{HashMap, HashSet}; -#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] +#[derive(Debug, Copy, Clone)] pub struct NodeId { index: usize, } impl NodeId { - fn with_index(index: usize) -> Self { + fn new(index: usize) -> Self { Self { index } } } +impl PartialEq for NodeId { + fn eq(&self, other: &Self) -> bool { + self.index == other.index + } +} + +impl Eq for NodeId {} + +impl PartialOrd for NodeId { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for NodeId { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.index.cmp(&other.index) + } +} + +impl std::hash::Hash for NodeId { + fn hash(&self, state: &mut H) { + self.index.hash(state) + } +} + #[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] pub enum Node { Package { @@ -134,7 +160,7 @@ impl<'a> Graph<'a> { /// Adds a new node to the graph, returning its new index. fn add_node(&mut self, node: Node) -> NodeId { - let from_index = NodeId::with_index(self.nodes.len()); + let from_index = NodeId::new(self.nodes.len()); self.nodes.push(node); self.edges.push(Edges::new()); self.index.insert(self.node(from_index).clone(), from_index); @@ -178,7 +204,7 @@ impl<'a> Graph<'a> { Node::Package { package_id, .. } => package_ids.contains(package_id), _ => false, }) - .map(|(i, node)| (node, NodeId::with_index(i))) + .map(|(i, node)| (node, NodeId::new(i))) .collect(); // Sort for consistent output (the same command should always return // the same output). "unstable" since nodes should always be unique. @@ -252,7 +278,7 @@ impl<'a> Graph<'a> { for edge in node_edges.all() { let new_edge = Edge { kind: edge.kind(), - node: NodeId::with_index(from_idx), + node: NodeId::new(from_idx), }; new_edges[edge.node().index].add_edge(new_edge); } @@ -273,7 +299,7 @@ impl<'a> Graph<'a> { packages .entry(package_id.name()) .or_insert_with(Vec::new) - .push((node, NodeId::with_index(i))); + .push((node, NodeId::new(i))); } } @@ -645,7 +671,7 @@ fn add_internal_features(graph: &mut Graph<'_>, resolve: &Resolve) { Node::Package { .. } => None, Node::Feature { node_index, name } => { let package_id = graph.package_id_for_index(*node_index); - Some((package_id, *node_index, NodeId::with_index(i), *name)) + Some((package_id, *node_index, NodeId::new(i), *name)) } }) .collect(); From 128604fe745988a048141fe95d879dd23da84acc Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Feb 2025 14:24:17 -0600 Subject: [PATCH 6/6] refactor(tree): Make debugging node ids easier I found when debugging some issues with edges, having to track indices was making things much more difficult. My hope is this will help with little negative impact. We could put this behind a `#[cfg(debug_asserts)]` but I'm assuming it doesn't make enough of a performance difference to matter. --- src/cargo/ops/tree/graph.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index 47a298d2db9..a16d452b925 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -13,11 +13,13 @@ use std::collections::{HashMap, HashSet}; #[derive(Debug, Copy, Clone)] pub struct NodeId { index: usize, + #[allow(dead_code)] // intended for `derive(Debug)` + debug: InternedString, } impl NodeId { - fn new(index: usize) -> Self { - Self { index } + fn new(index: usize, debug: InternedString) -> Self { + Self { index, debug } } } @@ -63,6 +65,15 @@ pub enum Node { }, } +impl Node { + fn name(&self) -> InternedString { + match self { + Self::Package { package_id, .. } => package_id.name(), + Self::Feature { name, .. } => *name, + } + } +} + #[derive(Debug, Copy, Hash, Eq, Clone, PartialEq)] pub struct Edge { kind: EdgeKind, @@ -160,7 +171,7 @@ impl<'a> Graph<'a> { /// Adds a new node to the graph, returning its new index. fn add_node(&mut self, node: Node) -> NodeId { - let from_index = NodeId::new(self.nodes.len()); + let from_index = NodeId::new(self.nodes.len(), node.name()); self.nodes.push(node); self.edges.push(Edges::new()); self.index.insert(self.node(from_index).clone(), from_index); @@ -204,7 +215,7 @@ impl<'a> Graph<'a> { Node::Package { package_id, .. } => package_ids.contains(package_id), _ => false, }) - .map(|(i, node)| (node, NodeId::new(i))) + .map(|(i, node)| (node, NodeId::new(i, node.name()))) .collect(); // Sort for consistent output (the same command should always return // the same output). "unstable" since nodes should always be unique. @@ -278,7 +289,7 @@ impl<'a> Graph<'a> { for edge in node_edges.all() { let new_edge = Edge { kind: edge.kind(), - node: NodeId::new(from_idx), + node: NodeId::new(from_idx, self.nodes[from_idx].name()), }; new_edges[edge.node().index].add_edge(new_edge); } @@ -299,7 +310,7 @@ impl<'a> Graph<'a> { packages .entry(package_id.name()) .or_insert_with(Vec::new) - .push((node, NodeId::new(i))); + .push((node, NodeId::new(i, node.name()))); } } @@ -671,7 +682,7 @@ fn add_internal_features(graph: &mut Graph<'_>, resolve: &Resolve) { Node::Package { .. } => None, Node::Feature { node_index, name } => { let package_id = graph.package_id_for_index(*node_index); - Some((package_id, *node_index, NodeId::new(i), *name)) + Some((package_id, *node_index, NodeId::new(i, *name), *name)) } }) .collect();