From aadb41d7c97dcc5f8fdbf7df210d25b71a5e3bee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Borgna?= Date: Tue, 12 May 2026 11:20:07 +0100 Subject: [PATCH 1/5] fix: Replace non-deterministic iterations on hash maps --- Cargo.lock | 1 + Cargo.toml | 1 + hugr-core/src/extension/declarative/ops.rs | 4 +++ hugr-core/src/hugr.rs | 7 ++-- hugr-core/src/hugr/hugrmut.rs | 8 +++++ hugr-core/src/hugr/linking.rs | 41 ++++++++++++++++++---- hugr-core/src/hugr/patch/outline_cfg.rs | 10 +++--- hugr-core/src/hugr/patch/replace.rs | 8 +++++ hugr-core/src/import.rs | 7 ++-- hugr-persistent/Cargo.toml | 1 + hugr-persistent/src/persistent_hugr.rs | 16 +++++---- hugr-persistent/src/tests.rs | 4 +++ 12 files changed, 83 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d6b0dac07..2dddd1159c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1171,6 +1171,7 @@ dependencies = [ "delegate", "derive_more 2.1.1", "hugr-core", + "indexmap 2.14.0", "insta", "itertools 0.14.0", "petgraph", diff --git a/Cargo.toml b/Cargo.toml index 4d547f037b..1b2299072a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ missing_docs = "warn" # https://github.com/rust-lang/rust-clippy/issues/5112 debug_assert_with_mut_call = "warn" too_long_first_doc_paragraph = "warn" +iter_over_hash_type = "warn" [workspace.dependencies] anyhow = "1.0.101" diff --git a/hugr-core/src/extension/declarative/ops.rs b/hugr-core/src/extension/declarative/ops.rs index 0987b5166b..8d89be51e2 100644 --- a/hugr-core/src/extension/declarative/ops.rs +++ b/hugr-core/src/extension/declarative/ops.rs @@ -100,6 +100,10 @@ impl OperationDeclaration { extension_ref, )?; + #[expect( + clippy::iter_over_hash_type, + reason = "copying independent map entries is deterministic" + )] for (k, v) in &self.misc { op_def.add_misc(k, v.clone()); } diff --git a/hugr-core/src/hugr.rs b/hugr-core/src/hugr.rs index dcbeff4f2a..97d21c487f 100644 --- a/hugr-core/src/hugr.rs +++ b/hugr-core/src/hugr.rs @@ -11,7 +11,6 @@ pub mod validate; pub mod views; use std::cmp::Ordering; -use std::collections::HashMap; use std::collections::VecDeque; use std::io; use std::iter; @@ -444,7 +443,7 @@ impl Hugr { /// (since these must be the entry and exit blocks) and in each dataflow /// sibling graph (since these must be the input and output nodes). fn order_siblings_by_node_index(&mut self) { - let mut node_children: HashMap> = HashMap::default(); + let mut node_children = Vec::new(); for node in self.nodes() { let mut children = self.children(node).collect_vec(); if self.contains_dsg_or_csg(node) { @@ -452,9 +451,9 @@ impl Hugr { } else { children.sort(); } - node_children.insert(node, children); + node_children.push((node, children)); } - for (node, children) in &node_children { + for (node, children) in node_children { self.hierarchy.detach_children(node.into_portgraph()); for child in children { self.hierarchy diff --git a/hugr-core/src/hugr/hugrmut.rs b/hugr-core/src/hugr/hugrmut.rs index d8168db9ee..f36c0844a7 100644 --- a/hugr-core/src/hugr/hugrmut.rs +++ b/hugr-core/src/hugr/hugrmut.rs @@ -663,6 +663,10 @@ impl HugrMut for Hugr { // Update the optypes and metadata, taking them from the other graph. // // No need to compute each node's extensions here, as we merge `other.extensions` directly. + #[expect( + clippy::iter_over_hash_type, + reason = "updating per-node payloads is independent for each node" + )] for (&node, &new_node) in &inserted.node_map { let node_pg = node.into_portgraph(); let new_node_pg = new_node.into_portgraph(); @@ -686,6 +690,10 @@ impl HugrMut for Hugr { // Update the optypes and metadata, copying them from the other graph. // // No need to compute each node's extensions here, as we merge `other.extensions` directly. + #[expect( + clippy::iter_over_hash_type, + reason = "updating per-node payloads is independent for each node" + )] for (&node, &new_node) in &inserted.node_map { let nodetype = other.get_optype(node); self.op_types diff --git a/hugr-core/src/hugr/linking.rs b/hugr-core/src/hugr/linking.rs index 709ddc5959..4c46477d43 100644 --- a/hugr-core/src/hugr/linking.rs +++ b/hugr-core/src/hugr/linking.rs @@ -91,6 +91,10 @@ pub trait HugrLinking: HugrMut { roots.insert(other.entrypoint(), parent); other.set_parent(other.entrypoint(), other.module_root()); }; + #[expect( + clippy::iter_over_hash_type, + reason = "roots are stored in a BTreeMap and child pruning is independent" + )] for (ch, dirv) in children.iter() { roots.insert(*ch, self.module_root()); if matches!(dirv, NodeLinkingDirective::UseExisting(_)) { @@ -759,10 +763,22 @@ fn check_directives( parent: Option, children: &NodeLinkingDirectives, ) -> Result, NodeLinkingError> { + /// Returns the minimum key in `children` for which `pred` returns true, or + /// `None` if there is no such key. + /// + /// Helper function to avoid indeterminism in error reporting from iterating + /// over `children` (a `HashMap`). + fn min_directive_key( + children: &NodeLinkingDirectives, + mut pred: impl FnMut(SN) -> bool, + ) -> Option { + children.keys().copied().filter(|&n| pred(n)).min() + } + if parent.is_some() { if other.entrypoint() == other.module_root() { - if let Some(c) = children.keys().next() { - return Err(NodeLinkingError::ChildOfEntrypoint(*c)); + if let Some(c) = min_directive_key(children, |_| true) { + return Err(NodeLinkingError::ChildOfEntrypoint(c)); } } else { let mut n = other.entrypoint(); @@ -787,10 +803,15 @@ fn check_directives( replace: HashMap::default(), use_existing: HashMap::default(), }; - for (&sn, dirv) in children { - if other.get_parent(sn) != Some(other.module_root()) { - return Err(NodeLinkingError::NotChildOfRoot(sn)); - } + if let Some(sn) = min_directive_key(children, |sn| { + other.get_parent(sn) != Some(other.module_root()) + }) { + return Err(NodeLinkingError::NotChildOfRoot(sn)); + } + for sn in other.children(other.module_root()) { + let Some(dirv) = children.get(&sn) else { + continue; + }; match dirv { NodeLinkingDirective::Add { replace } => { for &r in replace { @@ -814,12 +835,20 @@ fn link_by_node( ) { // Resolve `use_existing` first in case the existing node is also replaced by // a new node (which we know will not be in RHS of any entry in `replace`). + #[expect( + clippy::iter_over_hash_type, + reason = "each source is replaced independently, so order cannot affect the result" + )] for (sn, tn) in transfers.use_existing { let copy = node_map.remove(&sn).unwrap(); // Because of `UseExisting` we avoided adding `sn`s descendants debug_assert_eq!(hugr.children(copy).next(), None); replace_static_src(hugr, copy, tn); } + #[expect( + clippy::iter_over_hash_type, + reason = "each source is replaced independently, so order cannot affect the result" + )] for (tn, sn) in transfers.replace { let new_node = *node_map.get(&sn).unwrap(); replace_static_src(hugr, tn, new_node); diff --git a/hugr-core/src/hugr/patch/outline_cfg.rs b/hugr-core/src/hugr/patch/outline_cfg.rs index e37e749dde..c2bb512bd0 100644 --- a/hugr-core/src/hugr/patch/outline_cfg.rs +++ b/hugr-core/src/hugr/patch/outline_cfg.rs @@ -1,6 +1,6 @@ //! Rewrite for inserting a CFG-node into the hierarchy containing a subsection //! of an existing CFG -use std::collections::HashSet; +use std::collections::BTreeSet; use itertools::Itertools; use thiserror::Error; @@ -19,14 +19,14 @@ use super::{PatchHugrMut, PatchVerification}; /// Moves some of the blocks in a Control-flow region into a new CFG-node that /// is the only child of a new Basic Block in the original region. pub struct OutlineCfg { - blocks: HashSet, + blocks: BTreeSet, } impl OutlineCfg { /// Create a new `OutlineCfg` rewrite that will move the provided blocks. pub fn new(blocks: impl IntoIterator) -> Self { Self { - blocks: HashSet::from_iter(blocks), + blocks: BTreeSet::from_iter(blocks), } } @@ -236,7 +236,7 @@ pub enum OutlineCfgError { #[cfg(test)] mod test { - use std::collections::HashSet; + use std::collections::{BTreeSet, HashSet}; use crate::builder::{ BlockBuilder, BuildError, CFGBuilder, Container, Dataflow, DataflowSubContainer, @@ -481,7 +481,7 @@ mod test { cfg: Node, blocks: Vec, ) -> (Node, Node, Node) { - let mut other_blocks = h.children(cfg).collect::>(); + let mut other_blocks = h.children(cfg).collect::>(); assert!(blocks.iter().all(|b| other_blocks.remove(b))); let [new_block, new_cfg] = h.apply_patch(OutlineCfg::new(blocks.clone())).unwrap(); diff --git a/hugr-core/src/hugr/patch/replace.rs b/hugr-core/src/hugr/patch/replace.rs index bdd01914c5..37a66cab94 100644 --- a/hugr-core/src/hugr/patch/replace.rs +++ b/hugr-core/src/hugr/patch/replace.rs @@ -400,6 +400,10 @@ impl PatchHugrMut for Replacement { h.remove_node(inserted_entrypoint); // 6. Transfer to keys of `transfers` children of the corresponding values. + #[expect( + clippy::iter_over_hash_type, + reason = "adoptions move disjoint child sets, so order cannot affect the result" + )] for (new_parent, &old_parent) in &self.adoptions { let new_parent = node_map.get(new_parent).unwrap(); debug_assert!(h.children(old_parent).next().is_some()); @@ -409,6 +413,10 @@ impl PatchHugrMut for Replacement { } // 7. Remove remaining nodes + #[expect( + clippy::iter_over_hash_type, + reason = "all nodes in this set are removed, so removal order does not affect the result" + )] for n in to_remove { h.remove_node(n); } diff --git a/hugr-core/src/import.rs b/hugr-core/src/import.rs index 9f775f0b98..dbde9d19be 100644 --- a/hugr-core/src/import.rs +++ b/hugr-core/src/import.rs @@ -30,8 +30,9 @@ use crate::{ }; use hugr_model::v0::table; use hugr_model::v0::{self as model}; +use indexmap::IndexMap; use itertools::{Either, Itertools}; -use rustc_hash::FxHashMap; +use rustc_hash::{FxBuildHasher, FxHashMap}; use serde::Deserialize as _; use smol_str::{SmolStr, ToSmolStr}; use thiserror::Error; @@ -247,7 +248,7 @@ pub(crate) fn import_described_hugr( let mut ctx = Context { module, hugr: Hugr::new(), - link_ports: FxHashMap::default(), + link_ports: IndexMap::default(), static_edges: Vec::new(), extensions, nodes: FxHashMap::default(), @@ -287,7 +288,7 @@ struct Context<'a> { /// The ports that are part of each link. This is used to connect the ports at the end of the /// import process. - link_ports: FxHashMap<(table::RegionId, table::LinkIndex), Vec<(Node, Port)>>, + link_ports: IndexMap<(table::RegionId, table::LinkIndex), Vec<(Node, Port)>, FxBuildHasher>, /// Pairs of nodes that should be connected by a static edge. /// These are collected during the import process and connected at the end. diff --git a/hugr-persistent/Cargo.toml b/hugr-persistent/Cargo.toml index a94bca1530..b79d8a527b 100644 --- a/hugr-persistent/Cargo.toml +++ b/hugr-persistent/Cargo.toml @@ -26,6 +26,7 @@ derive_more = { workspace = true, features = [ ] } delegate.workspace = true itertools.workspace = true +indexmap.workspace = true petgraph.workspace = true portgraph.workspace = true relrc = { workspace = true, features = ["petgraph", "serde"] } diff --git a/hugr-persistent/src/persistent_hugr.rs b/hugr-persistent/src/persistent_hugr.rs index 01bbfd6bae..3ebc0e5a8c 100644 --- a/hugr-persistent/src/persistent_hugr.rs +++ b/hugr-persistent/src/persistent_hugr.rs @@ -8,6 +8,7 @@ use hugr_core::{ Hugr, HugrView, Node, hugr::patch::{Patch, simple_replace}, }; +use indexmap::IndexMap; use itertools::Itertools; use relrc::HistoryGraph; @@ -187,13 +188,10 @@ impl PersistentHugr { ) -> Result { // Check that `replacement` does not conflict with siblings at any of its // parents - let new_invalid_nodes = replacement - .subgraph() - .nodes() - .iter() - .map(|&PatchNode(id, node)| (id, node)) - .into_grouping_map() - .collect::>(); + let mut new_invalid_nodes = IndexMap::<_, BTreeSet<_>>::new(); + for &PatchNode(id, node) in replacement.subgraph().nodes() { + new_invalid_nodes.entry(id).or_default().insert(node); + } for (parent, new_invalid_nodes) in new_invalid_nodes { let invalidation_set = self.deleted_nodes(parent).collect(); if let Some(&node) = new_invalid_nodes.intersection(&invalidation_set).next() { @@ -394,6 +392,10 @@ impl PersistentHugr { hugr.mermaid_string() ); + #[expect( + clippy::iter_over_hash_type, + reason = "inserting independent node mappings is order-independent" + )] for (old_node, new_node) in new_node_map { let old_patch_node = PatchNode(commit_id, old_node); node_map.insert(old_patch_node, new_node); diff --git a/hugr-persistent/src/tests.rs b/hugr-persistent/src/tests.rs index 045b517460..4664606c30 100644 --- a/hugr-persistent/src/tests.rs +++ b/hugr-persistent/src/tests.rs @@ -276,6 +276,10 @@ pub(crate) fn test_state_space() -> TestStateSpace { // and nodes in the state space let inv_node_map = { let mut inv = BTreeMap::new(); + #[expect( + clippy::iter_over_hash_type, + reason = "insertion order of non-colliding keys does not matter for BTreeMaps" + )] for (repl_node, hugr_node) in node_map { inv.insert(hugr_node, repl_node); } From 88ec14d760e337f6f338057870db9cb2ab12c62a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Borgna?= <121866228+aborgna-q@users.noreply.github.com> Date: Wed, 20 May 2026 17:03:25 +0100 Subject: [PATCH 2/5] Update hugr-core/src/hugr/linking.rs Co-authored-by: Alan Lawrence --- hugr-core/src/hugr/linking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hugr-core/src/hugr/linking.rs b/hugr-core/src/hugr/linking.rs index 4c46477d43..1924f3a557 100644 --- a/hugr-core/src/hugr/linking.rs +++ b/hugr-core/src/hugr/linking.rs @@ -766,7 +766,7 @@ fn check_directives( /// Returns the minimum key in `children` for which `pred` returns true, or /// `None` if there is no such key. /// - /// Helper function to avoid indeterminism in error reporting from iterating + /// Helper function to avoid nondeterminism in error reporting from iterating /// over `children` (a `HashMap`). fn min_directive_key( children: &NodeLinkingDirectives, From 4d9c0889c2640b1f79e1a686ffea6534ee2e5778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Borgna?= Date: Thu, 4 Jun 2026 14:58:13 +0100 Subject: [PATCH 3/5] Review comments --- hugr-core/src/hugr.rs | 9 +++--- hugr-core/src/hugr/linking.rs | 33 ++++++-------------- hugr-core/src/hugr/patch/outline_cfg.rs | 5 +-- hugr-core/src/hugr/views/sibling_subgraph.rs | 2 +- 4 files changed, 18 insertions(+), 31 deletions(-) diff --git a/hugr-core/src/hugr.rs b/hugr-core/src/hugr.rs index 97d21c487f..99eeb32237 100644 --- a/hugr-core/src/hugr.rs +++ b/hugr-core/src/hugr.rs @@ -443,17 +443,16 @@ impl Hugr { /// (since these must be the entry and exit blocks) and in each dataflow /// sibling graph (since these must be the input and output nodes). fn order_siblings_by_node_index(&mut self) { - let mut node_children = Vec::new(); - for node in self.nodes() { + // Equivalent to `self.nodes()` that does not borrow the full structure, so we can mutate `self.hierarchy` while iterating. + let nodes = self.graph.nodes_iter().map_into(); + for node in nodes { let mut children = self.children(node).collect_vec(); if self.contains_dsg_or_csg(node) { children[2..].sort(); } else { children.sort(); } - node_children.push((node, children)); - } - for (node, children) in node_children { + self.hierarchy.detach_children(node.into_portgraph()); for child in children { self.hierarchy diff --git a/hugr-core/src/hugr/linking.rs b/hugr-core/src/hugr/linking.rs index 1924f3a557..0aea858ce6 100644 --- a/hugr-core/src/hugr/linking.rs +++ b/hugr-core/src/hugr/linking.rs @@ -763,22 +763,10 @@ fn check_directives( parent: Option, children: &NodeLinkingDirectives, ) -> Result, NodeLinkingError> { - /// Returns the minimum key in `children` for which `pred` returns true, or - /// `None` if there is no such key. - /// - /// Helper function to avoid nondeterminism in error reporting from iterating - /// over `children` (a `HashMap`). - fn min_directive_key( - children: &NodeLinkingDirectives, - mut pred: impl FnMut(SN) -> bool, - ) -> Option { - children.keys().copied().filter(|&n| pred(n)).min() - } - if parent.is_some() { if other.entrypoint() == other.module_root() { - if let Some(c) = min_directive_key(children, |_| true) { - return Err(NodeLinkingError::ChildOfEntrypoint(c)); + if let Some(c) = children.keys().next() { + return Err(NodeLinkingError::ChildOfEntrypoint(*c)); } } else { let mut n = other.entrypoint(); @@ -803,15 +791,14 @@ fn check_directives( replace: HashMap::default(), use_existing: HashMap::default(), }; - if let Some(sn) = min_directive_key(children, |sn| { - other.get_parent(sn) != Some(other.module_root()) - }) { - return Err(NodeLinkingError::NotChildOfRoot(sn)); - } - for sn in other.children(other.module_root()) { - let Some(dirv) = children.get(&sn) else { - continue; - }; + #[expect( + clippy::iter_over_hash_type, + reason = "Nondeterminism only affects the error reported." + )] + for (&sn, dirv) in children { + if other.get_parent(sn) != Some(other.module_root()) { + return Err(NodeLinkingError::NotChildOfRoot(sn)); + } match dirv { NodeLinkingDirective::Add { replace } => { for &r in replace { diff --git a/hugr-core/src/hugr/patch/outline_cfg.rs b/hugr-core/src/hugr/patch/outline_cfg.rs index c2bb512bd0..2735455b02 100644 --- a/hugr-core/src/hugr/patch/outline_cfg.rs +++ b/hugr-core/src/hugr/patch/outline_cfg.rs @@ -236,7 +236,7 @@ pub enum OutlineCfgError { #[cfg(test)] mod test { - use std::collections::{BTreeSet, HashSet}; + use std::collections::HashSet; use crate::builder::{ BlockBuilder, BuildError, CFGBuilder, Container, Dataflow, DataflowSubContainer, @@ -481,10 +481,11 @@ mod test { cfg: Node, blocks: Vec, ) -> (Node, Node, Node) { - let mut other_blocks = h.children(cfg).collect::>(); + let mut other_blocks = h.children(cfg).collect::>(); assert!(blocks.iter().all(|b| other_blocks.remove(b))); let [new_block, new_cfg] = h.apply_patch(OutlineCfg::new(blocks.clone())).unwrap(); + #[expect(clippy::iter_over_hash_type, reason = "order does not matter")] for n in other_blocks { assert_eq!(h.get_parent(n), Some(cfg)); } diff --git a/hugr-core/src/hugr/views/sibling_subgraph.rs b/hugr-core/src/hugr/views/sibling_subgraph.rs index 704e5a2d5a..f4a698e81d 100644 --- a/hugr-core/src/hugr/views/sibling_subgraph.rs +++ b/hugr-core/src/hugr/views/sibling_subgraph.rs @@ -550,7 +550,7 @@ impl SiblingSubgraph { } /// Check the validity of the subgraph, as described in the docs of - /// [`SiblingSubgraph::try_new`], but do not check convexity. + /// [`SiblingSubgraph::try_new`], but do not check convexity. pub fn validate_skip_convexity( &self, hugr: &impl HugrView, From edf2337267305c8b7e074bbe590e7c6e674e7583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Borgna?= Date: Fri, 5 Jun 2026 15:37:51 +0100 Subject: [PATCH 4/5] enforce different values in `Replacement::adoptions` --- hugr-core/src/hugr/patch/replace.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/hugr-core/src/hugr/patch/replace.rs b/hugr-core/src/hugr/patch/replace.rs index 37a66cab94..bd8d82aabd 100644 --- a/hugr-core/src/hugr/patch/replace.rs +++ b/hugr-core/src/hugr/patch/replace.rs @@ -71,12 +71,16 @@ pub struct Replacement { pub replacement: Hugr, /// Describes how parts of the Hugr that would otherwise be removed should /// instead be preserved but with new parents amongst the newly-inserted - /// nodes. This is a Map from container nodes in [`Self::replacement`] + /// nodes. + /// + /// This is a Map from container nodes in [`Self::replacement`] /// that have no children, to container nodes that are descended from /// [`Self::removal`]. The keys are the new parents for the children of /// the values. Note no value may be ancestor or descendant of another. /// This is "B" in the spec; "R" is the set of descendants of /// [`Self::removal`] that are not descendants of values here. + /// + /// The nodes in the values must be distinct descendants of the removed nodes. pub adoptions: HashMap, /// Edges from nodes in the existing Hugr that are not removed /// ([`NewEdgeSpec::src`] in Gamma\R) to inserted nodes @@ -402,11 +406,16 @@ impl PatchHugrMut for Replacement { // 6. Transfer to keys of `transfers` children of the corresponding values. #[expect( clippy::iter_over_hash_type, - reason = "adoptions move disjoint child sets, so order cannot affect the result" + reason = "adoptions move disjoint child sets, so order cannot affect the result. Repeated values return an error." )] for (new_parent, &old_parent) in &self.adoptions { let new_parent = node_map.get(new_parent).unwrap(); - debug_assert!(h.children(old_parent).next().is_some()); + + if h.children(old_parent).next().is_none() { + // `old_parent` appeared more than once on the right-hand side of `adoptions`. + return Err(ReplaceError::RepeatedAdoptee { node: old_parent }); + } + while let Some(ch) = h.first_child(old_parent) { h.set_parent(ch, *new_parent); } @@ -517,6 +526,12 @@ pub enum ReplaceError { /// The [`NewEdgeKind`] was not applicable for the source/target node(s) #[error("The edge kind was not applicable to the {0:?} node: {1:?}")] BadEdgeKind(Direction, WhichEdgeSpec), + /// Some value in [`Replacement::adoptions`] was repeated for multiple parents. The nodes are indicated on a best-effort basis. + #[error("Node {node:?} adopted by multiple new parents")] + RepeatedAdoptee { + /// The node that was repeated + node: HostNode, + }, } /// The three kinds of [`NewEdgeSpec`] that may appear in a [`ReplaceError`] From bbc5f04a60a1ceb2b7b0b7bce58f88a70b34be4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Borgna?= Date: Fri, 5 Jun 2026 15:40:34 +0100 Subject: [PATCH 5/5] Replace internal removed node HashMap with btreeset --- hugr-core/src/hugr/patch/replace.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/hugr-core/src/hugr/patch/replace.rs b/hugr-core/src/hugr/patch/replace.rs index bd8d82aabd..a90efaf565 100644 --- a/hugr-core/src/hugr/patch/replace.rs +++ b/hugr-core/src/hugr/patch/replace.rs @@ -1,6 +1,6 @@ //! Implementation of the `Replace` operation. -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{BTreeSet, HashMap, HashSet, VecDeque}; use itertools::Itertools; use thiserror::Error; @@ -150,7 +150,7 @@ impl NewEdgeSpec { fn check_existing_edge( &self, h: &impl HugrView, - legal_src_ancestors: &HashSet, + legal_src_ancestors: &BTreeSet, err_edge: impl Fn(Self) -> WhichEdgeSpec, ) -> Result<(), ReplaceError> { if let NewEdgeKind::Static { tgt_pos, .. } | NewEdgeKind::Value { tgt_pos, .. } = self.kind @@ -206,7 +206,7 @@ impl Replacement { fn get_removed_nodes( &self, h: &impl HugrView, - ) -> Result, ReplaceError> { + ) -> Result, ReplaceError> { // Check the keys of the transfer map too, the values we'll use imminently self.adoptions.keys().try_for_each(|&n| { (self.replacement.contains_node(n) @@ -226,7 +226,7 @@ impl Replacement { )); } - let mut removed = HashSet::new(); + let mut removed = BTreeSet::new(); let mut queue = VecDeque::from_iter(self.removal.iter().copied()); while let Some(n) = queue.pop_front() { let new = removed.insert(n); @@ -422,10 +422,6 @@ impl PatchHugrMut for Replacement { } // 7. Remove remaining nodes - #[expect( - clippy::iter_over_hash_type, - reason = "all nodes in this set are removed, so removal order does not affect the result" - )] for n in to_remove { h.remove_node(n); } @@ -439,7 +435,7 @@ fn transfer_edges<'a, SrcNode, TgtNode, HostNode>( trans_src: impl Fn(SrcNode) -> Option, trans_tgt: impl Fn(TgtNode) -> Option, err_spec: impl Fn(NewEdgeSpec) -> WhichEdgeSpec, - legal_src_ancestors: Option<&HashSet>, + legal_src_ancestors: Option<&BTreeSet>, ) -> Result<(), ReplaceError> where SrcNode: 'a + HugrNode,