From 65002d342760f38b3bc8cbf97e4f4c66f60c8440 Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Mon, 21 Aug 2023 22:22:53 +0100 Subject: [PATCH 01/10] feat: misc hugrmut changes - split methods between HugrMutInternals & HugrMut - Add a hierarchy root to `Hugr::canonical_order` - check the root type when querying a hugrview - Add a lifetime to `HugrInternals::Portgraph` so the views can return clones instead of double references. --- src/builder/build_traits.rs | 2 +- src/builder/cfg.rs | 2 +- src/builder/conditional.rs | 2 +- src/builder/dataflow.rs | 2 +- src/builder/module.rs | 6 +- src/hugr.rs | 62 ++- src/hugr/hugrmut.rs | 728 +++++++++++++++-------------- src/hugr/rewrite/outline_cfg.rs | 3 +- src/hugr/rewrite/simple_replace.rs | 2 +- src/hugr/serialize.rs | 7 +- src/hugr/validate.rs | 3 +- src/hugr/views.rs | 37 +- src/hugr/views/hierarchy.rs | 95 ++-- src/ops/custom.rs | 3 +- src/ops/handle.rs | 5 + 15 files changed, 538 insertions(+), 421 deletions(-) diff --git a/src/builder/build_traits.rs b/src/builder/build_traits.rs index bd6af4dd6..14932d7e2 100644 --- a/src/builder/build_traits.rs +++ b/src/builder/build_traits.rs @@ -31,7 +31,7 @@ use super::{ use crate::Hugr; -use crate::hugr::HugrInternalsMut; +use crate::hugr::HugrMut; /// Trait for HUGR container builders. /// Containers are nodes that are parents of sibling graphs. diff --git a/src/builder/cfg.rs b/src/builder/cfg.rs index f8d46081d..f6396f386 100644 --- a/src/builder/cfg.rs +++ b/src/builder/cfg.rs @@ -12,7 +12,7 @@ use crate::types::AbstractSignature; use crate::Node; use crate::{ - hugr::{HugrInternalsMut, NodeType}, + hugr::{HugrMut, NodeType}, type_row, types::{ClassicRow, SimpleRow, SimpleType}, Hugr, diff --git a/src/builder/conditional.rs b/src/builder/conditional.rs index fe3b9fd2e..32f854ef3 100644 --- a/src/builder/conditional.rs +++ b/src/builder/conditional.rs @@ -15,7 +15,7 @@ use super::{ use crate::Node; use crate::{ - hugr::{HugrInternalsMut, NodeType}, + hugr::{HugrMut, NodeType}, Hugr, }; diff --git a/src/builder/dataflow.rs b/src/builder/dataflow.rs index d330489c7..8b41f0608 100644 --- a/src/builder/dataflow.rs +++ b/src/builder/dataflow.rs @@ -11,7 +11,7 @@ use crate::types::{AbstractSignature, Signature}; use crate::resource::ResourceSet; use crate::Node; -use crate::{hugr::HugrInternalsMut, Hugr}; +use crate::{hugr::HugrMut, Hugr}; /// Builder for a [`ops::DFG`] node. #[derive(Debug, Clone, PartialEq)] diff --git a/src/builder/module.rs b/src/builder/module.rs index 803f4d830..a63838cdd 100644 --- a/src/builder/module.rs +++ b/src/builder/module.rs @@ -4,6 +4,7 @@ use super::{ BuildError, Container, }; +use crate::hugr::hugrmut::sealed::HugrMutInternals; use crate::{ hugr::{views::HugrView, ValidationError}, ops, @@ -18,10 +19,7 @@ use crate::types::Signature; use crate::Node; use smol_str::SmolStr; -use crate::{ - hugr::{HugrInternalsMut, NodeType}, - Hugr, -}; +use crate::{hugr::NodeType, Hugr}; /// Builder for a HUGR module. #[derive(Debug, Clone, PartialEq)] diff --git a/src/hugr.rs b/src/hugr.rs index ab4717dc4..b74364b0c 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -1,6 +1,6 @@ //! The Hugr data structure, and its basic component handles. -mod hugrmut; +pub mod hugrmut; pub mod rewrite; pub mod serialize; @@ -10,14 +10,14 @@ pub mod views; use std::collections::VecDeque; use std::iter; -pub(crate) use self::hugrmut::HugrInternalsMut; +pub(crate) use self::hugrmut::HugrMut; pub use self::validate::ValidationError; use derive_more::From; pub use rewrite::{Rewrite, SimpleReplacement, SimpleReplacementError}; use portgraph::multiportgraph::MultiPortGraph; -use portgraph::{Hierarchy, PortMut, UnmanagedDenseMap}; +use portgraph::{Hierarchy, NodeIndex, PortMut, UnmanagedDenseMap}; use thiserror::Error; #[cfg(feature = "pyo3")] @@ -224,12 +224,16 @@ impl Hugr { } } - /// Produce a canonical ordering of the nodes. + /// Produce a canonical ordering of the descendant nodes of a root, + /// following the graph hierarchy. + /// + /// This starts with the root, and then proceeds in BFS order through the + /// contained regions. /// /// Used by [`HugrMut::canonicalize_nodes`] and the serialization code. - fn canonical_order(&self) -> impl Iterator + '_ { + fn canonical_order(&self, root: Node) -> impl Iterator + '_ { // Generate a BFS-ordered list of nodes based on the hierarchy - let mut queue = VecDeque::from([self.root.into()]); + let mut queue = VecDeque::from([root]); iter::from_fn(move || { let node = queue.pop_front()?; for child in self.children(node) { @@ -238,6 +242,46 @@ impl Hugr { Some(node) }) } + + /// Compact the nodes indices of the hugr to be contiguous, and order them as a breadth-first + /// traversal of the hierarchy. + /// + /// The rekey function is called for each moved node with the old and new indices. + /// + /// After this operation, a serialization and deserialization of the Hugr is guaranteed to + /// preserve the indices. + pub fn canonicalize_nodes(&mut self, mut rekey: impl FnMut(Node, Node)) { + // Generate the ordered list of nodes + let mut ordered = Vec::with_capacity(self.node_count()); + let root = self.root(); + ordered.extend(self.as_mut().canonical_order(root)); + + // Permute the nodes in the graph to match the order. + // + // Invariant: All the elements before `position` are in the correct place. + for position in 0..ordered.len() { + // Find the element's location. If it originally came from a previous position + // then it has been swapped somewhere else, so we follow the permutation chain. + let mut source: Node = ordered[position]; + while position > source.index.index() { + source = ordered[source.index.index()]; + } + + let target: Node = NodeIndex::new(position).into(); + if target != source { + self.graph.swap_nodes(target.index, source.index); + self.op_types.swap(target.index, source.index); + self.hierarchy.swap_nodes(target.index, source.index); + rekey(source, target); + } + } + self.root = NodeIndex::new(0); + + // Finish by compacting the copy nodes. + // The operation nodes will be left in place. + // This step is not strictly necessary. + self.graph.compact_nodes(|_, _| {}); + } } impl Port { @@ -340,6 +384,12 @@ pub enum HugrError { /// An error occurred while manipulating the hierarchy. #[error("An error occurred while manipulating the hierarchy.")] HierarchyError(#[from] portgraph::hierarchy::AttachError), + /// The node doesn't exist. + #[error("Invalid node {node:?}.")] + InvalidNode { + /// The missing node + node: Node, + }, } #[cfg(feature = "pyo3")] diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 6a9a00e45..f0f098b5d 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -1,4 +1,4 @@ -//! Base HUGR builder providing low-level building blocks. +//! Low-level interface for modifying a HUGR. use std::collections::HashMap; use std::ops::Range; @@ -8,110 +8,376 @@ use portgraph::{LinkMut, NodeIndex, PortMut, PortView, SecondaryMap}; use crate::hugr::{Direction, HugrError, HugrView, Node, NodeType}; use crate::ops::OpType; -use crate::ops::handle::NodeHandle; use crate::{Hugr, Port}; +use self::sealed::HugrMutInternals; + use super::NodeMetadata; -pub(crate) use sealed::HugrInternalsMut; +/// Functions for low-level building of a HUGR. +pub trait HugrMut: HugrView + HugrMutInternals { + /// Returns the metadata associated with a node. + fn get_metadata_mut(&mut self, node: Node) -> &mut NodeMetadata; + + /// Sets the metadata associated with a node. + fn set_metadata(&mut self, node: Node, metadata: NodeMetadata) { + *self.get_metadata_mut(node) = metadata; + } -pub trait HugrMut: sealed::HugrInternalsMut { - /// The kind of handle that can be used to refer to the root node. + /// Add a node to the graph with a parent in the hierarchy. /// - /// The handle is guaranteed to be able to contain the operation returned by - /// [`HugrView::root_type`]. - type RootHandle: NodeHandle; + /// The node becomes the parent's last child. + #[inline] + fn add_op_with_parent( + &mut self, + parent: Node, + op: impl Into, + ) -> Result { + match self.contains_node(parent) { + true => self.hugr_mut().add_op_with_parent(parent, op), + false => Err(HugrError::InvalidNode { node: parent }), + } + } - /// The view interface for this mutable reference to a Hugr. - type View<'h>: 'h + HugrView - where - Self: 'h; + /// Add a node to the graph with a parent in the hierarchy. + /// + /// The node becomes the parent's last child. + #[inline] + fn add_node_with_parent(&mut self, parent: Node, op: NodeType) -> Result { + match self.contains_node(parent) { + true => self.hugr_mut().add_node_with_parent(parent, op), + false => Err(HugrError::InvalidNode { node: parent }), + } + } + + /// Add a node to the graph as the previous sibling of another node. + /// + /// The sibling node's parent becomes the new node's parent. + /// + /// # Errors + /// + /// - If the sibling node does not have a parent. + /// - If the attachment would introduce a cycle. + #[inline] + fn add_op_before(&mut self, sibling: Node, op: impl Into) -> Result { + match self.contains_node(sibling) { + true => self.hugr_mut().add_op_before(sibling, op), + false => Err(HugrError::InvalidNode { node: sibling }), + } + } + + /// Add a node to the graph as the next sibling of another node. + /// + /// The sibling node's parent becomes the new node's parent. + /// + /// # Errors + /// + /// - If the sibling node does not have a parent. + /// - If the attachment would introduce a cycle. + #[inline] + fn add_op_after(&mut self, sibling: Node, op: impl Into) -> Result { + match self.contains_node(sibling) { + true => self.hugr_mut().add_op_after(sibling, op), + false => Err(HugrError::InvalidNode { node: sibling }), + } + } + + /// Remove a node from the graph. + /// + /// # Panics + /// + /// Panics if the node is the root node. + #[inline] + fn remove_node(&mut self, node: Node) -> Result<(), HugrError> { + match self.contains_node(node) { + true => self.hugr_mut().remove_node(node), + false => Err(HugrError::InvalidNode { node }), + } + } - /// Returns a shared view of the Hugr. - fn view(&self) -> Self::View<'_>; + /// Connect two nodes at the given ports. + /// + /// The port must have already been created. See [`add_ports`] and [`set_num_ports`]. + /// + /// [`add_ports`]: #method.add_ports + /// [`set_num_ports`]: #method.set_num_ports. + #[inline] + fn connect( + &mut self, + src: Node, + src_port: usize, + dst: Node, + dst_port: usize, + ) -> Result<(), HugrError> { + match (self.contains_node(src), self.contains_node(dst)) { + (true, true) => self.hugr_mut().connect(src, src_port, dst, dst_port), + (false, _) => Err(HugrError::InvalidNode { node: src }), + (_, false) => Err(HugrError::InvalidNode { node: dst }), + } + } + + /// Disconnects all edges from the given port. + /// + /// The port is left in place. + #[inline] + fn disconnect(&mut self, node: Node, port: Port) -> Result<(), HugrError> { + match self.contains_node(node) { + true => self.hugr_mut().disconnect(node, port), + false => Err(HugrError::InvalidNode { node }), + } + } + + /// Adds a non-dataflow edge between two nodes. The kind is given by the + /// operation's [`OpType::other_input`] or [`OpType::other_output`]. + /// + /// Returns the offsets of the new input and output ports, or an error if + /// the connection failed. + /// + /// [`OpType::other_input`]: crate::ops::OpType::other_input + /// [`OpType::other_output`]: crate::ops::OpType::other_output. + fn add_other_edge(&mut self, src: Node, dst: Node) -> Result<(Port, Port), HugrError> { + match (self.contains_node(src), self.contains_node(dst)) { + (true, true) => self.hugr_mut().add_other_edge(src, dst), + (false, _) => Err(HugrError::InvalidNode { node: src }), + (_, false) => Err(HugrError::InvalidNode { node: dst }), + } + } + + /// Insert another hugr into this one, under a given root node. + /// + /// Returns the root node of the inserted hugr. + #[inline] + fn insert_hugr(&mut self, root: Node, other: Hugr) -> Result { + match self.contains_node(root) { + true => self.hugr_mut().insert_hugr(root, other), + false => Err(HugrError::InvalidNode { node: root }), + } + } + + /// Copy another hugr into this one, under a given root node. + /// + /// Returns the root node of the inserted hugr. + #[inline] + fn insert_from_view(&mut self, root: Node, other: &impl HugrView) -> Result { + match self.contains_node(root) { + true => self.hugr_mut().insert_from_view(root, other), + false => Err(HugrError::InvalidNode { node: root }), + } + } } +/// Impl for non-wrapped Hugrs. Overwrites the recursive default-impls to directly use the hugr. impl HugrMut for T where - T: AsRef + AsMut, + T: HugrView + AsMut, { - type RootHandle = Node; + fn get_metadata_mut(&mut self, node: Node) -> &mut NodeMetadata { + self.as_mut().metadata.get_mut(node.index) + } - type View<'h> = &'h Hugr where Self: 'h; + fn add_op_with_parent( + &mut self, + parent: Node, + op: impl Into, + ) -> Result { + // TODO: Default to `NodeType::open_resources` once we can infer resources + self.add_node_with_parent(parent, NodeType::pure(op)) + } - fn view(&self) -> Self::View<'_> { - self.as_ref() + fn add_node_with_parent(&mut self, parent: Node, node: NodeType) -> Result { + let node = self.add_node(node); + self.as_mut() + .hierarchy + .push_child(node.index, parent.index)?; + Ok(node) } -} -pub(crate) mod sealed { - use super::*; - /// Functions for low-level building of a HUGR. (Or, in the future, a subregion thereof) - pub trait HugrInternalsMut { - /// Add a node to the graph, with the default conversion from OpType to NodeType - fn add_op(&mut self, op: impl Into) -> Node; + fn add_op_before(&mut self, sibling: Node, op: impl Into) -> Result { + let node = self.add_op(op); + self.as_mut() + .hierarchy + .insert_before(node.index, sibling.index)?; + Ok(node) + } - /// Add a node to the graph. - fn add_node(&mut self, node: NodeType) -> Node; + fn add_op_after(&mut self, sibling: Node, op: impl Into) -> Result { + let node = self.add_op(op); + self.as_mut() + .hierarchy + .insert_after(node.index, sibling.index)?; + Ok(node) + } - /// Remove a node from the graph. - /// - /// # Panics - /// - /// Panics if the node is the root node. - fn remove_node(&mut self, node: Node) -> Result<(), HugrError>; + fn remove_node(&mut self, node: Node) -> Result<(), HugrError> { + if node == self.root() { + // TODO: Add a HugrMutError ? + panic!("cannot remove root node"); + } + self.as_mut().hierarchy.remove(node.index); + self.as_mut().graph.remove_node(node.index); + self.as_mut().op_types.remove(node.index); + Ok(()) + } + + fn connect( + &mut self, + src: Node, + src_port: usize, + dst: Node, + dst_port: usize, + ) -> Result<(), HugrError> { + self.as_mut() + .graph + .link_nodes(src.index, src_port, dst.index, dst_port)?; + Ok(()) + } + + fn disconnect(&mut self, node: Node, port: Port) -> Result<(), HugrError> { + let offset = port.offset; + let port = self.as_mut().graph.port_index(node.index, offset).ok_or( + portgraph::LinkError::UnknownOffset { + node: node.index, + offset, + }, + )?; + self.as_mut().graph.unlink_port(port); + Ok(()) + } - /// Returns the metadata associated with a node. - fn get_metadata_mut(&mut self, node: Node) -> &mut NodeMetadata; + fn add_other_edge(&mut self, src: Node, dst: Node) -> Result<(Port, Port), HugrError> { + let src_port: Port = self + .get_optype(src) + .other_port_index(Direction::Outgoing) + .expect("Source operation has no non-dataflow outgoing edges"); + let dst_port: Port = self + .get_optype(dst) + .other_port_index(Direction::Incoming) + .expect("Destination operation has no non-dataflow incoming edges"); + self.connect(src, src_port.index(), dst, dst_port.index())?; + Ok((src_port, dst_port)) + } - /// Sets the metadata associated with a node. - fn set_metadata(&mut self, node: Node, metadata: NodeMetadata) { - *self.get_metadata_mut(node) = metadata; + fn insert_hugr(&mut self, root: Node, mut other: Hugr) -> Result { + let (other_root, node_map) = insert_hugr_internal(self.as_mut(), root, &other)?; + // Update the optypes and metadata, taking them from the other graph. + for (&node, &new_node) in node_map.iter() { + let optype = other.op_types.take(node); + self.as_mut().op_types.set(new_node, optype); + let meta = other.metadata.take(node); + self.as_mut().set_metadata(node.into(), meta); } + Ok(other_root) + } - /// Connect two nodes at the given ports. - /// - /// The port must have already been created. See [`add_ports`] and [`set_num_ports`]. - /// - /// [`add_ports`]: #method.add_ports - /// [`set_num_ports`]: #method.set_num_ports. - fn connect( - &mut self, - src: Node, - src_port: usize, - dst: Node, - dst_port: usize, - ) -> Result<(), HugrError>; - - /// Disconnects all edges from the given port. - /// - /// The port is left in place. - fn disconnect(&mut self, node: Node, port: Port) -> Result<(), HugrError>; + fn insert_from_view(&mut self, root: Node, other: &impl HugrView) -> Result { + let (other_root, node_map) = insert_hugr_internal(self.as_mut(), root, other)?; + // Update the optypes and metadata, copying them from the other graph. + for (&node, &new_node) in node_map.iter() { + let nodetype = other.get_nodetype(node.into()); + self.as_mut().op_types.set(new_node, nodetype.clone()); + let meta = other.get_metadata(node.into()); + self.as_mut().set_metadata(node.into(), meta.clone()); + } + Ok(other_root) + } +} - /// Adds a non-dataflow edge between two nodes. The kind is given by the - /// operation's [`OpType::other_input`] or [`OpType::other_output`]. - /// - /// Returns the offsets of the new input and output ports, or an error if - /// the connection failed. - /// - /// [`OpType::other_input`]: crate::ops::OpType::other_input - /// [`OpType::other_output`]: crate::ops::OpType::other_output. - fn add_other_edge(&mut self, src: Node, dst: Node) -> Result<(Port, Port), HugrError>; +/// Internal implementation of `insert_hugr` and `insert_view` methods for +/// AsMut. +/// +/// Returns the root node of the inserted hierarchy and a mapping from the nodes +/// in the inserted graph to their new indices in `hugr`. +/// +/// This function does not update the optypes of the inserted nodes, so the +/// caller must do that. +fn insert_hugr_internal( + hugr: &mut Hugr, + root: Node, + other: &impl HugrView, +) -> Result<(Node, HashMap), HugrError> { + let node_map = hugr.graph.insert_graph(&other.portgraph())?; + let other_root = node_map[&other.root().index]; + + // Update hierarchy and optypes + hugr.hierarchy.push_child(other_root, root.index)?; + for (&node, &new_node) in node_map.iter() { + other + .children(node.into()) + .try_for_each(|child| -> Result<(), HugrError> { + hugr.hierarchy + .push_child(node_map[&child.index], new_node)?; + Ok(()) + })?; + } + + // The root node didn't have any ports. + let root_optype = other.get_optype(other.root()); + hugr.set_num_ports( + other_root.into(), + root_optype.input_count(), + root_optype.output_count(), + ); + + Ok((other_root.into(), node_map)) +} + +pub(crate) mod sealed { + use super::*; + + /// Trait for accessing the mutable internals of a Hugr(Mut). + /// + /// Specifically, this trait lets you apply arbitrary modifications that may + /// invalidate the HUGR. + pub trait HugrMutInternals: HugrView { + /// Returns the Hugr at the base of a chain of views. + fn hugr_mut(&mut self) -> &mut Hugr; + + /// Add a node to the graph, with the default conversion from OpType to NodeType + fn add_op(&mut self, op: impl Into) -> Node { + // TODO: Default to `NodeType::open_resources` once we can infer resources + self.add_node(NodeType::pure(op)) + } + + /// Add a node to the graph. + fn add_node(&mut self, nodetype: NodeType) -> Node { + let node = self + .hugr_mut() + .graph + .add_node(nodetype.input_count(), nodetype.output_count()); + self.hugr_mut().op_types[node] = nodetype; + node.into() + } /// Set the number of ports on a node. This may invalidate the node's `PortIndex`. - fn set_num_ports(&mut self, node: Node, incoming: usize, outgoing: usize); + fn set_num_ports(&mut self, node: Node, incoming: usize, outgoing: usize) { + match self.contains_node(node) { + true => self.hugr_mut().set_num_ports(node, incoming, outgoing), + false => panic!("Invalid node"), + } + } /// Alter the number of ports on a node and returns a range with the new /// port offsets, if any. This may invalidate the node's `PortIndex`. /// /// The `direction` parameter specifies whether to add ports to the incoming /// or outgoing list. - fn add_ports(&mut self, node: Node, direction: Direction, amount: isize) -> Range; + fn add_ports(&mut self, node: Node, direction: Direction, amount: isize) -> Range { + match self.contains_node(node) { + true => self.hugr_mut().add_ports(node, direction, amount), + false => panic!("Invalid node"), + } + } /// Sets the parent of a node. /// /// The node becomes the parent's last child. - fn set_parent(&mut self, node: Node, parent: Node) -> Result<(), HugrError>; + fn set_parent(&mut self, node: Node, parent: Node) -> Result<(), HugrError> { + match (self.contains_node(node), self.contains_node(parent)) { + (true, true) => self.hugr_mut().set_parent(node, parent), + (false, _) => Err(HugrError::InvalidNode { node }), + (_, false) => Err(HugrError::InvalidNode { node: parent }), + } + } /// Move a node in the hierarchy to be the subsequent sibling of another /// node. @@ -119,166 +385,58 @@ pub(crate) mod sealed { /// The sibling node's parent becomes the new node's parent. /// /// The node becomes the parent's last child. - fn move_after_sibling(&mut self, node: Node, after: Node) -> Result<(), HugrError>; + fn move_after_sibling(&mut self, node: Node, after: Node) -> Result<(), HugrError> { + match (self.contains_node(node), self.contains_node(after)) { + (true, true) => self.hugr_mut().move_after_sibling(node, after), + (false, _) => Err(HugrError::InvalidNode { node }), + (_, false) => Err(HugrError::InvalidNode { node: after }), + } + } /// Move a node in the hierarchy to be the prior sibling of another node. /// /// The sibling node's parent becomes the new node's parent. /// /// The node becomes the parent's last child. - fn move_before_sibling(&mut self, node: Node, before: Node) -> Result<(), HugrError>; - - /// Add a node to the graph with a parent in the hierarchy. - /// - /// The node becomes the parent's last child. - fn add_op_with_parent( - &mut self, - parent: Node, - op: impl Into, - ) -> Result; - - /// Add a node to the graph with a parent in the hierarchy. - /// - /// The node becomes the parent's last child. - fn add_node_with_parent(&mut self, parent: Node, op: NodeType) -> Result; - - /// Add a node to the graph as the previous sibling of another node. - /// - /// The sibling node's parent becomes the new node's parent. - /// - /// # Errors - /// - /// - If the sibling node does not have a parent. - /// - If the attachment would introduce a cycle. - fn add_op_before( - &mut self, - sibling: Node, - op: impl Into, - ) -> Result; - - /// Add a node to the graph as the next sibling of another node. - /// - /// The sibling node's parent becomes the new node's parent. - /// - /// # Errors - /// - /// - If the sibling node does not have a parent. - /// - If the attachment would introduce a cycle. - fn add_op_after(&mut self, sibling: Node, op: impl Into) - -> Result; + fn move_before_sibling(&mut self, node: Node, before: Node) -> Result<(), HugrError> { + match (self.contains_node(node), self.contains_node(before)) { + (true, true) => self.hugr_mut().move_before_sibling(node, before), + (false, _) => Err(HugrError::InvalidNode { node }), + (_, false) => Err(HugrError::InvalidNode { node: before }), + } + } /// Replace the OpType at node and return the old OpType. /// In general this invalidates the ports, which may need to be resized to /// match the OpType signature. /// TODO: Add a version which ignores input resources - fn replace_op(&mut self, node: Node, op: NodeType) -> NodeType; - - /// Insert another hugr into this one, under a given root node. - /// - /// Returns the root node of the inserted hugr. - fn insert_hugr(&mut self, root: Node, other: Hugr) -> Result; - - /// Copy another hugr into this one, under a given root node. - /// - /// Returns the root node of the inserted hugr. - fn insert_from_view( - &mut self, - root: Node, - other: &impl HugrView, - ) -> Result; - - /// Compact the nodes indices of the hugr to be contiguous, and order them as a breadth-first - /// traversal of the hierarchy. - /// - /// The rekey function is called for each moved node with the old and new indices. - /// - /// After this operation, a serialization and deserialization of the Hugr is guaranteed to - /// preserve the indices. - fn canonicalize_nodes(&mut self, rekey: impl FnMut(Node, Node)); + fn replace_op(&mut self, node: Node, op: NodeType) -> NodeType { + match self.contains_node(node) { + true => self.hugr_mut().replace_op(node, op), + false => panic!("Invalid node"), + } + } } - impl HugrInternalsMut for T + /// Impl for non-wrapped Hugrs. Overwrites the recursive default-impls to directly use the hugr. + impl HugrMutInternals for T where - T: AsRef + AsMut, + T: HugrView + AsMut, { - fn add_node(&mut self, nodetype: NodeType) -> Node { - let node = self - .as_mut() - .graph - .add_node(nodetype.input_count(), nodetype.output_count()); - self.as_mut().op_types[node] = nodetype; - node.into() - } - - fn add_op(&mut self, op: impl Into) -> Node { - // TODO: Default to `NodeType::open_resources` once we can infer resources - self.add_node(NodeType::pure(op)) - } - - fn remove_node(&mut self, node: Node) -> Result<(), HugrError> { - if node.index == self.as_ref().root { - // TODO: Add a HugrMutError ? - panic!("cannot remove root node"); - } - self.as_mut().hierarchy.remove(node.index); - self.as_mut().graph.remove_node(node.index); - self.as_mut().op_types.remove(node.index); - Ok(()) - } - - fn get_metadata_mut(&mut self, node: Node) -> &mut NodeMetadata { - self.as_mut().metadata.get_mut(node.index) - } - - fn connect( - &mut self, - src: Node, - src_port: usize, - dst: Node, - dst_port: usize, - ) -> Result<(), HugrError> { + fn hugr_mut(&mut self) -> &mut Hugr { self.as_mut() - .graph - .link_nodes(src.index, src_port, dst.index, dst_port)?; - Ok(()) - } - - fn disconnect(&mut self, node: Node, port: Port) -> Result<(), HugrError> { - let offset = port.offset; - let port = self.as_mut().graph.port_index(node.index, offset).ok_or( - portgraph::LinkError::UnknownOffset { - node: node.index, - offset, - }, - )?; - self.as_mut().graph.unlink_port(port); - Ok(()) - } - - fn add_other_edge(&mut self, src: Node, dst: Node) -> Result<(Port, Port), HugrError> { - let src_port: Port = self - .get_optype(src) - .other_port_index(Direction::Outgoing) - .expect("Source operation has no non-dataflow outgoing edges"); - let dst_port: Port = self - .get_optype(dst) - .other_port_index(Direction::Incoming) - .expect("Destination operation has no non-dataflow incoming edges"); - self.connect(src, src_port.index(), dst, dst_port.index())?; - Ok((src_port, dst_port)) } #[inline] fn set_num_ports(&mut self, node: Node, incoming: usize, outgoing: usize) { - self.as_mut() + self.hugr_mut() .graph .set_num_ports(node.index, incoming, outgoing, |_, _| {}) } - #[inline] fn add_ports(&mut self, node: Node, direction: Direction, amount: isize) -> Range { - let mut incoming = self.as_mut().graph.num_inputs(node.index); - let mut outgoing = self.as_mut().graph.num_outputs(node.index); + let mut incoming = self.hugr_mut().graph.num_inputs(node.index); + let mut outgoing = self.hugr_mut().graph.num_outputs(node.index); let increment = |num: &mut usize| { let new = num.saturating_add_signed(amount); let range = *num..new; @@ -289,185 +447,40 @@ pub(crate) mod sealed { Direction::Incoming => increment(&mut incoming), Direction::Outgoing => increment(&mut outgoing), }; - self.as_mut() + self.hugr_mut() .graph .set_num_ports(node.index, incoming, outgoing, |_, _| {}); range } fn set_parent(&mut self, node: Node, parent: Node) -> Result<(), HugrError> { - self.as_mut().hierarchy.detach(node.index); - self.as_mut() + self.hugr_mut().hierarchy.detach(node.index); + self.hugr_mut() .hierarchy .push_child(node.index, parent.index)?; Ok(()) } fn move_after_sibling(&mut self, node: Node, after: Node) -> Result<(), HugrError> { - self.as_mut().hierarchy.detach(node.index); - self.as_mut() + self.hugr_mut().hierarchy.detach(node.index); + self.hugr_mut() .hierarchy .insert_after(node.index, after.index)?; Ok(()) } fn move_before_sibling(&mut self, node: Node, before: Node) -> Result<(), HugrError> { - self.as_mut().hierarchy.detach(node.index); - self.as_mut() + self.hugr_mut().hierarchy.detach(node.index); + self.hugr_mut() .hierarchy .insert_before(node.index, before.index)?; Ok(()) } - fn add_op_with_parent( - &mut self, - parent: Node, - op: impl Into, - ) -> Result { - // TODO: Default to `NodeType::open_resources` once we can infer resources - self.add_node_with_parent(parent, NodeType::pure(op)) - } - - fn add_node_with_parent( - &mut self, - parent: Node, - node: NodeType, - ) -> Result { - let node = self.add_node(node); - self.as_mut() - .hierarchy - .push_child(node.index, parent.index)?; - Ok(node) - } - - fn add_op_before( - &mut self, - sibling: Node, - op: impl Into, - ) -> Result { - let node = self.add_op(op); - self.as_mut() - .hierarchy - .insert_before(node.index, sibling.index)?; - Ok(node) - } - - fn add_op_after( - &mut self, - sibling: Node, - op: impl Into, - ) -> Result { - let node = self.add_op(op); - self.as_mut() - .hierarchy - .insert_after(node.index, sibling.index)?; - Ok(node) - } - fn replace_op(&mut self, node: Node, op: NodeType) -> NodeType { - let cur = self.as_mut().op_types.get_mut(node.index); + let cur = self.hugr_mut().op_types.get_mut(node.index); std::mem::replace(cur, op) } - - fn insert_hugr(&mut self, root: Node, mut other: Hugr) -> Result { - let (other_root, node_map) = insert_hugr_internal(self.as_mut(), root, &other)?; - // Update the optypes and metadata, taking them from the other graph. - for (&node, &new_node) in node_map.iter() { - let optype = other.op_types.take(node); - self.as_mut().op_types.set(new_node, optype); - let meta = other.metadata.take(node); - self.as_mut().set_metadata(node.into(), meta); - } - Ok(other_root) - } - - fn insert_from_view( - &mut self, - root: Node, - other: &impl HugrView, - ) -> Result { - let (other_root, node_map) = insert_hugr_internal(self.as_mut(), root, other)?; - // Update the optypes and metadata, copying them from the other graph. - for (&node, &new_node) in node_map.iter() { - let nodetype = other.get_nodetype(node.into()); - self.as_mut().op_types.set(new_node, nodetype.clone()); - let meta = other.get_metadata(node.into()); - self.as_mut().set_metadata(node.into(), meta.clone()); - } - Ok(other_root) - } - - fn canonicalize_nodes(&mut self, mut rekey: impl FnMut(Node, Node)) { - // Generate the ordered list of nodes - let mut ordered = Vec::with_capacity(self.node_count()); - ordered.extend(self.as_ref().canonical_order()); - - // Permute the nodes in the graph to match the order. - // - // Invariant: All the elements before `position` are in the correct place. - for position in 0..ordered.len() { - // Find the element's location. If it originally came from a previous position - // then it has been swapped somewhere else, so we follow the permutation chain. - let mut source: Node = ordered[position]; - while position > source.index.index() { - source = ordered[source.index.index()]; - } - - let target: Node = NodeIndex::new(position).into(); - if target != source { - let hugr = self.as_mut(); - hugr.graph.swap_nodes(target.index, source.index); - hugr.op_types.swap(target.index, source.index); - hugr.hierarchy.swap_nodes(target.index, source.index); - rekey(source, target); - } - } - self.as_mut().root = NodeIndex::new(0); - - // Finish by compacting the copy nodes. - // The operation nodes will be left in place. - // This step is not strictly necessary. - self.as_mut().graph.compact_nodes(|_, _| {}); - } - } - - /// Internal implementation of `insert_hugr` and `insert_view` methods for - /// AsMut. - /// - /// Returns the root node of the inserted hierarchy and a mapping from the nodes - /// in the inserted graph to their new indices in `hugr`. - /// - /// This function does not update the optypes of the inserted nodes, so the - /// caller must do that. - fn insert_hugr_internal( - hugr: &mut Hugr, - root: Node, - other: &impl HugrView, - ) -> Result<(Node, HashMap), HugrError> { - let node_map = hugr.graph.insert_graph(other.portgraph())?; - let other_root = node_map[&other.root().index]; - - // Update hierarchy and optypes - hugr.hierarchy.push_child(other_root, root.index)?; - for (&node, &new_node) in node_map.iter() { - other - .children(node.into()) - .try_for_each(|child| -> Result<(), HugrError> { - hugr.hierarchy - .push_child(node_map[&child.index], new_node)?; - Ok(()) - })?; - } - - // The root node didn't have any ports. - let root_optype = other.get_optype(other.root()); - hugr.set_num_ports( - other_root.into(), - root_optype.input_count(), - root_optype.output_count(), - ); - - Ok((other_root.into(), node_map)) } } @@ -480,7 +493,6 @@ mod test { types::{AbstractSignature, ClassicType, SimpleType}, }; - use super::sealed::HugrInternalsMut; use super::*; const NAT: SimpleType = SimpleType::Classic(ClassicType::i64()); diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index ddbbd9c8c..b93ccbace 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -5,8 +5,9 @@ use itertools::Itertools; use thiserror::Error; use crate::builder::{BlockBuilder, Container, Dataflow, SubContainer}; +use crate::hugr::hugrmut::sealed::HugrMutInternals; use crate::hugr::rewrite::Rewrite; -use crate::hugr::{HugrInternalsMut, HugrView}; +use crate::hugr::{HugrMut, HugrView}; use crate::ops; use crate::ops::{BasicBlock, OpTag, OpTrait, OpType}; use crate::{type_row, Hugr, Node}; diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index 296faee5d..57df3cc1d 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -4,7 +4,7 @@ use std::collections::{HashMap, HashSet}; use itertools::Itertools; -use crate::hugr::{HugrInternalsMut, HugrView, NodeMetadata}; +use crate::hugr::{HugrMut, HugrView, NodeMetadata}; use crate::{ hugr::{Node, Rewrite}, ops::{OpTag, OpTrait, OpType}, diff --git a/src/hugr/serialize.rs b/src/hugr/serialize.rs index 8793d3b9f..f8e4567fe 100644 --- a/src/hugr/serialize.rs +++ b/src/hugr/serialize.rs @@ -8,7 +8,7 @@ use thiserror::Error; #[cfg(feature = "pyo3")] use pyo3::prelude::*; -use crate::hugr::{Hugr, HugrInternalsMut, NodeType}; +use crate::hugr::{Hugr, NodeType}; use crate::ops::OpTrait; use crate::ops::OpType; use crate::resource::ResourceSet; @@ -18,7 +18,7 @@ use portgraph::{Direction, LinkError, NodeIndex, PortView}; use serde::{Deserialize, Deserializer, Serialize}; -use super::{HugrError, HugrView}; +use super::{HugrError, HugrMut, HugrView}; /// A wrapper over the available HUGR serialization formats. /// @@ -129,7 +129,7 @@ impl TryFrom<&Hugr> for SerHugrV0 { // We compact the operation nodes during the serialization process, // and ignore the copy nodes. let mut node_rekey: HashMap = HashMap::with_capacity(hugr.node_count()); - for (order, node) in hugr.canonical_order().enumerate() { + for (order, node) in hugr.canonical_order(hugr.root()).enumerate() { node_rekey.insert(node, NodeIndex::new(order).into()); } @@ -267,6 +267,7 @@ impl TryFrom for Hugr { pub mod test { use super::*; + use crate::hugr::hugrmut::sealed::HugrMutInternals; use crate::{ builder::{ Container, DFGBuilder, Dataflow, DataflowHugr, DataflowSubContainer, HugrBuilder, diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index e6b7484f2..3fbff9239 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -672,7 +672,8 @@ mod test { BuildError, Container, DFGBuilder, Dataflow, DataflowHugr, DataflowSubContainer, HugrBuilder, ModuleBuilder, }; - use crate::hugr::{HugrError, HugrInternalsMut, NodeType}; + use crate::hugr::hugrmut::sealed::HugrMutInternals; + use crate::hugr::{HugrError, HugrMut, NodeType}; use crate::ops::dataflow::IOTrait; use crate::ops::{self, LeafOp, OpType}; use crate::resource::ResourceSet; diff --git a/src/hugr/views.rs b/src/hugr/views.rs index 23bce9450..856435869 100644 --- a/src/hugr/views.rs +++ b/src/hugr/views.rs @@ -50,14 +50,23 @@ pub trait HugrView: sealed::HugrInternals { where Self: 'a; - /// Return index of HUGR root node. - fn root(&self) -> Node; + /// Return the root node of this view. + #[inline] + fn root(&self) -> Node { + self.root_node() + } /// Return the type of the HUGR root node. + #[inline] fn root_type(&self) -> &NodeType { - self.get_nodetype(self.root()) + let node_type = self.get_nodetype(self.root()); + debug_assert!(Self::RootHandle::can_hold(node_type.tag())); + node_type } + /// Returns whether the node exists. + fn contains_node(&self, node: Node) -> bool; + /// Returns the parent of a node. fn get_parent(&self, node: Node) -> Option; @@ -226,8 +235,8 @@ where Self: 'a; #[inline] - fn root(&self) -> Node { - self.as_ref().root.into() + fn contains_node(&self, node: Node) -> bool { + self.as_ref().graph.contains_node(node.index) } #[inline] @@ -330,23 +339,28 @@ pub(crate) mod sealed { /// view. pub trait HugrInternals { /// The underlying portgraph view type. - type Portgraph: LinkView; + type Portgraph<'p>: LinkView + 'p + where + Self: 'p; /// Returns a reference to the underlying portgraph. - fn portgraph(&self) -> &Self::Portgraph; + fn portgraph(&self) -> Self::Portgraph<'_>; /// Returns the Hugr at the base of a chain of views. fn base_hugr(&self) -> &Hugr; + + /// Return the root node of this view. + fn root_node(&self) -> Node; } impl HugrInternals for T where T: AsRef, { - type Portgraph = MultiPortGraph; + type Portgraph<'p> = &'p MultiPortGraph where Self: 'p; #[inline] - fn portgraph(&self) -> &Self::Portgraph { + fn portgraph(&self) -> Self::Portgraph<'_> { &self.as_ref().graph } @@ -354,5 +368,10 @@ pub(crate) mod sealed { fn base_hugr(&self) -> &Hugr { self.as_ref() } + + #[inline] + fn root_node(&self) -> Node { + self.as_ref().root.into() + } } } diff --git a/src/hugr/views/hierarchy.rs b/src/hugr/views/hierarchy.rs index 995d96578..021a54a09 100644 --- a/src/hugr/views/hierarchy.rs +++ b/src/hugr/views/hierarchy.rs @@ -21,7 +21,7 @@ use std::iter; use ::petgraph::visit as pv; use context_iterators::{ContextIterator, IntoContextIterator, MapWithCtx}; use itertools::{Itertools, MapInto}; -use portgraph::{LinkView, PortIndex, PortView}; +use portgraph::{LinkView, MultiPortGraph, PortIndex, PortView}; use crate::ops::handle::NodeHandle; use crate::ops::OpTrait; @@ -29,8 +29,7 @@ use crate::{hugr::NodeType, hugr::OpType, Direction, Hugr, Node, Port}; use super::{sealed::HugrInternals, HugrView, NodeMetadata}; -type FlatRegionGraph<'g, Base> = - portgraph::view::FlatRegion<'g, ::Portgraph>; +type FlatRegionGraph<'g> = portgraph::view::FlatRegion<'g, MultiPortGraph>; /// View of a HUGR sibling graph. /// @@ -45,7 +44,7 @@ where root: Node, /// The filtered portgraph encoding the adjacency structure of the HUGR. - graph: FlatRegionGraph<'g, Base>, + graph: FlatRegionGraph<'g>, /// The rest of the HUGR. hugr: &'g Base, @@ -54,6 +53,20 @@ where _phantom: std::marker::PhantomData, } +impl<'g, Root, Base> SiblingGraph<'g, Root, Base> +where + Base: HugrInternals, +{ + /// Helper function to initialize the internal portgraph view. + fn init_graph(hugr: &Base, root: Node) -> FlatRegionGraph<'_> { + FlatRegionGraph::new_flat_region( + &hugr.base_hugr().graph, + &hugr.base_hugr().hierarchy, + root.index, + ) + } +} + impl<'g, Root, Base> Clone for SiblingGraph<'g, Root, Base> where Root: NodeHandle, @@ -75,7 +88,7 @@ where where Self: 'a; - type NodePorts<'a> = MapInto< as PortView>::NodePortOffsets<'a>, Port> + type NodePorts<'a> = MapInto< as PortView>::NodePortOffsets<'a>, Port> where Self: 'a; @@ -83,20 +96,20 @@ where where Self: 'a; - type Neighbours<'a> = MapInto< as LinkView>::Neighbours<'a>, Node> + type Neighbours<'a> = MapInto< as LinkView>::Neighbours<'a>, Node> where Self: 'a; type PortLinks<'a> = MapWithCtx< - as LinkView>::PortLinks<'a>, + as LinkView>::PortLinks<'a>, &'a Self, (Node, Port), > where Self: 'a; #[inline] - fn root(&self) -> Node { - self.root + fn contains_node(&self, node: Node) -> bool { + self.graph.contains_node(node.index) } #[inline] @@ -199,7 +212,7 @@ where } } -type RegionGraph<'g, Base> = portgraph::view::Region<'g, ::Portgraph>; +type RegionGraph<'g> = portgraph::view::Region<'g, MultiPortGraph>; /// View of a HUGR descendants graph. /// @@ -216,7 +229,7 @@ where root: Node, /// The graph encoding the adjacency structure of the HUGR. - graph: RegionGraph<'g, Base>, + graph: RegionGraph<'g>, /// The node hierarchy. hugr: &'g Base, @@ -235,6 +248,20 @@ where } } +impl<'g, Root, Base> DescendantsGraph<'g, Root, Base> +where + Base: HugrInternals, +{ + /// Helper function to initialize the internal portgraph view. + fn init_graph(hugr: &Base, root: Node) -> RegionGraph<'_> { + RegionGraph::new_region( + &hugr.base_hugr().graph, + &hugr.base_hugr().hierarchy, + root.index, + ) + } +} + impl<'g, Root, Base> HugrView for DescendantsGraph<'g, Root, Base> where Root: NodeHandle, @@ -242,11 +269,11 @@ where { type RootHandle = Root; - type Nodes<'a> = MapInto< as PortView>::Nodes<'a>, Node> + type Nodes<'a> = MapInto< as PortView>::Nodes<'a>, Node> where Self: 'a; - type NodePorts<'a> = MapInto< as PortView>::NodePortOffsets<'a>, Port> + type NodePorts<'a> = MapInto< as PortView>::NodePortOffsets<'a>, Port> where Self: 'a; @@ -254,20 +281,20 @@ where where Self: 'a; - type Neighbours<'a> = MapInto< as LinkView>::Neighbours<'a>, Node> + type Neighbours<'a> = MapInto< as LinkView>::Neighbours<'a>, Node> where Self: 'a; type PortLinks<'a> = MapWithCtx< - as LinkView>::PortLinks<'a>, + as LinkView>::PortLinks<'a>, &'a Self, (Node, Port), > where Self: 'a; #[inline] - fn root(&self) -> Node { - self.root + fn contains_node(&self, node: Node) -> bool { + self.graph.contains_node(node.index) } #[inline] @@ -396,11 +423,7 @@ where } Self { root, - graph: FlatRegionGraph::::new_flat_region( - hugr.portgraph(), - &hugr.base_hugr().hierarchy, - root.index, - ), + graph: Self::init_graph(hugr, root), hugr, _phantom: std::marker::PhantomData, } @@ -422,11 +445,7 @@ where } Self { root, - graph: RegionGraph::::new_region( - hugr.portgraph(), - &hugr.base_hugr().hierarchy, - root.index, - ), + graph: Self::init_graph(hugr, root), hugr, _phantom: std::marker::PhantomData, } @@ -438,17 +457,22 @@ where Root: NodeHandle, Base: HugrInternals, { - type Portgraph = FlatRegionGraph<'g, Base>; + type Portgraph<'p> = FlatRegionGraph<'p> where Self: 'p; #[inline] - fn portgraph(&self) -> &Self::Portgraph { - &self.graph + fn portgraph(&self) -> Self::Portgraph<'_> { + Self::init_graph(self.hugr, self.root) } #[inline] fn base_hugr(&self) -> &Hugr { self.hugr.base_hugr() } + + #[inline] + fn root_node(&self) -> Node { + self.root + } } impl<'g, Root, Base> super::sealed::HugrInternals for DescendantsGraph<'g, Root, Base> @@ -456,17 +480,22 @@ where Root: NodeHandle, Base: HugrInternals, { - type Portgraph = RegionGraph<'g, Base>; + type Portgraph<'p> = RegionGraph<'p> where Self: 'p; #[inline] - fn portgraph(&self) -> &Self::Portgraph { - &self.graph + fn portgraph(&self) -> Self::Portgraph<'_> { + Self::init_graph(self.hugr, self.root) } #[inline] fn base_hugr(&self) -> &Hugr { self.hugr.base_hugr() } + + #[inline] + fn root_node(&self) -> Node { + self.root + } } #[cfg(test)] diff --git a/src/ops/custom.rs b/src/ops/custom.rs index 969c95fc1..0dd1e1b01 100644 --- a/src/ops/custom.rs +++ b/src/ops/custom.rs @@ -5,7 +5,8 @@ use std::collections::HashMap; use std::sync::Arc; use thiserror::Error; -use crate::hugr::{HugrInternalsMut, HugrView, NodeType}; +use crate::hugr::hugrmut::sealed::HugrMutInternals; +use crate::hugr::{HugrView, NodeType}; use crate::resource::{OpDef, ResourceId, SignatureError}; use crate::types::{type_param::TypeArg, AbstractSignature, SignatureDescription}; use crate::{Hugr, Node, Resource}; diff --git a/src/ops/handle.rs b/src/ops/handle.rs index 753870f53..45e7d6d7b 100644 --- a/src/ops/handle.rs +++ b/src/ops/handle.rs @@ -27,6 +27,11 @@ pub trait NodeHandle: Clone { fn try_cast>(&self) -> Option { T::TAG.is_superset(Self::TAG).then(|| self.node().into()) } + + /// Checks whether the handle can hold an operation with the given tag. + fn can_hold(tag: OpTag) -> bool { + Self::TAG.is_superset(tag) + } } /// Trait for handles that contain children. From 5cbac2edf12bf4b5ec5a04cd2adcb127a67f840f Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Mon, 21 Aug 2023 23:48:30 +0100 Subject: [PATCH 02/10] rollback HugrInternals::Portgraph lifetime param It clashes with the new convexCheck in portgraph since it requires `Copy` on the LinkView :/ --- src/hugr/views.rs | 10 ++++------ src/hugr/views/hierarchy.rs | 12 ++++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/hugr/views.rs b/src/hugr/views.rs index eafb3dcb0..140641507 100644 --- a/src/hugr/views.rs +++ b/src/hugr/views.rs @@ -354,12 +354,10 @@ pub(crate) mod sealed { /// view. pub trait HugrInternals { /// The underlying portgraph view type. - type Portgraph<'p>: LinkView + 'p - where - Self: 'p; + type Portgraph: LinkView; /// Returns a reference to the underlying portgraph. - fn portgraph(&self) -> Self::Portgraph<'_>; + fn portgraph(&self) -> &Self::Portgraph; /// Returns the Hugr at the base of a chain of views. fn base_hugr(&self) -> &Hugr; @@ -372,10 +370,10 @@ pub(crate) mod sealed { where T: AsRef, { - type Portgraph<'p> = &'p MultiPortGraph where Self: 'p; + type Portgraph = MultiPortGraph; #[inline] - fn portgraph(&self) -> Self::Portgraph<'_> { + fn portgraph(&self) -> &Self::Portgraph { &self.as_ref().graph } diff --git a/src/hugr/views/hierarchy.rs b/src/hugr/views/hierarchy.rs index e04901d78..3f00366da 100644 --- a/src/hugr/views/hierarchy.rs +++ b/src/hugr/views/hierarchy.rs @@ -465,11 +465,11 @@ where Root: NodeHandle, Base: HugrInternals, { - type Portgraph<'p> = FlatRegionGraph<'p> where Self: 'p; + type Portgraph = FlatRegionGraph<'g>; #[inline] - fn portgraph(&self) -> Self::Portgraph<'_> { - Self::init_graph(self.hugr, self.root) + fn portgraph(&self) -> &Self::Portgraph { + &self.graph } #[inline] @@ -488,11 +488,11 @@ where Root: NodeHandle, Base: HugrInternals, { - type Portgraph<'p> = RegionGraph<'p> where Self: 'p; + type Portgraph = RegionGraph<'g>; #[inline] - fn portgraph(&self) -> Self::Portgraph<'_> { - Self::init_graph(self.hugr, self.root) + fn portgraph(&self) -> &Self::Portgraph { + &self.graph } #[inline] From 2eff599df67e1413a57eda65a42ff8e8fd511830 Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Tue, 22 Aug 2023 00:31:00 +0100 Subject: [PATCH 03/10] fix doc --- src/hugr/hugrmut.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 8c98788cb..75cae000a 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -128,13 +128,13 @@ pub trait HugrMut: HugrView + HugrMutInternals { } /// Adds a non-dataflow edge between two nodes. The kind is given by the - /// operation's [`OpType::other_input`] or [`OpType::other_output`]. + /// operation's [`OpTrait::other_input`] or [`OpTrait::other_output`]. /// /// Returns the offsets of the new input and output ports, or an error if /// the connection failed. /// - /// [`OpType::other_input`]: crate::ops::OpType::other_input - /// [`OpType::other_output`]: crate::ops::OpType::other_output. + /// [`OpTrait::other_input`]: crate::ops::OpTrait::other_input + /// [`OpTrait::other_output`]: crate::ops::OpTrait::other_output fn add_other_edge(&mut self, src: Node, dst: Node) -> Result<(Port, Port), HugrError> { match (self.contains_node(src), self.contains_node(dst)) { (true, true) => self.hugr_mut().add_other_edge(src, dst), From f14907b7fdc21299cfc3a3d09c71b22380b3d07e Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Tue, 22 Aug 2023 01:02:14 +0100 Subject: [PATCH 04/10] Simplify the horrible boilerplate co-authored-by: Seyon Sivarajah --- src/hugr/hugrmut.rs | 116 ++++++++++++++++++-------------------------- 1 file changed, 47 insertions(+), 69 deletions(-) diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 75cae000a..4cd96d570 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -33,10 +33,8 @@ pub trait HugrMut: HugrView + HugrMutInternals { parent: Node, op: impl Into, ) -> Result { - match self.contains_node(parent) { - true => self.hugr_mut().add_op_with_parent(parent, op), - false => Err(HugrError::InvalidNode { node: parent }), - } + self.valid_node(parent)?; + self.hugr_mut().add_op_with_parent(parent, op) } /// Add a node to the graph with a parent in the hierarchy. @@ -44,10 +42,8 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// The node becomes the parent's last child. #[inline] fn add_node_with_parent(&mut self, parent: Node, op: NodeType) -> Result { - match self.contains_node(parent) { - true => self.hugr_mut().add_node_with_parent(parent, op), - false => Err(HugrError::InvalidNode { node: parent }), - } + self.valid_node(parent)?; + self.hugr_mut().add_node_with_parent(parent, op) } /// Add a node to the graph as the previous sibling of another node. @@ -60,10 +56,8 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// - If the attachment would introduce a cycle. #[inline] fn add_op_before(&mut self, sibling: Node, op: impl Into) -> Result { - match self.contains_node(sibling) { - true => self.hugr_mut().add_op_before(sibling, op), - false => Err(HugrError::InvalidNode { node: sibling }), - } + self.valid_node(sibling)?; + self.hugr_mut().add_op_before(sibling, op) } /// Add a node to the graph as the next sibling of another node. @@ -76,10 +70,8 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// - If the attachment would introduce a cycle. #[inline] fn add_op_after(&mut self, sibling: Node, op: impl Into) -> Result { - match self.contains_node(sibling) { - true => self.hugr_mut().add_op_after(sibling, op), - false => Err(HugrError::InvalidNode { node: sibling }), - } + self.valid_node(sibling)?; + self.hugr_mut().add_op_after(sibling, op) } /// Remove a node from the graph. @@ -89,10 +81,8 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// Panics if the node is the root node. #[inline] fn remove_node(&mut self, node: Node) -> Result<(), HugrError> { - match self.contains_node(node) { - true => self.hugr_mut().remove_node(node), - false => Err(HugrError::InvalidNode { node }), - } + self.valid_node(node)?; + self.hugr_mut().remove_node(node) } /// Connect two nodes at the given ports. @@ -109,11 +99,9 @@ pub trait HugrMut: HugrView + HugrMutInternals { dst: Node, dst_port: usize, ) -> Result<(), HugrError> { - match (self.contains_node(src), self.contains_node(dst)) { - (true, true) => self.hugr_mut().connect(src, src_port, dst, dst_port), - (false, _) => Err(HugrError::InvalidNode { node: src }), - (_, false) => Err(HugrError::InvalidNode { node: dst }), - } + self.valid_node(src)?; + self.valid_node(dst)?; + self.hugr_mut().connect(src, src_port, dst, dst_port) } /// Disconnects all edges from the given port. @@ -121,10 +109,8 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// The port is left in place. #[inline] fn disconnect(&mut self, node: Node, port: Port) -> Result<(), HugrError> { - match self.contains_node(node) { - true => self.hugr_mut().disconnect(node, port), - false => Err(HugrError::InvalidNode { node }), - } + self.valid_node(node)?; + self.hugr_mut().disconnect(node, port) } /// Adds a non-dataflow edge between two nodes. The kind is given by the @@ -136,11 +122,9 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// [`OpTrait::other_input`]: crate::ops::OpTrait::other_input /// [`OpTrait::other_output`]: crate::ops::OpTrait::other_output fn add_other_edge(&mut self, src: Node, dst: Node) -> Result<(Port, Port), HugrError> { - match (self.contains_node(src), self.contains_node(dst)) { - (true, true) => self.hugr_mut().add_other_edge(src, dst), - (false, _) => Err(HugrError::InvalidNode { node: src }), - (_, false) => Err(HugrError::InvalidNode { node: dst }), - } + self.valid_node(src)?; + self.valid_node(dst)?; + self.hugr_mut().add_other_edge(src, dst) } /// Insert another hugr into this one, under a given root node. @@ -148,10 +132,8 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// Returns the root node of the inserted hugr. #[inline] fn insert_hugr(&mut self, root: Node, other: Hugr) -> Result { - match self.contains_node(root) { - true => self.hugr_mut().insert_hugr(root, other), - false => Err(HugrError::InvalidNode { node: root }), - } + self.valid_node(root)?; + self.hugr_mut().insert_hugr(root, other) } /// Copy another hugr into this one, under a given root node. @@ -159,10 +141,8 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// Returns the root node of the inserted hugr. #[inline] fn insert_from_view(&mut self, root: Node, other: &impl HugrView) -> Result { - match self.contains_node(root) { - true => self.hugr_mut().insert_from_view(root, other), - false => Err(HugrError::InvalidNode { node: root }), - } + self.valid_node(root)?; + self.hugr_mut().insert_from_view(root, other) } } @@ -332,6 +312,17 @@ pub(crate) mod sealed { /// Returns the Hugr at the base of a chain of views. fn hugr_mut(&mut self) -> &mut Hugr; + /// Validates that a node is valid in the graph. + /// + /// Returns a [`HugrError::InvalidNode`] otherwise. + #[inline] + fn valid_node(&self, node: Node) -> Result<(), HugrError> { + match self.contains_node(node) { + true => Ok(()), + false => Err(HugrError::InvalidNode { node }), + } + } + /// Add a node to the graph, with the default conversion from OpType to NodeType fn add_op(&mut self, op: impl Into) -> Node { // TODO: Default to `NodeType::open_extensions` once we can infer extensions @@ -350,10 +341,8 @@ pub(crate) mod sealed { /// Set the number of ports on a node. This may invalidate the node's `PortIndex`. fn set_num_ports(&mut self, node: Node, incoming: usize, outgoing: usize) { - match self.contains_node(node) { - true => self.hugr_mut().set_num_ports(node, incoming, outgoing), - false => panic!("Invalid node"), - } + self.valid_node(node).unwrap_or_else(|e| panic!("{}", e)); + self.hugr_mut().set_num_ports(node, incoming, outgoing) } /// Alter the number of ports on a node and returns a range with the new @@ -362,21 +351,16 @@ pub(crate) mod sealed { /// The `direction` parameter specifies whether to add ports to the incoming /// or outgoing list. fn add_ports(&mut self, node: Node, direction: Direction, amount: isize) -> Range { - match self.contains_node(node) { - true => self.hugr_mut().add_ports(node, direction, amount), - false => panic!("Invalid node"), - } + self.valid_node(node).unwrap_or_else(|e| panic!("{}", e)); + self.hugr_mut().add_ports(node, direction, amount) } /// Sets the parent of a node. /// /// The node becomes the parent's last child. fn set_parent(&mut self, node: Node, parent: Node) -> Result<(), HugrError> { - match (self.contains_node(node), self.contains_node(parent)) { - (true, true) => self.hugr_mut().set_parent(node, parent), - (false, _) => Err(HugrError::InvalidNode { node }), - (_, false) => Err(HugrError::InvalidNode { node: parent }), - } + self.valid_node(node)?; + self.hugr_mut().set_parent(node, parent) } /// Move a node in the hierarchy to be the subsequent sibling of another @@ -386,11 +370,9 @@ pub(crate) mod sealed { /// /// The node becomes the parent's last child. fn move_after_sibling(&mut self, node: Node, after: Node) -> Result<(), HugrError> { - match (self.contains_node(node), self.contains_node(after)) { - (true, true) => self.hugr_mut().move_after_sibling(node, after), - (false, _) => Err(HugrError::InvalidNode { node }), - (_, false) => Err(HugrError::InvalidNode { node: after }), - } + self.valid_node(node)?; + self.valid_node(after)?; + self.hugr_mut().move_after_sibling(node, after) } /// Move a node in the hierarchy to be the prior sibling of another node. @@ -399,11 +381,9 @@ pub(crate) mod sealed { /// /// The node becomes the parent's last child. fn move_before_sibling(&mut self, node: Node, before: Node) -> Result<(), HugrError> { - match (self.contains_node(node), self.contains_node(before)) { - (true, true) => self.hugr_mut().move_before_sibling(node, before), - (false, _) => Err(HugrError::InvalidNode { node }), - (_, false) => Err(HugrError::InvalidNode { node: before }), - } + self.valid_node(node)?; + self.valid_node(before)?; + self.hugr_mut().move_before_sibling(node, before) } /// Replace the OpType at node and return the old OpType. @@ -411,10 +391,8 @@ pub(crate) mod sealed { /// match the OpType signature. /// TODO: Add a version which ignores input extensions fn replace_op(&mut self, node: Node, op: NodeType) -> NodeType { - match self.contains_node(node) { - true => self.hugr_mut().replace_op(node, op), - false => panic!("Invalid node"), - } + self.valid_node(node).unwrap_or_else(|e| panic!("{}", e)); + self.hugr_mut().replace_op(node, op) } } From 6f90316056dde61da9383b1753abd3b316de9a7e Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Thu, 24 Aug 2023 02:02:59 +0100 Subject: [PATCH 05/10] Inline `init_graph`s --- src/hugr/views/hierarchy.rs | 40 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/src/hugr/views/hierarchy.rs b/src/hugr/views/hierarchy.rs index 3f00366da..98eedd5b4 100644 --- a/src/hugr/views/hierarchy.rs +++ b/src/hugr/views/hierarchy.rs @@ -53,20 +53,6 @@ where _phantom: std::marker::PhantomData, } -impl<'g, Root, Base> SiblingGraph<'g, Root, Base> -where - Base: HugrInternals, -{ - /// Helper function to initialize the internal portgraph view. - fn init_graph(hugr: &Base, root: Node) -> FlatRegionGraph<'_> { - FlatRegionGraph::new_flat_region( - &hugr.base_hugr().graph, - &hugr.base_hugr().hierarchy, - root.index, - ) - } -} - impl<'g, Root, Base> Clone for SiblingGraph<'g, Root, Base> where Root: NodeHandle, @@ -252,20 +238,6 @@ where } } -impl<'g, Root, Base> DescendantsGraph<'g, Root, Base> -where - Base: HugrInternals, -{ - /// Helper function to initialize the internal portgraph view. - fn init_graph(hugr: &Base, root: Node) -> RegionGraph<'_> { - RegionGraph::new_region( - &hugr.base_hugr().graph, - &hugr.base_hugr().hierarchy, - root.index, - ) - } -} - impl<'g, Root, Base> HugrView for DescendantsGraph<'g, Root, Base> where Root: NodeHandle, @@ -431,7 +403,11 @@ where } Self { root, - graph: Self::init_graph(hugr, root), + graph: FlatRegionGraph::new_flat_region( + &hugr.base_hugr().graph, + &hugr.base_hugr().hierarchy, + root.index, + ), hugr, _phantom: std::marker::PhantomData, } @@ -453,7 +429,11 @@ where } Self { root, - graph: Self::init_graph(hugr, root), + graph: RegionGraph::new_region( + &hugr.base_hugr().graph, + &hugr.base_hugr().hierarchy, + root.index, + ), hugr, _phantom: std::marker::PhantomData, } From 87e8d89b3e850a05a08fd3ecdd272e0bf85c9cfa Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Thu, 24 Aug 2023 02:03:51 +0100 Subject: [PATCH 06/10] Disallow modifying the root in some hugrmut methods --- src/hugr/hugrmut.rs | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 4cd96d570..0649baa27 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -56,7 +56,7 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// - If the attachment would introduce a cycle. #[inline] fn add_op_before(&mut self, sibling: Node, op: impl Into) -> Result { - self.valid_node(sibling)?; + self.valid_descendant(sibling)?; self.hugr_mut().add_op_before(sibling, op) } @@ -70,7 +70,7 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// - If the attachment would introduce a cycle. #[inline] fn add_op_after(&mut self, sibling: Node, op: impl Into) -> Result { - self.valid_node(sibling)?; + self.valid_descendant(sibling)?; self.hugr_mut().add_op_after(sibling, op) } @@ -81,7 +81,7 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// Panics if the node is the root node. #[inline] fn remove_node(&mut self, node: Node) -> Result<(), HugrError> { - self.valid_node(node)?; + self.valid_descendant(node)?; self.hugr_mut().remove_node(node) } @@ -323,6 +323,19 @@ pub(crate) mod sealed { } } + /// Validates that a node is a valid root descendant in the graph. + /// + /// To include the root node use [`HugrMutInternals::valid_node`] instead. + /// + /// Returns a [`HugrError::InvalidNode`] otherwise. + #[inline] + fn valid_descendant(&self, node: Node) -> Result<(), HugrError> { + match self.root() == node { + true => Err(HugrError::InvalidNode { node }), + false => self.valid_node(node), + } + } + /// Add a node to the graph, with the default conversion from OpType to NodeType fn add_op(&mut self, op: impl Into) -> Node { // TODO: Default to `NodeType::open_extensions` once we can infer extensions @@ -359,7 +372,8 @@ pub(crate) mod sealed { /// /// The node becomes the parent's last child. fn set_parent(&mut self, node: Node, parent: Node) -> Result<(), HugrError> { - self.valid_node(node)?; + self.valid_node(parent)?; + self.valid_descendant(node)?; self.hugr_mut().set_parent(node, parent) } @@ -370,8 +384,8 @@ pub(crate) mod sealed { /// /// The node becomes the parent's last child. fn move_after_sibling(&mut self, node: Node, after: Node) -> Result<(), HugrError> { - self.valid_node(node)?; - self.valid_node(after)?; + self.valid_descendant(node)?; + self.valid_descendant(after)?; self.hugr_mut().move_after_sibling(node, after) } @@ -381,8 +395,8 @@ pub(crate) mod sealed { /// /// The node becomes the parent's last child. fn move_before_sibling(&mut self, node: Node, before: Node) -> Result<(), HugrError> { - self.valid_node(node)?; - self.valid_node(before)?; + self.valid_descendant(node)?; + self.valid_descendant(before)?; self.hugr_mut().move_before_sibling(node, before) } From cb242e3c54a6e96f418bd904db4a69a642208f91 Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Tue, 29 Aug 2023 10:13:53 +0100 Subject: [PATCH 07/10] Move `add_op` and `add_node` to Hugr --- src/hugr.rs | 15 +++++++++++++++ src/hugr/hugrmut.rs | 10 ++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 8ec19b5a4..5bdaf2d1d 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -238,6 +238,21 @@ impl Hugr { } } + /// Add a node to the graph, with the default conversion from OpType to NodeType + pub(crate) fn add_op(&mut self, op: impl Into) -> Node { + // TODO: Default to `NodeType::open_extensions` once we can infer extensions + self.add_node(NodeType::pure(op)) + } + + /// Add a node to the graph. + pub(crate) fn add_node(&mut self, nodetype: NodeType) -> Node { + let node = self + .graph + .add_node(nodetype.input_count(), nodetype.output_count()); + self.op_types[node] = nodetype; + node.into() + } + /// Produce a canonical ordering of the descendant nodes of a root, /// following the graph hierarchy. /// diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 0649baa27..bdb8b898b 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -338,18 +338,12 @@ pub(crate) mod sealed { /// Add a node to the graph, with the default conversion from OpType to NodeType fn add_op(&mut self, op: impl Into) -> Node { - // TODO: Default to `NodeType::open_extensions` once we can infer extensions - self.add_node(NodeType::pure(op)) + self.hugr_mut().add_op(op) } /// Add a node to the graph. fn add_node(&mut self, nodetype: NodeType) -> Node { - let node = self - .hugr_mut() - .graph - .add_node(nodetype.input_count(), nodetype.output_count()); - self.hugr_mut().op_types[node] = nodetype; - node.into() + self.hugr_mut().add_node(nodetype) } /// Set the number of ports on a node. This may invalidate the node's `PortIndex`. From 2419c752b3da4c6e2649e2b0b4b521e01c9abea9 Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Tue, 29 Aug 2023 10:16:01 +0100 Subject: [PATCH 08/10] Fix merged imports --- src/extension/infer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension/infer.rs b/src/extension/infer.rs index 9024c66c9..bae062066 100644 --- a/src/extension/infer.rs +++ b/src/extension/infer.rs @@ -626,7 +626,7 @@ mod test { use super::*; use crate::builder::{BuildError, DFGBuilder, Dataflow, DataflowHugr}; use crate::extension::ExtensionSet; - use crate::hugr::HugrInternalsMut; + use crate::hugr::HugrMut; use crate::hugr::{validate::ValidationError, Hugr, HugrView, NodeType}; use crate::ops::{self, dataflow::IOTrait, handle::NodeHandle}; use crate::type_row; From 3ba5511ee07ae89c9c8ac750e569f23b8a54bb7d Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Tue, 29 Aug 2023 10:16:57 +0100 Subject: [PATCH 09/10] s/valid_descendant/valid_non_root/ --- src/hugr/hugrmut.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index bdb8b898b..29f08618f 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -56,7 +56,7 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// - If the attachment would introduce a cycle. #[inline] fn add_op_before(&mut self, sibling: Node, op: impl Into) -> Result { - self.valid_descendant(sibling)?; + self.valid_non_root(sibling)?; self.hugr_mut().add_op_before(sibling, op) } @@ -70,7 +70,7 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// - If the attachment would introduce a cycle. #[inline] fn add_op_after(&mut self, sibling: Node, op: impl Into) -> Result { - self.valid_descendant(sibling)?; + self.valid_non_root(sibling)?; self.hugr_mut().add_op_after(sibling, op) } @@ -81,7 +81,7 @@ pub trait HugrMut: HugrView + HugrMutInternals { /// Panics if the node is the root node. #[inline] fn remove_node(&mut self, node: Node) -> Result<(), HugrError> { - self.valid_descendant(node)?; + self.valid_non_root(node)?; self.hugr_mut().remove_node(node) } @@ -329,7 +329,7 @@ pub(crate) mod sealed { /// /// Returns a [`HugrError::InvalidNode`] otherwise. #[inline] - fn valid_descendant(&self, node: Node) -> Result<(), HugrError> { + fn valid_non_root(&self, node: Node) -> Result<(), HugrError> { match self.root() == node { true => Err(HugrError::InvalidNode { node }), false => self.valid_node(node), @@ -367,7 +367,7 @@ pub(crate) mod sealed { /// The node becomes the parent's last child. fn set_parent(&mut self, node: Node, parent: Node) -> Result<(), HugrError> { self.valid_node(parent)?; - self.valid_descendant(node)?; + self.valid_non_root(node)?; self.hugr_mut().set_parent(node, parent) } @@ -378,8 +378,8 @@ pub(crate) mod sealed { /// /// The node becomes the parent's last child. fn move_after_sibling(&mut self, node: Node, after: Node) -> Result<(), HugrError> { - self.valid_descendant(node)?; - self.valid_descendant(after)?; + self.valid_non_root(node)?; + self.valid_non_root(after)?; self.hugr_mut().move_after_sibling(node, after) } @@ -389,8 +389,8 @@ pub(crate) mod sealed { /// /// The node becomes the parent's last child. fn move_before_sibling(&mut self, node: Node, before: Node) -> Result<(), HugrError> { - self.valid_descendant(node)?; - self.valid_descendant(before)?; + self.valid_non_root(node)?; + self.valid_non_root(before)?; self.hugr_mut().move_before_sibling(node, before) } From bc2beb6068a0bf3253f41e03fe89e01acdfa39de Mon Sep 17 00:00:00 2001 From: Agustin Borgna Date: Tue, 29 Aug 2023 10:18:17 +0100 Subject: [PATCH 10/10] Convert hugr node error to tuple struct --- src/hugr.rs | 7 ++----- src/hugr/hugrmut.rs | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 5bdaf2d1d..87698e02f 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -414,11 +414,8 @@ pub enum HugrError { #[error("An error occurred while manipulating the hierarchy.")] HierarchyError(#[from] portgraph::hierarchy::AttachError), /// The node doesn't exist. - #[error("Invalid node {node:?}.")] - InvalidNode { - /// The missing node - node: Node, - }, + #[error("Invalid node {0:?}.")] + InvalidNode(Node), } #[cfg(feature = "pyo3")] diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 29f08618f..d0250f7aa 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -319,7 +319,7 @@ pub(crate) mod sealed { fn valid_node(&self, node: Node) -> Result<(), HugrError> { match self.contains_node(node) { true => Ok(()), - false => Err(HugrError::InvalidNode { node }), + false => Err(HugrError::InvalidNode(node)), } } @@ -331,7 +331,7 @@ pub(crate) mod sealed { #[inline] fn valid_non_root(&self, node: Node) -> Result<(), HugrError> { match self.root() == node { - true => Err(HugrError::InvalidNode { node }), + true => Err(HugrError::InvalidNode(node)), false => self.valid_node(node), } }