From 5b450ff058f4f503e2f3655d59aa017f1a561558 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Borgna?= <121866228+aborgna-q@users.noreply.github.com> Date: Fri, 8 Mar 2024 11:11:12 +0000 Subject: [PATCH] feat!: infallible `HugrMut` methods (#869) While all `HugrView` methods are infallible (panicking when a node is not found), most `HugrMut` methods returned a `Result`, even though they only failed on the same case. This PR makes most `HugrMut` methods infallible (except for `replace_op`). This simplify the API and lets us remove a lot of `unwrap`s all around the codebase. We can further cascade the `unwrap` removal to the builder (as `create_with_io` and others can now be made infallible), but I'll do that on another PR as this one is noisy enough. The first commit contains the relevant changes. The 2nd one just removes all the `unwrap`s. BREAKING CHANGE: Made most `HugrMut` methods infallible. --- quantinuum-hugr/src/builder.rs | 11 +- quantinuum-hugr/src/builder/build_traits.rs | 20 +- quantinuum-hugr/src/builder/cfg.rs | 7 +- quantinuum-hugr/src/builder/conditional.rs | 2 +- quantinuum-hugr/src/builder/dataflow.rs | 4 +- quantinuum-hugr/src/builder/module.rs | 10 +- quantinuum-hugr/src/extension/infer/test.rs | 165 ++++--- quantinuum-hugr/src/hugr.rs | 15 +- quantinuum-hugr/src/hugr/hugrmut.rs | 454 ++++++++++-------- quantinuum-hugr/src/hugr/rewrite.rs | 9 +- quantinuum-hugr/src/hugr/rewrite/consts.rs | 14 +- .../src/hugr/rewrite/inline_dfg.rs | 28 +- .../src/hugr/rewrite/insert_identity.rs | 14 +- .../src/hugr/rewrite/outline_cfg.rs | 20 +- quantinuum-hugr/src/hugr/rewrite/replace.rs | 93 ++-- .../src/hugr/rewrite/simple_replace.rs | 30 +- quantinuum-hugr/src/hugr/serialize.rs | 26 +- quantinuum-hugr/src/hugr/validate/test.rs | 265 +++++----- quantinuum-hugr/src/hugr/views.rs | 37 +- quantinuum-hugr/src/hugr/views/render.rs | 6 +- .../src/hugr/views/root_checked.rs | 2 +- quantinuum-hugr/src/hugr/views/sibling.rs | 15 +- .../src/hugr/views/sibling_subgraph.rs | 27 +- 23 files changed, 629 insertions(+), 645 deletions(-) diff --git a/quantinuum-hugr/src/builder.rs b/quantinuum-hugr/src/builder.rs index c0cecfc65..7b8cd06bb 100644 --- a/quantinuum-hugr/src/builder.rs +++ b/quantinuum-hugr/src/builder.rs @@ -89,7 +89,7 @@ use thiserror::Error; use crate::extension::SignatureError; -use crate::hugr::{HugrError, ValidationError}; +use crate::hugr::ValidationError; use crate::ops::handle::{BasicBlockID, CfgID, ConditionalID, DfgID, FuncID, TailLoopID}; use crate::types::ConstTypeError; use crate::types::Type; @@ -136,9 +136,6 @@ pub enum BuildError { /// [Const]: crate::ops::constant::Const #[error("Constant failed typechecking: {0}")] BadConstant(#[from] ConstTypeError), - /// HUGR construction error. - #[error("Error when mutating HUGR: {0}.")] - ConstructError(#[from] HugrError), /// CFG can only have one entry. #[error("CFG entry node already built for CFG node: {0:?}.")] EntryBuiltError(Node), @@ -235,15 +232,13 @@ pub(crate) mod test { ops::Input { types: signature.input, }, - ) - .unwrap(); + ); hugr.add_node_with_parent( hugr.root(), ops::Output { types: signature.output, }, - ) - .unwrap(); + ); hugr } } diff --git a/quantinuum-hugr/src/builder/build_traits.rs b/quantinuum-hugr/src/builder/build_traits.rs index 7b1b4621e..43902b748 100644 --- a/quantinuum-hugr/src/builder/build_traits.rs +++ b/quantinuum-hugr/src/builder/build_traits.rs @@ -47,12 +47,12 @@ pub trait Container { /// Add an [`OpType`] as the final child of the container. fn add_child_op(&mut self, op: impl Into) -> Result { let parent = self.container_node(); - Ok(self.hugr_mut().add_node_with_parent(parent, op)?) + Ok(self.hugr_mut().add_node_with_parent(parent, op)) } /// Add a [`NodeType`] as the final child of the container. fn add_child_node(&mut self, node: NodeType) -> Result { let parent = self.container_node(); - Ok(self.hugr_mut().add_node_with_parent(parent, node)?) + Ok(self.hugr_mut().add_node_with_parent(parent, node)) } /// Adds a non-dataflow edge between two nodes. The kind is given by the operation's [`other_inputs`] or [`other_outputs`] @@ -60,7 +60,7 @@ pub trait Container { /// [`other_inputs`]: crate::ops::OpTrait::other_input /// [`other_outputs`]: crate::ops::OpTrait::other_output fn add_other_wire(&mut self, src: Node, dst: Node) -> Result { - let (src_port, _) = self.hugr_mut().add_other_edge(src, dst)?; + let (src_port, _) = self.hugr_mut().add_other_edge(src, dst); Ok(Wire::new(src, src_port)) } @@ -102,20 +102,20 @@ pub trait Container { /// Insert a HUGR as a child of the container. fn add_hugr(&mut self, child: Hugr) -> Result { let parent = self.container_node(); - Ok(self.hugr_mut().insert_hugr(parent, child)?) + Ok(self.hugr_mut().insert_hugr(parent, child)) } /// Insert a copy of a HUGR as a child of the container. fn add_hugr_view(&mut self, child: &impl HugrView) -> Result { let parent = self.container_node(); - Ok(self.hugr_mut().insert_from_view(parent, child)?) + Ok(self.hugr_mut().insert_from_view(parent, child)) } /// Add metadata to the container node. fn set_metadata(&mut self, key: impl AsRef, meta: impl Into) { let parent = self.container_node(); // Implementor's container_node() should be a valid node - self.hugr_mut().set_metadata(parent, key, meta).unwrap(); + self.hugr_mut().set_metadata(parent, key, meta); } /// Add metadata to a child node. @@ -127,7 +127,7 @@ pub trait Container { key: impl AsRef, meta: impl Into, ) -> Result<(), BuildError> { - self.hugr_mut().set_metadata(child, key, meta)?; + self.hugr_mut().set_metadata(child, key, meta); Ok(()) } } @@ -604,7 +604,7 @@ pub trait Dataflow: Container { let src_port = self.hugr_mut().num_outputs(function.node()) - 1; self.hugr_mut() - .connect(function.node(), src_port, op_id.node(), const_in_port)?; + .connect(function.node(), src_port, op_id.node(), const_in_port); Ok(op_id) } @@ -695,7 +695,7 @@ fn wire_up( && !OpTag::BasicBlock.is_superset(base.get_optype(src_sibling).tag()) { // Add a state order constraint unless one of the nodes is a CFG BasicBlock - base.add_other_edge(src, src_sibling)?; + base.add_other_edge(src, src_sibling); } } else if !typ.copyable() & base.linked_ports(src, src_port).next().is_some() { // Don't copy linear edges. @@ -705,7 +705,7 @@ fn wire_up( data_builder .hugr_mut() - .connect(src, src_port, dst, dst_port)?; + .connect(src, src_port, dst, dst_port); Ok(local_source && matches!( data_builder diff --git a/quantinuum-hugr/src/builder/cfg.rs b/quantinuum-hugr/src/builder/cfg.rs index b9b7e2621..88ae2ffd9 100644 --- a/quantinuum-hugr/src/builder/cfg.rs +++ b/quantinuum-hugr/src/builder/cfg.rs @@ -190,7 +190,7 @@ impl + AsRef> CFGBuilder { let exit_node = base .as_mut() // Make the extensions a parameter - .add_node_with_parent(cfg_node, exit_block_type)?; + .add_node_with_parent(cfg_node, exit_block_type); Ok(Self { base, cfg_node, @@ -246,7 +246,7 @@ impl + AsRef> CFGBuilder { } else { // TODO: Make extensions a parameter self.hugr_mut().add_node_with_parent(parent, op) - }?; + }; BlockBuilder::create(self.hugr_mut(), block_n) } @@ -323,7 +323,8 @@ impl + AsRef> CFGBuilder { ) -> Result<(), BuildError> { let from = predecessor.node(); let to = successor.node(); - Ok(self.hugr_mut().connect(from, branch, to, 0)?) + self.hugr_mut().connect(from, branch, to, 0); + Ok(()) } } diff --git a/quantinuum-hugr/src/builder/conditional.rs b/quantinuum-hugr/src/builder/conditional.rs index 426f80512..8af475127 100644 --- a/quantinuum-hugr/src/builder/conditional.rs +++ b/quantinuum-hugr/src/builder/conditional.rs @@ -127,7 +127,7 @@ impl + AsRef> ConditionalBuilder { let case_node = // add case before any existing subsequent cases if let Some(&sibling_node) = self.case_nodes[case + 1..].iter().flatten().next() { - self.hugr_mut().add_node_before(sibling_node, case_op)? + self.hugr_mut().add_node_before(sibling_node, case_op) } else { self.add_child_op(case_op)? }; diff --git a/quantinuum-hugr/src/builder/dataflow.rs b/quantinuum-hugr/src/builder/dataflow.rs index 2aa471882..d11c394fe 100644 --- a/quantinuum-hugr/src/builder/dataflow.rs +++ b/quantinuum-hugr/src/builder/dataflow.rs @@ -50,14 +50,14 @@ impl + AsRef> DFGBuilder { types: signature.output().clone(), }; base.as_mut() - .add_node_with_parent(parent, NodeType::new(input, input_extensions.clone()))?; + .add_node_with_parent(parent, NodeType::new(input, input_extensions.clone())); base.as_mut().add_node_with_parent( parent, NodeType::new( output, input_extensions.map(|inp| inp.union(signature.extension_reqs)), ), - )?; + ); Ok(Self { base, diff --git a/quantinuum-hugr/src/builder/module.rs b/quantinuum-hugr/src/builder/module.rs index 174b8028f..93599618f 100644 --- a/quantinuum-hugr/src/builder/module.rs +++ b/quantinuum-hugr/src/builder/module.rs @@ -85,10 +85,12 @@ impl + AsRef> ModuleBuilder { })? .clone(); let body = signature.body().clone(); - self.hugr_mut().replace_op( - f_node, - NodeType::new_pure(ops::FuncDefn { name, signature }), - )?; + self.hugr_mut() + .replace_op( + f_node, + NodeType::new_pure(ops::FuncDefn { name, signature }), + ) + .expect("Replacing a FuncDecl node with a FuncDefn should always be valid"); let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body, None)?; Ok(FunctionBuilder::from_dfg_builder(db)) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index f070abcf1..86fa509c4 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -52,8 +52,8 @@ fn from_graph() -> Result<(), Box> { let input = ops::Input::new(type_row![NAT, NAT]); let output = ops::Output::new(type_row![NAT]); - let input = hugr.add_node_with_parent(hugr.root(), input)?; - let output = hugr.add_node_with_parent(hugr.root(), output)?; + let input = hugr.add_node_with_parent(hugr.root(), input); + let output = hugr.add_node_with_parent(hugr.root(), output); assert_matches!(hugr.get_io(hugr.root()), Some(_)); @@ -71,34 +71,34 @@ fn from_graph() -> Result<(), Box> { ops::DFG { signature: add_a_sig, }, - )?; + ); let add_b = hugr.add_node_with_parent( hugr.root(), ops::DFG { signature: add_b_sig, }, - )?; + ); let add_ab = hugr.add_node_with_parent( hugr.root(), ops::DFG { signature: add_ab_sig, }, - )?; + ); let mult_c = hugr.add_node_with_parent( hugr.root(), ops::DFG { signature: mult_c_sig, }, - )?; + ); - hugr.connect(input, 0, add_a, 0)?; - hugr.connect(add_a, 0, add_b, 0)?; - hugr.connect(add_b, 0, mult_c, 0)?; + hugr.connect(input, 0, add_a, 0); + hugr.connect(add_a, 0, add_b, 0); + hugr.connect(add_b, 0, mult_c, 0); - hugr.connect(input, 1, add_ab, 0)?; - hugr.connect(add_ab, 0, mult_c, 1)?; + hugr.connect(input, 1, add_ab, 0); + hugr.connect(add_ab, 0, mult_c, 1); - hugr.connect(mult_c, 0, output, 0)?; + hugr.connect(mult_c, 0, output, 0); let (_, closure) = infer_extensions(&hugr)?; let empty = ExtensionSet::new(); @@ -153,7 +153,7 @@ fn plus() -> Result<(), InferExtensionError> { #[test] // This generates a solution that causes validation to fail // because of a missing lift node -fn missing_lift_node() -> Result<(), Box> { +fn missing_lift_node() { let mut hugr = Hugr::new(NodeType::new_pure(ops::DFG { signature: FunctionType::new(type_row![NAT], type_row![NAT]).with_extension_delta(A), })); @@ -163,16 +163,16 @@ fn missing_lift_node() -> Result<(), Box> { NodeType::new_pure(ops::Input { types: type_row![NAT], }), - )?; + ); let output = hugr.add_node_with_parent( hugr.root(), NodeType::new_pure(ops::Output { types: type_row![NAT], }), - )?; + ); - hugr.connect(input, 0, output, 0)?; + hugr.connect(input, 0, output, 0); // Fail to catch the actual error because it's a difference between I/O // nodes and their parents and `report_mismatch` isn't yet smart enough @@ -181,7 +181,6 @@ fn missing_lift_node() -> Result<(), Box> { hugr.update_validate(&PRELUDE_REGISTRY), Err(ValidationError::CantInfer(_)) ); - Ok(()) } #[test] @@ -229,12 +228,12 @@ fn dangling_src() -> Result<(), Box> { ops::DFG { signature: add_r_sig, }, - )?; + ); // Dangling thingy let src_sig = FunctionType::new(type_row![], type_row![NAT]); - let src = hugr.add_node_with_parent(hugr.root(), ops::DFG { signature: src_sig })?; + let src = hugr.add_node_with_parent(hugr.root(), ops::DFG { signature: src_sig }); let mult_sig = FunctionType::new(type_row![NAT, NAT], type_row![NAT]); // Mult has open extension requirements, which we should solve to be "R" @@ -243,12 +242,12 @@ fn dangling_src() -> Result<(), Box> { ops::DFG { signature: mult_sig, }, - )?; + ); - hugr.connect(input, 0, add_r, 0)?; - hugr.connect(add_r, 0, mult, 0)?; - hugr.connect(src, 0, mult, 1)?; - hugr.connect(mult, 0, output, 0)?; + hugr.connect(input, 0, add_r, 0); + hugr.connect(add_r, 0, mult, 0); + hugr.connect(src, 0, mult, 1); + hugr.connect(mult, 0, output, 0); let closure = hugr.infer_extensions()?; assert!(closure.is_empty()); @@ -286,19 +285,19 @@ fn create_with_io( ) -> Result<[Node; 3], Box> { let op: OpType = op.into(); - let node = hugr.add_node_with_parent(parent, op)?; + let node = hugr.add_node_with_parent(parent, op); let input = hugr.add_node_with_parent( node, ops::Input { types: op_sig.input, }, - )?; + ); let output = hugr.add_node_with_parent( node, ops::Output { types: op_sig.output, }, - )?; + ); Ok([node, input, output]) } @@ -321,7 +320,7 @@ fn test_conditional_inference() -> Result<(), Box> { type_row: type_row![NAT], new_extension: first_ext, }, - )?; + ); let lift2 = hugr.add_node_with_parent( case, @@ -329,11 +328,11 @@ fn test_conditional_inference() -> Result<(), Box> { type_row: type_row![NAT], new_extension: second_ext, }, - )?; + ); - hugr.connect(case_in, 0, lift1, 0)?; - hugr.connect(lift1, 0, lift2, 0)?; - hugr.connect(lift2, 0, case_out, 0)?; + hugr.connect(case_in, 0, lift1, 0); + hugr.connect(lift1, 0, lift2, 0); + hugr.connect(lift2, 0, case_out, 0); Ok(case) } @@ -392,13 +391,13 @@ fn extension_adding_sequence() -> Result<(), Box> { ops::Input { types: type_row![NAT], }, - )?; + ); let output = hugr.add_node_with_parent( root, ops::Output { types: type_row![NAT], }, - )?; + ); // Make identical dataflow nodes which add extension requirement "A" or "B" let df_nodes: Vec = vec![A, A, B, B, A, B] @@ -415,18 +414,16 @@ fn extension_adding_sequence() -> Result<(), Box> { ) .unwrap(); - let lift = hugr - .add_node_with_parent( - node, - ops::LeafOp::Lift { - type_row: type_row![NAT], - new_extension: ext, - }, - ) - .unwrap(); + let lift = hugr.add_node_with_parent( + node, + ops::LeafOp::Lift { + type_row: type_row![NAT], + new_extension: ext, + }, + ); - hugr.connect(input, 0, lift, 0).unwrap(); - hugr.connect(lift, 0, output, 0).unwrap(); + hugr.connect(input, 0, lift, 0); + hugr.connect(lift, 0, output, 0); node }) @@ -435,7 +432,7 @@ fn extension_adding_sequence() -> Result<(), Box> { // Connect nodes in order (0 -> 1 -> 2 ...) let nodes = [vec![input], df_nodes, vec![output]].concat(); for (src, tgt) in nodes.into_iter().tuple_windows() { - hugr.connect(src, 0, tgt, 0)?; + hugr.connect(src, 0, tgt, 0); } hugr.update_validate(&PRELUDE_REGISTRY)?; Ok(()) @@ -467,10 +464,10 @@ fn make_block( let [bb, bb_in, bb_out] = create_with_io(hugr, bb_parent, dfb, dfb_sig)?; - let dfg = hugr.add_node_with_parent(bb, op)?; + let dfg = hugr.add_node_with_parent(bb, op); - hugr.connect(bb_in, 0, dfg, 0)?; - hugr.connect(dfg, 0, bb_out, 0)?; + hugr.connect(bb_in, 0, dfg, 0); + hugr.connect(dfg, 0, bb_out, 0); Ok(bb) } @@ -504,16 +501,16 @@ fn create_entry_exit( ops::ExitBlock { cfg_outputs: exit_types.into(), }, - )?; + ); - let entry = hugr.add_node_before(exit, dfb)?; - let entry_in = hugr.add_node_with_parent(entry, ops::Input { types: inputs })?; + let entry = hugr.add_node_before(exit, dfb); + let entry_in = hugr.add_node_with_parent(entry, ops::Input { types: inputs }); let entry_out = hugr.add_node_with_parent( entry, ops::Output { types: vec![entry_tuple_sum].into(), }, - )?; + ); Ok(([entry, entry_in, entry_out], exit)) } @@ -566,11 +563,11 @@ fn infer_cfg_test() -> Result<(), Box> { A, FunctionType::new(vec![NAT], twoway(NAT)).with_extension_delta(A), ), - )?; + ); // Internal wiring for DFGs - hugr.connect(entry_in, 0, mkpred, 0)?; - hugr.connect(mkpred, 0, entry_out, 0)?; + hugr.connect(entry_in, 0, mkpred, 0); + hugr.connect(mkpred, 0, entry_out, 0); let bb0 = make_block( &mut hugr, @@ -605,14 +602,14 @@ fn infer_cfg_test() -> Result<(), Box> { )?; // CFG Wiring - hugr.connect(entry, 0, bb0, 0)?; - hugr.connect(entry, 0, bb1, 0)?; - hugr.connect(bb1, 0, bb10, 0)?; - hugr.connect(bb1, 0, bb11, 0)?; + hugr.connect(entry, 0, bb0, 0); + hugr.connect(entry, 0, bb1, 0); + hugr.connect(bb1, 0, bb10, 0); + hugr.connect(bb1, 0, bb11, 0); - hugr.connect(bb0, 0, exit, 0)?; - hugr.connect(bb10, 0, exit, 0)?; - hugr.connect(bb11, 0, exit, 0)?; + hugr.connect(bb0, 0, exit, 0); + hugr.connect(bb10, 0, exit, 0); + hugr.connect(bb11, 0, exit, 0); hugr.infer_extensions()?; @@ -659,10 +656,10 @@ fn multi_entry() -> Result<(), Box> { let entry_mid = hugr.add_node_with_parent( entry, make_opaque(UNKNOWN_EXTENSION, FunctionType::new(vec![NAT], twoway(NAT))), - )?; + ); - hugr.connect(entry_in, 0, entry_mid, 0)?; - hugr.connect(entry_mid, 0, entry_out, 0)?; + hugr.connect(entry_in, 0, entry_mid, 0); + hugr.connect(entry_mid, 0, entry_out, 0); let bb0 = make_block( &mut hugr, @@ -688,11 +685,11 @@ fn multi_entry() -> Result<(), Box> { ExtensionSet::new(), )?; - hugr.connect(entry, 0, bb0, 0)?; - hugr.connect(entry, 0, bb1, 0)?; - hugr.connect(bb0, 0, bb2, 0)?; - hugr.connect(bb1, 0, bb2, 0)?; - hugr.connect(bb2, 0, exit, 0)?; + hugr.connect(entry, 0, bb0, 0); + hugr.connect(entry, 0, bb1, 0); + hugr.connect(bb0, 0, bb2, 0); + hugr.connect(bb1, 0, bb2, 0); + hugr.connect(bb2, 0, exit, 0); hugr.update_validate(&PRELUDE_REGISTRY)?; @@ -751,10 +748,10 @@ fn make_looping_cfg( UNKNOWN_EXTENSION, FunctionType::new(vec![NAT], oneway(NAT)).with_extension_delta(entry_ext), ), - )?; + ); - hugr.connect(entry_in, 0, entry_dfg, 0)?; - hugr.connect(entry_dfg, 0, entry_out, 0)?; + hugr.connect(entry_in, 0, entry_dfg, 0); + hugr.connect(entry_dfg, 0, entry_out, 0); let bb1 = make_block( &mut hugr, @@ -772,10 +769,10 @@ fn make_looping_cfg( bb2_ext.clone(), )?; - hugr.connect(entry, 0, bb1, 0)?; - hugr.connect(bb1, 0, bb2, 0)?; - hugr.connect(bb1, 0, exit, 0)?; - hugr.connect(bb2, 0, entry, 0)?; + hugr.connect(entry, 0, bb1, 0); + hugr.connect(bb1, 0, bb2, 0); + hugr.connect(bb1, 0, exit, 0); + hugr.connect(bb2, 0, entry, 0); Ok(hugr) } @@ -825,10 +822,10 @@ fn simple_cfg_loop() -> Result<(), Box> { let entry_mid = hugr.add_node_with_parent( entry, make_opaque(UNKNOWN_EXTENSION, FunctionType::new(vec![NAT], oneway(NAT))), - )?; + ); - hugr.connect(entry_in, 0, entry_mid, 0)?; - hugr.connect(entry_mid, 0, entry_out, 0)?; + hugr.connect(entry_in, 0, entry_mid, 0); + hugr.connect(entry_mid, 0, entry_out, 0); let bb = make_block( &mut hugr, @@ -838,9 +835,9 @@ fn simple_cfg_loop() -> Result<(), Box> { just_a.clone(), )?; - hugr.connect(entry, 0, bb, 0)?; - hugr.connect(bb, 0, bb, 0)?; - hugr.connect(bb, 0, exit, 0)?; + hugr.connect(entry, 0, bb, 0); + hugr.connect(bb, 0, bb, 0); + hugr.connect(bb, 0, exit, 0); hugr.update_validate(&PRELUDE_REGISTRY)?; diff --git a/quantinuum-hugr/src/hugr.rs b/quantinuum-hugr/src/hugr.rs index 514c8dcbc..c2018c883 100644 --- a/quantinuum-hugr/src/hugr.rs +++ b/quantinuum-hugr/src/hugr.rs @@ -332,15 +332,6 @@ impl Hugr { #[derive(Debug, Clone, PartialEq, Eq, Error)] #[non_exhaustive] pub enum HugrError { - /// An error occurred while connecting nodes. - #[error("An error occurred while connecting the nodes.")] - ConnectionError(#[from] portgraph::LinkError), - /// 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 {0:?}.")] - InvalidNode(Node), /// The node was not of the required [OpTag] /// (e.g. to conform to the [RootTagged::RootHandle] of a [HugrView]) #[error("Invalid tag: required a tag in {required} but found {actual}")] @@ -397,9 +388,9 @@ mod test { type_row: type_row![BIT], new_extension: "R".try_into().unwrap(), }, - )?; - hugr.connect(input, 0, lift, 0)?; - hugr.connect(lift, 0, output, 0)?; + ); + hugr.connect(input, 0, lift, 0); + hugr.connect(lift, 0, output, 0); hugr.infer_extensions()?; assert_eq!( diff --git a/quantinuum-hugr/src/hugr/hugrmut.rs b/quantinuum-hugr/src/hugr/hugrmut.rs index 597c8f1f6..a0be5aade 100644 --- a/quantinuum-hugr/src/hugr/hugrmut.rs +++ b/quantinuum-hugr/src/hugr/hugrmut.rs @@ -1,5 +1,6 @@ //! Low-level interface for modifying a HUGR. +use core::panic; use std::collections::HashMap; use std::ops::Range; @@ -18,74 +19,78 @@ use super::NodeMetadataMap; /// Functions for low-level building of a HUGR. pub trait HugrMut: HugrMutInternals { /// Returns a metadata entry associated with a node. - fn get_metadata_mut( - &mut self, - node: Node, - key: impl AsRef, - ) -> Result<&mut NodeMetadata, HugrError> { - self.valid_node(node)?; + /// + /// # Panics + /// + /// If the node is not in the graph. + fn get_metadata_mut(&mut self, node: Node, key: impl AsRef) -> &mut NodeMetadata { + panic_invalid_node(self, node); let node_meta = self .hugr_mut() .metadata .get_mut(node.pg_index()) .get_or_insert_with(Default::default); - Ok(node_meta + node_meta .entry(key.as_ref()) - .or_insert(serde_json::Value::Null)) + .or_insert(serde_json::Value::Null) } /// Sets a metadata value associated with a node. + /// + /// # Panics + /// + /// If the node is not in the graph. fn set_metadata( &mut self, node: Node, key: impl AsRef, metadata: impl Into, - ) -> Result<(), HugrError> { - let entry = self.get_metadata_mut(node, key)?; + ) { + let entry = self.get_metadata_mut(node, key); *entry = metadata.into(); - Ok(()) } /// Retrieve the complete metadata map for a node. fn take_node_metadata(&mut self, node: Node) -> Option { - self.valid_node(node).ok()?; + if !self.valid_node(node) { + return None; + } self.hugr_mut().metadata.take(node.pg_index()) } /// Overwrite the complete metadata map for a node. - fn overwrite_node_metadata( - &mut self, - node: Node, - metadata: Option, - ) -> Result<(), HugrError> { - self.valid_node(node)?; + /// + /// # Panics + /// + /// If the node is not in the graph. + fn overwrite_node_metadata(&mut self, node: Node, metadata: Option) { + panic_invalid_node(self, node); self.hugr_mut().metadata.set(node.pg_index(), metadata); - Ok(()) } /// Add a node to the graph with a parent in the hierarchy. /// /// The node becomes the parent's last child. + /// + /// # Panics + /// + /// If the parent is not in the graph. #[inline] - fn add_node_with_parent( - &mut self, - parent: Node, - op: impl Into, - ) -> Result { - self.valid_node(parent)?; + fn add_node_with_parent(&mut self, parent: Node, op: impl Into) -> Node { + panic_invalid_node(self, parent); self.hugr_mut().add_node_with_parent(parent, op) } /// Add a node to the graph as the previous sibling of another node. /// /// The sibling node's parent becomes the new node's parent. + /// + /// # Panics + /// + /// If the sibling is not in the graph, or if the sibling is the root node. #[inline] - fn add_node_before( - &mut self, - sibling: Node, - nodetype: impl Into, - ) -> Result { - self.valid_non_root(sibling)?; + fn add_node_before(&mut self, sibling: Node, nodetype: impl Into) -> Node { + panic_invalid_non_root(self, sibling); self.hugr_mut().add_node_before(sibling, nodetype) } @@ -93,17 +98,12 @@ pub trait HugrMut: HugrMutInternals { /// /// The sibling node's parent becomes the new node's parent. /// - /// # Errors + /// # Panics /// - /// - If the sibling node does not have a parent. - /// - If the attachment would introduce a cycle. + /// If the sibling is not in the graph, or if the sibling is the root node. #[inline] - fn add_node_after( - &mut self, - sibling: Node, - op: impl Into, - ) -> Result { - self.valid_non_root(sibling)?; + fn add_node_after(&mut self, sibling: Node, op: impl Into) -> Node { + panic_invalid_non_root(self, sibling); self.hugr_mut().add_node_after(sibling, op) } @@ -111,19 +111,18 @@ pub trait HugrMut: HugrMutInternals { /// /// # Panics /// - /// Panics if the node is the root node. + /// If the node is not in the graph, or if the node is the root node. #[inline] - fn remove_node(&mut self, node: Node) -> Result<(), HugrError> { - self.valid_non_root(node)?; + fn remove_node(&mut self, node: Node) { + panic_invalid_non_root(self, node); self.hugr_mut().remove_node(node) } /// Connect two nodes at the given ports. /// - /// The port must have already been created. See [`add_ports`] and [`set_num_ports`]. + /// # Panics /// - /// [`add_ports`]: #method.add_ports - /// [`set_num_ports`]: #method.set_num_ports. + /// If either node is not in the graph or if the ports are invalid. #[inline] fn connect( &mut self, @@ -131,54 +130,61 @@ pub trait HugrMut: HugrMutInternals { src_port: impl Into, dst: Node, dst_port: impl Into, - ) -> Result<(), HugrError> { - self.valid_node(src)?; - self.valid_node(dst)?; - self.hugr_mut().connect(src, src_port, dst, dst_port) + ) { + panic_invalid_node(self, src); + panic_invalid_node(self, dst); + self.hugr_mut().connect(src, src_port, dst, dst_port); } /// Disconnects all edges from the given port. /// /// The port is left in place. + /// + /// # Panics + /// + /// If the node is not in the graph, or if the port is invalid. #[inline] - fn disconnect(&mut self, node: Node, port: impl Into) -> Result<(), HugrError> { - self.valid_node(node)?; - self.hugr_mut().disconnect(node, port) + fn disconnect(&mut self, node: Node, port: impl Into) { + panic_invalid_node(self, node); + self.hugr_mut().disconnect(node, port); } /// Adds a non-dataflow edge between two nodes. The kind is given by the /// 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. + /// Returns the offsets of the new input and output ports. /// /// [`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<(OutgoingPort, IncomingPort), HugrError> { - self.valid_node(src)?; - self.valid_node(dst)?; + /// + /// # Panics + /// + /// If the node is not in the graph, or if the port is invalid. + fn add_other_edge(&mut self, src: Node, dst: Node) -> (OutgoingPort, IncomingPort) { + panic_invalid_node(self, src); + panic_invalid_node(self, dst); self.hugr_mut().add_other_edge(src, dst) } /// Insert another hugr into this one, under a given root node. + /// + /// # Panics + /// + /// If the root node is not in the graph. #[inline] - fn insert_hugr(&mut self, root: Node, other: Hugr) -> Result { - self.valid_node(root)?; + fn insert_hugr(&mut self, root: Node, other: Hugr) -> InsertionResult { + panic_invalid_node(self, root); self.hugr_mut().insert_hugr(root, other) } /// Copy another hugr into this one, under a given root node. + /// + /// # Panics + /// + /// If the root node is not in the graph. #[inline] - fn insert_from_view( - &mut self, - root: Node, - other: &impl HugrView, - ) -> Result { - self.valid_node(root)?; + fn insert_from_view(&mut self, root: Node, other: &impl HugrView) -> InsertionResult { + panic_invalid_node(self, root); self.hugr_mut().insert_from_view(root, other) } @@ -188,6 +194,10 @@ pub trait HugrMut: HugrMutInternals { /// /// The return value is a map from indices in `other` to the indices of the /// corresponding new nodes in `self`. + /// + /// # Panics + /// + /// If the root node is not in the graph. // // TODO: Try to preserve the order when possible? We cannot always ensure // it, since the subgraph may have arbitrary nodes without including their @@ -197,8 +207,8 @@ pub trait HugrMut: HugrMutInternals { root: Node, other: &impl HugrView, subgraph: &SiblingSubgraph, - ) -> Result, HugrError> { - self.valid_node(root)?; + ) -> HashMap { + panic_invalid_node(self, root); self.hugr_mut().insert_subgraph(root, other, subgraph) } @@ -229,51 +239,38 @@ fn translate_indices(node_map: HashMap) -> HashMap + AsMut> HugrMut for T { - fn add_node_with_parent( - &mut self, - parent: Node, - node: impl Into, - ) -> Result { + fn add_node_with_parent(&mut self, parent: Node, node: impl Into) -> Node { let node = self.as_mut().add_node(node.into()); self.as_mut() .hierarchy - .push_child(node.pg_index(), parent.pg_index())?; - Ok(node) + .push_child(node.pg_index(), parent.pg_index()) + .expect("Inserting a newly-created node into the hierarchy should never fail."); + node } - fn add_node_before( - &mut self, - sibling: Node, - nodetype: impl Into, - ) -> Result { + fn add_node_before(&mut self, sibling: Node, nodetype: impl Into) -> Node { let node = self.as_mut().add_node(nodetype.into()); self.as_mut() .hierarchy - .insert_before(node.pg_index(), sibling.pg_index())?; - Ok(node) + .insert_before(node.pg_index(), sibling.pg_index()) + .expect("Inserting a newly-created node into the hierarchy should never fail."); + node } - fn add_node_after( - &mut self, - sibling: Node, - op: impl Into, - ) -> Result { + fn add_node_after(&mut self, sibling: Node, op: impl Into) -> Node { let node = self.as_mut().add_node(op.into()); self.as_mut() .hierarchy - .insert_after(node.pg_index(), sibling.pg_index())?; - Ok(node) + .insert_after(node.pg_index(), sibling.pg_index()) + .expect("Inserting a newly-created node into the hierarchy should never fail."); + node } - fn remove_node(&mut self, node: Node) -> Result<(), HugrError> { - if node == self.root() { - // TODO: Add a HugrMutError ? - panic!("cannot remove root node"); - } + fn remove_node(&mut self, node: Node) { + panic_invalid_non_root(self, node); self.as_mut().hierarchy.remove(node.pg_index()); self.as_mut().graph.remove_node(node.pg_index()); self.as_mut().op_types.remove(node.pg_index()); - Ok(()) } fn connect( @@ -282,36 +279,35 @@ impl + AsMut> HugrMut for T { src_port: impl Into, dst: Node, dst_port: impl Into, - ) -> Result<(), HugrError> { - self.as_mut().graph.link_nodes( - src.pg_index(), - src_port.into().index(), - dst.pg_index(), - dst_port.into().index(), - )?; - Ok(()) + ) { + let src_port = src_port.into(); + let dst_port = dst_port.into(); + panic_invalid_port(self, src, src_port); + panic_invalid_port(self, dst, dst_port); + self.as_mut() + .graph + .link_nodes( + src.pg_index(), + src_port.index(), + dst.pg_index(), + dst_port.index(), + ) + .expect("The ports should exist at this point."); } - fn disconnect(&mut self, node: Node, port: impl Into) -> Result<(), HugrError> { + fn disconnect(&mut self, node: Node, port: impl Into) { let port = port.into(); let offset = port.pg_offset(); + panic_invalid_port(self, node, port); let port = self .as_mut() .graph .port_index(node.pg_index(), offset) - .ok_or(portgraph::LinkError::UnknownOffset { - node: node.pg_index(), - offset, - })?; + .expect("The port should exist at this point."); self.as_mut().graph.unlink_port(port); - Ok(()) } - fn add_other_edge( - &mut self, - src: Node, - dst: Node, - ) -> Result<(OutgoingPort, IncomingPort), HugrError> { + fn add_other_edge(&mut self, src: Node, dst: Node) -> (OutgoingPort, IncomingPort) { let src_port = self .get_optype(src) .other_output_port() @@ -320,12 +316,12 @@ impl + AsMut> HugrMut for T { .get_optype(dst) .other_input_port() .expect("Destination operation has no non-dataflow incoming edges"); - self.connect(src, src_port, dst, dst_port)?; - Ok((src_port, dst_port)) + self.connect(src, src_port, dst, dst_port); + (src_port, dst_port) } - fn insert_hugr(&mut self, root: Node, mut other: Hugr) -> Result { - let (new_root, node_map) = insert_hugr_internal(self.as_mut(), root, &other)?; + fn insert_hugr(&mut self, root: Node, mut other: Hugr) -> InsertionResult { + let (new_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); @@ -337,18 +333,14 @@ impl + AsMut> HugrMut for T { Some(&new_root.pg_index()), node_map.get(&other.root().pg_index()) ); - Ok(InsertionResult { + InsertionResult { new_root, node_map: translate_indices(node_map), - }) + } } - fn insert_from_view( - &mut self, - root: Node, - other: &impl HugrView, - ) -> Result { - let (new_root, node_map) = insert_hugr_internal(self.as_mut(), root, other)?; + fn insert_from_view(&mut self, root: Node, other: &impl HugrView) -> InsertionResult { + let (new_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()); @@ -360,10 +352,10 @@ impl + AsMut> HugrMut for T { Some(&new_root.pg_index()), node_map.get(&other.root().pg_index()) ); - Ok(InsertionResult { + InsertionResult { new_root, node_map: translate_indices(node_map), - }) + } } fn insert_subgraph( @@ -371,7 +363,7 @@ impl + AsMut> HugrMut for T { root: Node, other: &impl HugrView, subgraph: &SiblingSubgraph, - ) -> Result, HugrError> { + ) -> HashMap { // Create a portgraph view with the explicit list of nodes defined by the subgraph. let portgraph: NodeFiltered<_, NodeFilter<&[Node]>, &[Node]> = NodeFiltered::new_node_filtered( @@ -379,7 +371,7 @@ impl + AsMut> HugrMut for T { |node, ctx| ctx.contains(&node.into()), subgraph.nodes(), ); - let node_map = insert_subgraph_internal(self.as_mut(), root, other, &portgraph)?; + let node_map = insert_subgraph_internal(self.as_mut(), root, other, &portgraph); // 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()); @@ -387,7 +379,7 @@ impl + AsMut> HugrMut for T { let meta = other.base_hugr().metadata.get(node); self.as_mut().metadata.set(new_node, meta.clone()); } - Ok(translate_indices(node_map)) + translate_indices(node_map) } } @@ -403,20 +395,23 @@ 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())?; +) -> (Node, HashMap) { + let node_map = hugr + .graph + .insert_graph(&other.portgraph()) + .unwrap_or_else(|e| panic!("Internal error while inserting a hugr into another: {e}")); let other_root = node_map[&other.root().pg_index()]; // Update hierarchy and optypes - hugr.hierarchy.push_child(other_root, root.pg_index())?; + hugr.hierarchy + .push_child(other_root, root.pg_index()) + .expect("Inserting a newly-created node into the hierarchy should never fail."); 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.pg_index()], new_node)?; - Ok(()) - })?; + other.children(node.into()).for_each(|child| { + hugr.hierarchy + .push_child(node_map[&child.pg_index()], new_node) + .expect("Inserting a newly-created node into the hierarchy should never fail."); + }); } // The root node didn't have any ports. @@ -427,7 +422,7 @@ fn insert_hugr_internal( root_optype.output_count(), ); - Ok((other_root.into(), node_map)) + (other_root.into(), node_map) } /// Internal implementation of the `insert_subgraph` method for AsMut. @@ -447,8 +442,11 @@ fn insert_subgraph_internal( root: Node, other: &impl HugrView, portgraph: &impl portgraph::LinkView, -) -> Result, HugrError> { - let node_map = hugr.graph.insert_graph(&portgraph)?; +) -> HashMap { + let node_map = hugr + .graph + .insert_graph(&portgraph) + .expect("Internal error while inserting a subgraph into another"); // A map for nodes that we inserted before their parent, so we couldn't // update the hierarchy with their new id. @@ -457,10 +455,50 @@ fn insert_subgraph_internal( .get_parent(node.into()) .and_then(|parent| node_map.get(&parent.pg_index()).copied()) .unwrap_or(root.pg_index()); - hugr.hierarchy.push_child(new_node, new_parent)?; + hugr.hierarchy + .push_child(new_node, new_parent) + .expect("Inserting a newly-created node into the hierarchy should never fail."); + } + + node_map +} + +/// Panic if [`HugrView::valid_node`] fails. +#[track_caller] +fn panic_invalid_node(hugr: &H, node: Node) { + if !hugr.valid_node(node) { + panic!( + "Received an invalid node {node} while mutating a HUGR:\n\n {}", + hugr.mermaid_string() + ); + } +} + +/// Panic if [`HugrView::valid_non_root`] fails. +#[track_caller] +fn panic_invalid_non_root(hugr: &H, node: Node) { + if !hugr.valid_non_root(node) { + panic!( + "Received an invalid non-root node {node} while mutating a HUGR:\n\n {}", + hugr.mermaid_string() + ); } +} - Ok(node_map) +/// Panic if [`HugrView::valid_node`] fails. +#[track_caller] +fn panic_invalid_port(hugr: &H, node: Node, port: impl Into) { + let port = port.into(); + if hugr + .portgraph() + .port_index(node.pg_index(), port.pg_offset()) + .is_none() + { + panic!( + "Received an invalid port {port} for node {node} while mutating a HUGR:\n\n {}", + hugr.mermaid_string() + ); + } } pub(crate) mod sealed { @@ -476,8 +514,12 @@ pub(crate) mod sealed { fn hugr_mut(&mut self) -> &mut Hugr; /// Set the number of ports on a node. This may invalidate the node's `PortIndex`. + /// + /// # Panics + /// + /// If the node is not in the graph. fn set_num_ports(&mut self, node: Node, incoming: usize, outgoing: usize) { - self.valid_node(node).unwrap_or_else(|e| panic!("{}", e)); + panic_invalid_node(self, node); self.hugr_mut().set_num_ports(node, incoming, outgoing) } @@ -486,18 +528,26 @@ pub(crate) mod sealed { /// /// The `direction` parameter specifies whether to add ports to the incoming /// or outgoing list. + /// + /// # Panics + /// + /// If the node is not in the graph. fn add_ports(&mut self, node: Node, direction: Direction, amount: isize) -> Range { - self.valid_node(node).unwrap_or_else(|e| panic!("{}", e)); + panic_invalid_node(self, node); 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> { - self.valid_node(parent)?; - self.valid_non_root(node)?; - self.hugr_mut().set_parent(node, parent) + /// + /// # Panics + /// + /// If either the node or the parent is not in the graph. + fn set_parent(&mut self, node: Node, parent: Node) { + panic_invalid_node(self, parent); + panic_invalid_non_root(self, node); + self.hugr_mut().set_parent(node, parent); } /// Move a node in the hierarchy to be the subsequent sibling of another @@ -506,10 +556,14 @@ 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> { - self.valid_non_root(node)?; - self.valid_non_root(after)?; - self.hugr_mut().move_after_sibling(node, after) + /// + /// # Panics + /// + /// If either node is not in the graph, or if it is a root. + fn move_after_sibling(&mut self, node: Node, after: Node) { + panic_invalid_non_root(self, node); + panic_invalid_non_root(self, after); + self.hugr_mut().move_after_sibling(node, after); } /// Move a node in the hierarchy to be the prior sibling of another node. @@ -517,9 +571,13 @@ 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_before_sibling(&mut self, node: Node, before: Node) -> Result<(), HugrError> { - self.valid_non_root(node)?; - self.valid_non_root(before)?; + /// + /// # Panics + /// + /// If either node is not in the graph, or if it is a root. + fn move_before_sibling(&mut self, node: Node, before: Node) { + panic_invalid_non_root(self, node); + panic_invalid_non_root(self, before); self.hugr_mut().move_before_sibling(node, before) } @@ -529,10 +587,15 @@ pub(crate) mod sealed { /// TODO: Add a version which ignores input extensions /// /// # Errors + /// /// Returns a [`HugrError::InvalidTag`] if this would break the bound - /// ([`Self::RootHandle`]) on the root node's [OpTag] + /// ([`Self::RootHandle`]) on the root node's [OpTag]. + /// + /// # Panics + /// + /// If the node is not in the graph. fn replace_op(&mut self, node: Node, op: NodeType) -> Result { - self.valid_node(node)?; + panic_invalid_node(self, node); if node == self.root() && !Self::RootHandle::TAG.is_superset(op.tag()) { return Err(HugrError::InvalidTag { required: Self::RootHandle::TAG, @@ -575,28 +638,28 @@ pub(crate) mod sealed { range } - fn set_parent(&mut self, node: Node, parent: Node) -> Result<(), HugrError> { + fn set_parent(&mut self, node: Node, parent: Node) { self.hugr_mut().hierarchy.detach(node.pg_index()); self.hugr_mut() .hierarchy - .push_child(node.pg_index(), parent.pg_index())?; - Ok(()) + .push_child(node.pg_index(), parent.pg_index()) + .expect("Inserting a newly-created node into the hierarchy should never fail."); } - fn move_after_sibling(&mut self, node: Node, after: Node) -> Result<(), HugrError> { + fn move_after_sibling(&mut self, node: Node, after: Node) { self.hugr_mut().hierarchy.detach(node.pg_index()); self.hugr_mut() .hierarchy - .insert_after(node.pg_index(), after.pg_index())?; - Ok(()) + .insert_after(node.pg_index(), after.pg_index()) + .expect("Inserting a newly-created node into the hierarchy should never fail."); } - fn move_before_sibling(&mut self, node: Node, before: Node) -> Result<(), HugrError> { + fn move_before_sibling(&mut self, node: Node, before: Node) { self.hugr_mut().hierarchy.detach(node.pg_index()); self.hugr_mut() .hierarchy - .insert_before(node.pg_index(), before.pg_index())?; - Ok(()) + .insert_before(node.pg_index(), before.pg_index()) + .expect("Inserting a newly-created node into the hierarchy should never fail."); } fn replace_op(&mut self, node: Node, op: NodeType) -> Result { @@ -622,39 +685,34 @@ mod test { const NAT: Type = USIZE_T; #[test] - fn simple_function() { + fn simple_function() -> Result<(), Box> { let mut hugr = Hugr::default(); // Create the root module definition let module: Node = hugr.root(); // Start a main function with two nat inputs. - let f: Node = hugr - .add_node_with_parent( - module, - ops::FuncDefn { - name: "main".into(), - signature: FunctionType::new(type_row![NAT], type_row![NAT, NAT]).into(), - }, - ) - .expect("Failed to add function definition node"); + let f: Node = hugr.add_node_with_parent( + module, + ops::FuncDefn { + name: "main".into(), + signature: FunctionType::new(type_row![NAT], type_row![NAT, NAT]).into(), + }, + ); { - let f_in = hugr - .add_node_with_parent(f, NodeType::new_pure(ops::Input::new(type_row![NAT]))) - .unwrap(); - let f_out = hugr - .add_node_with_parent(f, ops::Output::new(type_row![NAT, NAT])) - .unwrap(); - let noop = hugr - .add_node_with_parent(f, LeafOp::Noop { ty: NAT }) - .unwrap(); - - hugr.connect(f_in, 0, noop, 0).unwrap(); - hugr.connect(noop, 0, f_out, 0).unwrap(); - hugr.connect(noop, 0, f_out, 1).unwrap(); + let f_in = + hugr.add_node_with_parent(f, NodeType::new_pure(ops::Input::new(type_row![NAT]))); + let f_out = hugr.add_node_with_parent(f, ops::Output::new(type_row![NAT, NAT])); + let noop = hugr.add_node_with_parent(f, LeafOp::Noop { ty: NAT }); + + hugr.connect(f_in, 0, noop, 0); + hugr.connect(noop, 0, f_out, 0); + hugr.connect(noop, 0, f_out, 1); } - hugr.update_validate(&PRELUDE_REGISTRY).unwrap(); + hugr.update_validate(&PRELUDE_REGISTRY)?; + + Ok(()) } } diff --git a/quantinuum-hugr/src/hugr/rewrite.rs b/quantinuum-hugr/src/hugr/rewrite.rs index 3b1ccaa09..151fcaca0 100644 --- a/quantinuum-hugr/src/hugr/rewrite.rs +++ b/quantinuum-hugr/src/hugr/rewrite.rs @@ -75,18 +75,19 @@ impl Rewrite for Transactional { } // Try to backup just the contents of this HugrMut. let mut backup = Hugr::new(h.root_type().clone()); - backup.insert_from_view(backup.root(), h).unwrap(); + backup.insert_from_view(backup.root(), h); let r = self.underlying.apply(h); fn first_child(h: &impl HugrView) -> Option { h.children(h.root()).next() } if r.is_err() { // Try to restore backup. - h.replace_op(h.root(), backup.root_type().clone()).unwrap(); + h.replace_op(h.root(), backup.root_type().clone()) + .expect("The root replacement should always match the old root type"); while let Some(child) = first_child(h) { - h.remove_node(child).unwrap(); + h.remove_node(child); } - h.insert_from_view(h.root(), &backup).unwrap(); + h.insert_from_view(h.root(), &backup); } r } diff --git a/quantinuum-hugr/src/hugr/rewrite/consts.rs b/quantinuum-hugr/src/hugr/rewrite/consts.rs index e1a0fd487..4927e50fc 100644 --- a/quantinuum-hugr/src/hugr/rewrite/consts.rs +++ b/quantinuum-hugr/src/hugr/rewrite/consts.rs @@ -2,10 +2,7 @@ use std::iter; -use crate::{ - hugr::{HugrError, HugrMut}, - HugrView, Node, -}; +use crate::{hugr::HugrMut, HugrView, Node}; use itertools::Itertools; use thiserror::Error; @@ -24,9 +21,6 @@ pub enum RemoveError { /// Node in use. #[error("Node: {0:?} has non-zero outgoing connections.")] ValueUsed(Node), - /// Removal error - #[error("Removing node caused error: {0:?}.")] - RemoveFail(#[from] HugrError), } impl Rewrite for RemoveLoadConstant { @@ -65,7 +59,7 @@ impl Rewrite for RemoveLoadConstant { .exactly_one() .ok() .expect("Validation should check a Const is connected to LoadConstant."); - h.remove_node(node)?; + h.remove_node(node); Ok(source) } @@ -109,7 +103,7 @@ impl Rewrite for RemoveConst { let parent = h .get_parent(node) .expect("Const node without a parent shouldn't happen."); - h.remove_node(node)?; + h.remove_node(node); Ok(parent) } @@ -189,7 +183,7 @@ mod test { ); // remove the use - h.remove_node(tup_node)?; + h.remove_node(tup_node); // remove first load let reported_con_node = h.apply_rewrite(remove_1)?; diff --git a/quantinuum-hugr/src/hugr/rewrite/inline_dfg.rs b/quantinuum-hugr/src/hugr/rewrite/inline_dfg.rs index 0d576e292..ac4105b41 100644 --- a/quantinuum-hugr/src/hugr/rewrite/inline_dfg.rs +++ b/quantinuum-hugr/src/hugr/rewrite/inline_dfg.rs @@ -53,14 +53,14 @@ impl Rewrite for InlineDFG { let parent = h.get_parent(n).unwrap(); let [input, output] = h.get_io(n).unwrap(); for ch in h.children(n).skip(2).collect::>().into_iter() { - h.set_parent(ch, parent).unwrap(); + h.set_parent(ch, parent); } // DFG Inputs. Deal with Order inputs first for (src_n, src_p) in h.linked_outputs(n, oth_in).collect::>() { // Order edge from src_n to DFG => add order edge to each successor of Input node debug_assert_eq!(Some(src_p), h.get_optype(src_n).other_output_port()); for tgt_n in h.output_neighbours(input).collect::>() { - h.add_other_edge(src_n, tgt_n).unwrap(); + h.add_other_edge(src_n, tgt_n); } } // And remaining (Value) inputs @@ -73,24 +73,24 @@ impl Rewrite for InlineDFG { }; // Hugr is invalid if there is no output linked to the DFG input. let (src_n, src_p) = h.single_linked_output(n, inp).unwrap(); - h.disconnect(n, inp).unwrap(); // These disconnects allow permutations to work trivially. + h.disconnect(n, inp); // These disconnects allow permutations to work trivially. let outp = OutgoingPort::from(inp.index()); let targets = h.linked_inputs(input, outp).collect::>(); - h.disconnect(input, outp).unwrap(); + h.disconnect(input, outp); for (tgt_n, tgt_p) in targets { - h.connect(src_n, src_p, tgt_n, tgt_p).unwrap(); + h.connect(src_n, src_p, tgt_n, tgt_p); } // Ensure order-successors of Input node execute after any node producing an input for (tgt, _) in input_ord_succs.iter() { - h.add_other_edge(src_n, *tgt).unwrap(); + h.add_other_edge(src_n, *tgt); } } // DFG Outputs. Deal with Order outputs first. for (tgt_n, tgt_p) in h.linked_inputs(n, oth_out).collect::>() { debug_assert_eq!(Some(tgt_p), h.get_optype(tgt_n).other_input_port()); for src_n in h.input_neighbours(output).collect::>() { - h.add_other_edge(src_n, tgt_n).unwrap(); + h.add_other_edge(src_n, tgt_n); } } // And remaining (Value) outputs @@ -104,21 +104,21 @@ impl Rewrite for InlineDFG { let inpp = IncomingPort::from(outport.index()); // Hugr is invalid if the Output node has no corresponding input let (src_n, src_p) = h.single_linked_output(output, inpp).unwrap(); - h.disconnect(output, inpp).unwrap(); + h.disconnect(output, inpp); for (tgt_n, tgt_p) in h.linked_inputs(n, outport).collect::>() { - h.connect(src_n, src_p, tgt_n, tgt_p).unwrap(); + h.connect(src_n, src_p, tgt_n, tgt_p); // Ensure order-predecessors of Output node execute before any node consuming a DFG output for (src, _) in output_ord_preds.iter() { - h.add_other_edge(*src, tgt_n).unwrap(); + h.add_other_edge(*src, tgt_n); } } - h.disconnect(n, outport).unwrap(); + h.disconnect(n, outport); } - h.remove_node(input).unwrap(); - h.remove_node(output).unwrap(); + h.remove_node(input); + h.remove_node(output); assert!(h.children(n).next().is_none()); - h.remove_node(n).unwrap(); + h.remove_node(n); Ok([n, input, output]) } diff --git a/quantinuum-hugr/src/hugr/rewrite/insert_identity.rs b/quantinuum-hugr/src/hugr/rewrite/insert_identity.rs index e3fd29318..540ff05cc 100644 --- a/quantinuum-hugr/src/hugr/rewrite/insert_identity.rs +++ b/quantinuum-hugr/src/hugr/rewrite/insert_identity.rs @@ -75,21 +75,17 @@ impl Rewrite for IdentityInsertion { .single_linked_output(self.post_node, self.post_port) .expect("Value kind input can only have one connection."); - h.disconnect(self.post_node, self.post_port).unwrap(); + h.disconnect(self.post_node, self.post_port); let parent = h .get_parent(self.post_node) .ok_or(IdentityInsertionError::InvalidParentNode)?; if !OpTag::DataflowParent.is_superset(h.get_optype(parent).tag()) { return Err(IdentityInsertionError::InvalidParentNode); } - let new_node = h - .add_node_with_parent(parent, LeafOp::Noop { ty }) - .expect("Parent validity already checked."); - h.connect(pre_node, pre_port, new_node, 0) - .expect("Should only fail if ports don't exist."); - - h.connect(new_node, 0, self.post_node, self.post_port) - .expect("Should only fail if ports don't exist."); + let new_node = h.add_node_with_parent(parent, LeafOp::Noop { ty }); + h.connect(pre_node, pre_port, new_node, 0); + + h.connect(new_node, 0, self.post_node, self.post_port); Ok(new_node) } diff --git a/quantinuum-hugr/src/hugr/rewrite/outline_cfg.rs b/quantinuum-hugr/src/hugr/rewrite/outline_cfg.rs index 42156433a..c773d5da7 100644 --- a/quantinuum-hugr/src/hugr/rewrite/outline_cfg.rs +++ b/quantinuum-hugr/src/hugr/rewrite/outline_cfg.rs @@ -150,9 +150,7 @@ impl Rewrite for OutlineCfg { new_block_bldr .set_outputs(pred_wire, cfg.outputs()) .unwrap(); - let ins_res = h - .insert_hugr(outer_cfg, new_block_bldr.hugr().clone()) - .unwrap(); + let ins_res = h.insert_hugr(outer_cfg, new_block_bldr.hugr().clone()); ( ins_res.new_root, *ins_res.node_map.get(&cfg.node()).unwrap(), @@ -165,14 +163,14 @@ impl Rewrite for OutlineCfg { .collect(); for (pred, br) in preds { if !self.blocks.contains(&pred) { - h.disconnect(pred, br).unwrap(); - h.connect(pred, br, new_block, 0).unwrap(); + h.disconnect(pred, br); + h.connect(pred, br, new_block, 0); } } if entry == outer_entry { // new_block must be the entry node, i.e. first child, of the enclosing CFG // (the current entry node will be reparented inside new_block below) - h.move_before_sibling(new_block, outer_entry).unwrap(); + h.move_before_sibling(new_block, outer_entry); } // 4(a). Exit edges. @@ -187,9 +185,9 @@ impl Rewrite for OutlineCfg { .exactly_one() .ok() // NodePorts does not implement Debug .unwrap(); - h.disconnect(exit, exit_port).unwrap(); + h.disconnect(exit, exit_port); // And connect new_block to outside instead - h.connect(new_block, 0, outside, 0).unwrap(); + h.connect(new_block, 0, outside, 0); // 5. Children of new CFG. let inner_exit = { @@ -198,12 +196,12 @@ impl Rewrite for OutlineCfg { let h = h.hugr_mut(); let inner_exit = h.children(cfg_node).exactly_one().ok().unwrap(); // Entry node must be first - h.move_before_sibling(entry, inner_exit).unwrap(); + h.move_before_sibling(entry, inner_exit); // And remaining nodes for n in self.blocks { // Do not move the entry node, as we have already if n != entry { - h.set_parent(n, cfg_node).unwrap(); + h.set_parent(n, cfg_node); } } inner_exit @@ -215,7 +213,7 @@ impl Rewrite for OutlineCfg { SiblingMut::try_new(h, new_block).unwrap(); let mut in_cfg_view: SiblingMut<'_, CfgID> = SiblingMut::try_new(&mut in_bb_view, cfg_node).unwrap(); - in_cfg_view.connect(exit, exit_port, inner_exit, 0).unwrap(); + in_cfg_view.connect(exit, exit_port, inner_exit, 0); Ok((new_block, cfg_node)) } diff --git a/quantinuum-hugr/src/hugr/rewrite/replace.rs b/quantinuum-hugr/src/hugr/rewrite/replace.rs index 863965376..1943f8f53 100644 --- a/quantinuum-hugr/src/hugr/rewrite/replace.rs +++ b/quantinuum-hugr/src/hugr/rewrite/replace.rs @@ -235,19 +235,27 @@ impl Rewrite for Replacement { } e.check_src(h, e)?; } - self.mu_out.iter().try_for_each(|e| { - self.replacement.valid_non_root(e.src).map_err(|_| { - ReplaceError::BadEdgeSpec(Direction::Outgoing, WhichHugr::Replacement, e.clone()) + self.mu_out + .iter() + .try_for_each(|e| match self.replacement.valid_non_root(e.src) { + true => e.check_src(&self.replacement, e), + false => Err(ReplaceError::BadEdgeSpec( + Direction::Outgoing, + WhichHugr::Replacement, + e.clone(), + )), })?; - e.check_src(&self.replacement, e) - })?; // Edge targets... - self.mu_inp.iter().try_for_each(|e| { - self.replacement.valid_non_root(e.tgt).map_err(|_| { - ReplaceError::BadEdgeSpec(Direction::Incoming, WhichHugr::Replacement, e.clone()) + self.mu_inp + .iter() + .try_for_each(|e| match self.replacement.valid_non_root(e.tgt) { + true => e.check_tgt(&self.replacement, e), + false => Err(ReplaceError::BadEdgeSpec( + Direction::Incoming, + WhichHugr::Replacement, + e.clone(), + )), })?; - e.check_tgt(&self.replacement, e) - })?; for e in self.mu_out.iter().chain(self.mu_new.iter()) { if !h.contains_node(e.tgt) || removed.contains(&e.tgt) { return Err(ReplaceError::BadEdgeSpec( @@ -274,8 +282,7 @@ impl Rewrite for Replacement { // 1. Add all the new nodes. Note this includes replacement.root(), which we don't want. // TODO what would an error here mean? e.g. malformed self.replacement?? - let InsertionResult { new_root, node_map } = - h.insert_hugr(parent, self.replacement).unwrap(); + let InsertionResult { new_root, node_map } = h.insert_hugr(parent, self.replacement); // 2. Add new edges from existing to copied nodes according to mu_in let translate_idx = |n| node_map.get(&n).copied().ok_or(WhichHugr::Replacement); @@ -298,13 +305,13 @@ impl Rewrite for Replacement { let mut remove_top_sibs = self.removal.iter(); for new_node in h.children(new_root).collect::>().into_iter() { if let Some(top_sib) = remove_top_sibs.next() { - h.move_before_sibling(new_node, *top_sib).unwrap(); + h.move_before_sibling(new_node, *top_sib); } else { - h.set_parent(new_node, parent).unwrap(); + h.set_parent(new_node, parent); } } debug_assert!(h.children(new_root).next().is_none()); - h.remove_node(new_root).unwrap(); + h.remove_node(new_root); // 6. Transfer to keys of `transfers` children of the corresponding values. for (new_parent, &old_parent) in self.adoptions.iter() { @@ -315,14 +322,12 @@ impl Rewrite for Replacement { None => break, Some(c) => c, }; - h.set_parent(ch, *new_parent).unwrap(); + h.set_parent(ch, *new_parent); } } // 7. Remove remaining nodes - to_remove - .into_iter() - .for_each(|n| h.remove_node(n).unwrap()); + to_remove.into_iter().for_each(|n| h.remove_node(n)); Ok(()) } @@ -347,26 +352,34 @@ fn transfer_edges<'a>( .map_err(|h| ReplaceError::BadEdgeSpec(Direction::Incoming, h, oe.clone()))?, ..oe.clone() }; - h.valid_node(e.src).map_err(|_| { - ReplaceError::BadEdgeSpec(Direction::Outgoing, WhichHugr::Retained, oe.clone()) - })?; - h.valid_node(e.tgt).map_err(|_| { - ReplaceError::BadEdgeSpec(Direction::Incoming, WhichHugr::Retained, oe.clone()) - })?; + if !h.valid_node(e.src) { + return Err(ReplaceError::BadEdgeSpec( + Direction::Outgoing, + WhichHugr::Retained, + oe.clone(), + )); + } + if !h.valid_node(e.tgt) { + return Err(ReplaceError::BadEdgeSpec( + Direction::Incoming, + WhichHugr::Retained, + oe.clone(), + )); + }; e.check_src(h, oe)?; e.check_tgt(h, oe)?; match e.kind { NewEdgeKind::Order => { - h.add_other_edge(e.src, e.tgt).unwrap(); + h.add_other_edge(e.src, e.tgt); } NewEdgeKind::Value { src_pos, tgt_pos } | NewEdgeKind::Static { src_pos, tgt_pos } => { if let Some(legal_src_ancestors) = legal_src_ancestors { e.check_existing_edge(h, legal_src_ancestors, || oe.clone())?; - h.disconnect(e.tgt, tgt_pos).unwrap(); + h.disconnect(e.tgt, tgt_pos); } - h.connect(e.src, src_pos, e.tgt, tgt_pos).unwrap(); + h.connect(e.src, src_pos, e.tgt, tgt_pos); } - NewEdgeKind::ControlFlow { src_pos } => h.connect(e.src, src_pos, e.tgt, 0).unwrap(), + NewEdgeKind::ControlFlow { src_pos } => h.connect(e.src, src_pos, e.tgt, 0), } } Ok(()) @@ -525,7 +538,7 @@ mod test { // at least when https://github.com/CQCL/issues/388 is fixed extension_delta: ExtensionSet::new(), }, - )?; + ); let r_df1 = replacement.add_node_with_parent( r_bb, DFG { @@ -534,16 +547,16 @@ mod test { simple_unary_plus(intermed.clone()), ), }, - )?; + ); let r_df2 = replacement.add_node_with_parent( r_bb, DFG { signature: FunctionType::new(intermed, simple_unary_plus(just_list.clone())), }, - )?; + ); [0, 1] .iter() - .try_for_each(|p| replacement.connect(r_df1, *p + 1, r_df2, *p))?; + .for_each(|p| replacement.connect(r_df1, *p + 1, r_df2, *p)); { let inp = replacement.add_node_before( @@ -551,16 +564,16 @@ mod test { ops::Input { types: just_list.clone(), }, - )?; + ); let out = replacement.add_node_before( r_df1, ops::Output { types: simple_unary_plus(just_list), }, - )?; - replacement.connect(inp, 0, r_df1, 0)?; - replacement.connect(r_df2, 0, out, 0)?; - replacement.connect(r_df2, 1, out, 1)?; + ); + replacement.connect(inp, 0, r_df1, 0); + replacement.connect(r_df2, 0, out, 0); + replacement.connect(r_df2, 1, out, 1); } h.apply_rewrite(Replacement { @@ -685,13 +698,13 @@ mod test { Case { signature: utou.clone(), }, - )?; + ); let r2 = rep1.add_node_with_parent( rep1.root(), Case { signature: utou.clone(), }, - )?; + ); let mut r = Replacement { removal: vec![case1, case2], replacement: rep1, diff --git a/quantinuum-hugr/src/hugr/rewrite/simple_replace.rs b/quantinuum-hugr/src/hugr/rewrite/simple_replace.rs index 694faa835..f7d836d58 100644 --- a/quantinuum-hugr/src/hugr/rewrite/simple_replace.rs +++ b/quantinuum-hugr/src/hugr/rewrite/simple_replace.rs @@ -101,12 +101,12 @@ impl Rewrite for SimpleReplacement { for &node in replacement_inner_nodes { // Add the nodes. let op: &OpType = self.replacement.get_optype(node); - let new_node = h.add_node_after(self_output_node, op.clone()).unwrap(); + let new_node = h.add_node_after(self_output_node, op.clone()); index_map.insert(node, new_node); // Move the metadata let meta: Option = self.replacement.take_node_metadata(node); - h.overwrite_node_metadata(new_node, meta).unwrap(); + h.overwrite_node_metadata(new_node, meta); } // Add edges between all newly added nodes matching those in replacement. // TODO This will probably change when implicit copies are implemented. @@ -116,8 +116,7 @@ impl Rewrite for SimpleReplacement { for target in self.replacement.linked_inputs(node, outport) { if self.replacement.get_optype(target.0).tag() != OpTag::Output { let new_target = index_map.get(&target.0).unwrap(); - h.connect(*new_node, outport, *new_target, target.1) - .unwrap(); + h.connect(*new_node, outport, *new_target, target.1); } } } @@ -130,15 +129,14 @@ impl Rewrite for SimpleReplacement { let (rem_inp_pred_node, rem_inp_pred_port) = h .single_linked_output(*rem_inp_node, *rem_inp_port) .unwrap(); - h.disconnect(*rem_inp_node, *rem_inp_port).unwrap(); + h.disconnect(*rem_inp_node, *rem_inp_port); let new_inp_node = index_map.get(rep_inp_node).unwrap(); h.connect( rem_inp_pred_node, rem_inp_pred_port, *new_inp_node, *rep_inp_port, - ) - .unwrap(); + ); } } // 3.3. For each q = self.nu_out[p] such that the predecessor of q is not an Input port, add an @@ -150,14 +148,13 @@ impl Rewrite for SimpleReplacement { .unwrap(); if self.replacement.get_optype(rep_out_pred_node).tag() != OpTag::Input { let new_out_node = index_map.get(&rep_out_pred_node).unwrap(); - h.disconnect(*rem_out_node, *rem_out_port).unwrap(); + h.disconnect(*rem_out_node, *rem_out_port); h.connect( *new_out_node, rep_out_pred_port, *rem_out_node, *rem_out_port, - ) - .unwrap(); + ); } } // 3.4. For each q = self.nu_out[p1], p0 = self.nu_inp[q], add an edge from the predecessor of p0 @@ -169,20 +166,19 @@ impl Rewrite for SimpleReplacement { let (rem_inp_pred_node, rem_inp_pred_port) = h .single_linked_output(*rem_inp_node, *rem_inp_port) .unwrap(); - h.disconnect(*rem_inp_node, *rem_inp_port).unwrap(); - h.disconnect(*rem_out_node, *rem_out_port).unwrap(); + h.disconnect(*rem_inp_node, *rem_inp_port); + h.disconnect(*rem_out_node, *rem_out_port); h.connect( rem_inp_pred_node, rem_inp_pred_port, *rem_out_node, *rem_out_port, - ) - .unwrap(); + ); } } // 3.5. Remove all nodes in self.removal and edges between them. for &node in self.subgraph.nodes() { - h.remove_node(node).unwrap(); + h.remove_node(node); } Ok(()) } @@ -635,8 +631,8 @@ pub(in crate::hugr::rewrite) mod test { } }) .collect(); - replacement.remove_node(in_).unwrap(); - replacement.remove_node(out).unwrap(); + replacement.remove_node(in_); + replacement.remove_node(out); Replacement { removal: s.subgraph.nodes().to_vec(), replacement, diff --git a/quantinuum-hugr/src/hugr/serialize.rs b/quantinuum-hugr/src/hugr/serialize.rs index da6805d9c..1e46cb418 100644 --- a/quantinuum-hugr/src/hugr/serialize.rs +++ b/quantinuum-hugr/src/hugr/serialize.rs @@ -14,7 +14,7 @@ use portgraph::{Direction, LinkError, PortView}; use serde::{Deserialize, Deserializer, Serialize}; -use super::{HugrError, HugrMut, HugrView}; +use super::{HugrMut, HugrView}; /// A wrapper over the available HUGR serialization formats. /// @@ -80,9 +80,6 @@ pub enum HUGRSerializationError { /// The node that has the port without offset. node: Node, }, - /// Error building HUGR. - #[error("HugrError: {0:?}")] - HugrError(#[from] HugrError), /// First node in node list must be the HUGR root. #[error("The first node in the node list has parent {0:?}, should be itself (index 0)")] FirstNodeNotRoot(Node), @@ -209,7 +206,7 @@ impl TryFrom for Hugr { hugr.add_node_with_parent( node_ser.parent, NodeType::new(node_ser.op, node_ser.input_extensions), - )?; + ); } for (node, metadata) in metadata.into_iter().enumerate() { @@ -240,7 +237,7 @@ impl TryFrom for Hugr { let src_port = unwrap_offset(src, from_offset, Direction::Outgoing, &hugr)?; let dst_port = unwrap_offset(dst, to_offset, Direction::Incoming, &hugr)?; - hugr.connect(src, src_port, dst, dst_port)?; + hugr.connect(src, src_port, dst, dst_port); } Ok(hugr) @@ -501,22 +498,23 @@ pub mod test { } #[test] - fn hierarchy_order() { + fn hierarchy_order() -> Result<(), Box> { let mut hugr = closed_dfg_root_hugr(FunctionType::new(vec![QB], vec![QB])); let [old_in, out] = hugr.get_io(hugr.root()).unwrap(); - hugr.connect(old_in, 0, out, 0).unwrap(); + hugr.connect(old_in, 0, out, 0); // Now add a new input let new_in = hugr.add_node(Input::new([QB].to_vec()).into()); - hugr.disconnect(old_in, OutgoingPort::from(0)).unwrap(); - hugr.connect(new_in, 0, out, 0).unwrap(); - hugr.move_before_sibling(new_in, old_in).unwrap(); - hugr.remove_node(old_in).unwrap(); - hugr.update_validate(&PRELUDE_REGISTRY).unwrap(); + hugr.disconnect(old_in, OutgoingPort::from(0)); + hugr.connect(new_in, 0, out, 0); + hugr.move_before_sibling(new_in, old_in); + hugr.remove_node(old_in); + hugr.update_validate(&PRELUDE_REGISTRY)?; let new_hugr: Hugr = check_hugr_roundtrip(&hugr); new_hugr.validate(&EMPTY_REG).unwrap_err(); - new_hugr.validate(&PRELUDE_REGISTRY).unwrap(); + new_hugr.validate(&PRELUDE_REGISTRY)?; + Ok(()) } #[test] diff --git a/quantinuum-hugr/src/hugr/validate/test.rs b/quantinuum-hugr/src/hugr/validate/test.rs index 5f8210807..bf75c93a7 100644 --- a/quantinuum-hugr/src/hugr/validate/test.rs +++ b/quantinuum-hugr/src/hugr/validate/test.rs @@ -8,7 +8,7 @@ use crate::builder::{ use crate::extension::prelude::{BOOL_T, PRELUDE, USIZE_T}; use crate::extension::{Extension, ExtensionId, TypeDefBound, EMPTY_REG, PRELUDE_REGISTRY}; use crate::hugr::hugrmut::sealed::HugrMutInternals; -use crate::hugr::{HugrError, HugrMut, NodeType}; +use crate::hugr::{HugrMut, NodeType}; use crate::ops::dataflow::IOTrait; use crate::ops::{self, Const, LeafOp, OpType}; use crate::std_extensions::logic::test::{and_op, or_op}; @@ -33,7 +33,7 @@ fn make_simple_hugr(copies: usize) -> (Hugr, Node) { let mut b = Hugr::default(); let root = b.root(); - let def = b.add_node_with_parent(root, def_op).unwrap(); + let def = b.add_node_with_parent(root, def_op); let _ = add_df_children(&mut b, def, copies); (b, def) @@ -43,19 +43,13 @@ fn make_simple_hugr(copies: usize) -> (Hugr, Node) { /// /// Returns the node indices of each of the operations. fn add_df_children(b: &mut Hugr, parent: Node, copies: usize) -> (Node, Node, Node) { - let input = b - .add_node_with_parent(parent, ops::Input::new(type_row![BOOL_T])) - .unwrap(); - let output = b - .add_node_with_parent(parent, ops::Output::new(vec![BOOL_T; copies])) - .unwrap(); - let copy = b - .add_node_with_parent(parent, LeafOp::Noop { ty: BOOL_T }) - .unwrap(); + let input = b.add_node_with_parent(parent, ops::Input::new(type_row![BOOL_T])); + let output = b.add_node_with_parent(parent, ops::Output::new(vec![BOOL_T; copies])); + let copy = b.add_node_with_parent(parent, LeafOp::Noop { ty: BOOL_T }); - b.connect(input, 0, copy, 0).unwrap(); + b.connect(input, 0, copy, 0); for i in 0..copies { - b.connect(copy, 0, output, i).unwrap(); + b.connect(copy, 0, output, i); } (input, copy, output) @@ -79,7 +73,7 @@ fn invalid_root() { b.validate(&EMPTY_REG), Err(ValidationError::NoParent { node }) => assert_eq!(node, other) ); - b.set_parent(other, root).unwrap(); + b.set_parent(other, root); b.replace_op(other, NodeType::new_pure(declare_op)).unwrap(); b.add_ports(other, Direction::Outgoing, 1); assert_eq!(b.validate(&EMPTY_REG), Ok(())); @@ -136,15 +130,13 @@ fn children_restrictions() { // Add a definition without children let def_sig = FunctionType::new(type_row![BOOL_T], type_row![BOOL_T, BOOL_T]); - let new_def = b - .add_node_with_parent( - root, - ops::FuncDefn { - signature: def_sig.into(), - name: "main".into(), - }, - ) - .unwrap(); + let new_def = b.add_node_with_parent( + root, + ops::FuncDefn { + signature: def_sig.into(), + name: "main".into(), + }, + ); assert_matches!( b.update_validate(&EMPTY_REG), Err(ValidationError::ContainerWithoutChildren { node, .. }) => assert_eq!(node, new_def) @@ -152,19 +144,17 @@ fn children_restrictions() { // Add children to the definition, but move it to be a child of the copy add_df_children(&mut b, new_def, 2); - b.set_parent(new_def, copy).unwrap(); + b.set_parent(new_def, copy); assert_matches!( b.update_validate(&EMPTY_REG), Err(ValidationError::NonContainerWithChildren { node, .. }) => assert_eq!(node, copy) ); let closure = b.infer_extensions().unwrap(); - b.set_parent(new_def, root).unwrap(); + b.set_parent(new_def, root); // After moving the previous definition to a valid place, // add an input node to the module subgraph - let new_input = b - .add_node_with_parent(root, ops::Input::new(type_row![])) - .unwrap(); + let new_input = b.add_node_with_parent(root, ops::Input::new(type_row![])); assert_matches!( b.validate_with_extension_closure(closure, &EMPTY_REG), Err(ValidationError::InvalidParentOp { parent, child, .. }) => {assert_eq!(parent, root); assert_eq!(child, new_input)} @@ -221,7 +211,7 @@ fn df_children_restrictions() { } #[test] -fn test_ext_edge() -> Result<(), HugrError> { +fn test_ext_edge() { let mut h = closed_dfg_root_hugr(FunctionType::new( type_row![BOOL_T, BOOL_T], type_row![BOOL_T], @@ -234,26 +224,26 @@ fn test_ext_edge() -> Result<(), HugrError> { ops::DFG { signature: FunctionType::new_endo(type_row![BOOL_T]), }, - )?; + ); // this Xor has its 2nd input unconnected let sub_op = { - let sub_input = h.add_node_with_parent(sub_dfg, ops::Input::new(type_row![BOOL_T]))?; - let sub_output = h.add_node_with_parent(sub_dfg, ops::Output::new(type_row![BOOL_T]))?; - let sub_op = h.add_node_with_parent(sub_dfg, and_op())?; - h.connect(sub_input, 0, sub_op, 0)?; - h.connect(sub_op, 0, sub_output, 0)?; + let sub_input = h.add_node_with_parent(sub_dfg, ops::Input::new(type_row![BOOL_T])); + let sub_output = h.add_node_with_parent(sub_dfg, ops::Output::new(type_row![BOOL_T])); + let sub_op = h.add_node_with_parent(sub_dfg, and_op()); + h.connect(sub_input, 0, sub_op, 0); + h.connect(sub_op, 0, sub_output, 0); sub_op }; - h.connect(input, 0, sub_dfg, 0)?; - h.connect(sub_dfg, 0, output, 0)?; + h.connect(input, 0, sub_dfg, 0); + h.connect(sub_dfg, 0, output, 0); assert_matches!( h.update_validate(&EMPTY_REG), Err(ValidationError::UnconnectedPort { .. }) ); - h.connect(input, 1, sub_op, 1)?; + h.connect(input, 1, sub_op, 1); assert_matches!( h.update_validate(&EMPTY_REG), Err(ValidationError::InterGraphEdgeError( @@ -261,18 +251,17 @@ fn test_ext_edge() -> Result<(), HugrError> { )) ); //Order edge. This will need metadata indicating its purpose. - h.add_other_edge(input, sub_dfg)?; + h.add_other_edge(input, sub_dfg); h.update_validate(&EMPTY_REG).unwrap(); - Ok(()) } #[test] -fn test_local_const() -> Result<(), HugrError> { +fn test_local_const() { let mut h = closed_dfg_root_hugr(FunctionType::new(type_row![BOOL_T], type_row![BOOL_T])); let [input, output] = h.get_io(h.root()).unwrap(); - let and = h.add_node_with_parent(h.root(), and_op())?; - h.connect(input, 0, and, 0)?; - h.connect(and, 0, output, 0)?; + let and = h.add_node_with_parent(h.root(), and_op()); + h.connect(input, 0, and, 0); + h.connect(and, 0, output, 0); assert_eq!( h.update_validate(&EMPTY_REG), Err(ValidationError::UnconnectedPort { @@ -287,56 +276,50 @@ fn test_local_const() -> Result<(), HugrError> { .typed_value() .clone(); // Second input of Xor from a constant - let cst = h.add_node_with_parent(h.root(), const_op)?; - let lcst = h.add_node_with_parent(h.root(), ops::LoadConstant { datatype: BOOL_T })?; + let cst = h.add_node_with_parent(h.root(), const_op); + let lcst = h.add_node_with_parent(h.root(), ops::LoadConstant { datatype: BOOL_T }); - h.connect(cst, 0, lcst, 0)?; - h.connect(lcst, 0, and, 1)?; + h.connect(cst, 0, lcst, 0); + h.connect(lcst, 0, and, 1); assert_eq!(h.static_source(lcst), Some(cst)); // There is no edge from Input to LoadConstant, but that's OK: h.update_validate(&EMPTY_REG).unwrap(); - Ok(()) } #[test] -fn dfg_with_cycles() -> Result<(), HugrError> { +fn dfg_with_cycles() { let mut h = closed_dfg_root_hugr(FunctionType::new( type_row![BOOL_T, BOOL_T], type_row![BOOL_T], )); let [input, output] = h.get_io(h.root()).unwrap(); - let or = h.add_node_with_parent(h.root(), or_op())?; - let not1 = h.add_node_with_parent(h.root(), NotOp)?; - let not2 = h.add_node_with_parent(h.root(), NotOp)?; - h.connect(input, 0, or, 0)?; - h.connect(or, 0, not1, 0)?; - h.connect(not1, 0, or, 1)?; - h.connect(input, 1, not2, 0)?; - h.connect(not2, 0, output, 0)?; + let or = h.add_node_with_parent(h.root(), or_op()); + let not1 = h.add_node_with_parent(h.root(), NotOp); + let not2 = h.add_node_with_parent(h.root(), NotOp); + h.connect(input, 0, or, 0); + h.connect(or, 0, not1, 0); + h.connect(not1, 0, or, 1); + h.connect(input, 1, not2, 0); + h.connect(not2, 0, output, 0); // The graph contains a cycle: assert_matches!(h.validate(&EMPTY_REG), Err(ValidationError::NotADag { .. })); - Ok(()) } fn identity_hugr_with_type(t: Type) -> (Hugr, Node) { let mut b = Hugr::default(); let row: TypeRow = vec![t].into(); - let def = b - .add_node_with_parent( - b.root(), - ops::FuncDefn { - name: "main".into(), - signature: FunctionType::new(row.clone(), row.clone()).into(), - }, - ) - .unwrap(); + let def = b.add_node_with_parent( + b.root(), + ops::FuncDefn { + name: "main".into(), + signature: FunctionType::new(row.clone(), row.clone()).into(), + }, + ); - let input = b - .add_node_with_parent(def, ops::Input::new(row.clone())) - .unwrap(); - let output = b.add_node_with_parent(def, ops::Output::new(row)).unwrap(); - b.connect(input, 0, output, 0).unwrap(); + let input = b.add_node_with_parent(def, ops::Input::new(row.clone())); + let output = b.add_node_with_parent(def, ops::Output::new(row)); + b.connect(input, 0, output, 0); (b, def) } #[test] @@ -586,21 +569,16 @@ mod extension_tests { let const_op = ops::Const::unit_sum(0, tuple_sum_size as u8); let tag_type = Type::new_unit_sum(tuple_sum_size as u8); - let input = b - .add_node_with_parent(parent, ops::Input::new(type_row![BOOL_T])) - .unwrap(); - let output = b - .add_node_with_parent(parent, ops::Output::new(vec![tag_type.clone(), BOOL_T])) - .unwrap(); - let tag_def = b.add_node_with_parent(b.root(), const_op).unwrap(); - let tag = b - .add_node_with_parent(parent, ops::LoadConstant { datatype: tag_type }) - .unwrap(); + let input = b.add_node_with_parent(parent, ops::Input::new(type_row![BOOL_T])); + let output = + b.add_node_with_parent(parent, ops::Output::new(vec![tag_type.clone(), BOOL_T])); + let tag_def = b.add_node_with_parent(b.root(), const_op); + let tag = b.add_node_with_parent(parent, ops::LoadConstant { datatype: tag_type }); - b.connect(tag_def, 0, tag, 0).unwrap(); - b.add_other_edge(input, tag).unwrap(); - b.connect(tag, 0, output, 0).unwrap(); - b.connect(input, 0, output, 1).unwrap(); + b.connect(tag_def, 0, tag, 0); + b.add_other_edge(input, tag); + b.connect(tag, 0, output, 0); + b.connect(input, 0, output, 1); (input, tag_def, tag, output) } @@ -634,46 +612,40 @@ mod extension_tests { let cfg = copy; // Construct a valid CFG, with one BasicBlock node and one exit node - let block = b - .add_node_with_parent( - cfg, - ops::DataflowBlock { - inputs: type_row![BOOL_T], - tuple_sum_rows: vec![type_row![]], - other_outputs: type_row![BOOL_T], - extension_delta: ExtensionSet::new(), - }, - ) - .unwrap(); + let block = b.add_node_with_parent( + cfg, + ops::DataflowBlock { + inputs: type_row![BOOL_T], + tuple_sum_rows: vec![type_row![]], + other_outputs: type_row![BOOL_T], + extension_delta: ExtensionSet::new(), + }, + ); add_block_children(&mut b, block, 1); - let exit = b - .add_node_with_parent( - cfg, - ops::ExitBlock { - cfg_outputs: type_row![BOOL_T], - }, - ) - .unwrap(); - b.add_other_edge(block, exit).unwrap(); + let exit = b.add_node_with_parent( + cfg, + ops::ExitBlock { + cfg_outputs: type_row![BOOL_T], + }, + ); + b.add_other_edge(block, exit); assert_eq!(b.update_validate(&EMPTY_REG), Ok(())); // Test malformed errors // Add an internal exit node - let exit2 = b - .add_node_after( - exit, - ops::ExitBlock { - cfg_outputs: type_row![BOOL_T], - }, - ) - .unwrap(); + let exit2 = b.add_node_after( + exit, + ops::ExitBlock { + cfg_outputs: type_row![BOOL_T], + }, + ); assert_matches!( b.validate(&EMPTY_REG), Err(ValidationError::InvalidChildren { parent, source: ChildrenValidationError::InternalExitChildren { child, .. }, .. }) => {assert_eq!(parent, cfg); assert_eq!(child, exit2.pg_index())} ); - b.remove_node(exit2).unwrap(); + b.remove_node(exit2); // Change the types in the BasicBlock node to work on qubits instead of bits b.replace_op( @@ -714,38 +686,32 @@ mod extension_tests { signature: FunctionType::new(type_row![USIZE_T], type_row![USIZE_T]), })); - let input = hugr - .add_node_with_parent( - hugr.root(), - NodeType::new_pure(ops::Input { + let input = hugr.add_node_with_parent( + hugr.root(), + NodeType::new_pure(ops::Input { + types: type_row![USIZE_T], + }), + ); + let output = hugr.add_node_with_parent( + hugr.root(), + NodeType::new( + ops::Output { types: type_row![USIZE_T], - }), - ) - .unwrap(); - let output = hugr - .add_node_with_parent( - hugr.root(), - NodeType::new( - ops::Output { - types: type_row![USIZE_T], - }, - Some(XB.into()), - ), - ) - .unwrap(); + }, + Some(XB.into()), + ), + ); - let lift = hugr - .add_node_with_parent( - hugr.root(), - NodeType::new_pure(ops::LeafOp::Lift { - type_row: type_row![USIZE_T], - new_extension: XB, - }), - ) - .unwrap(); + let lift = hugr.add_node_with_parent( + hugr.root(), + NodeType::new_pure(ops::LeafOp::Lift { + type_row: type_row![USIZE_T], + new_extension: XB, + }), + ); - hugr.connect(input, 0, lift, 0).unwrap(); - hugr.connect(lift, 0, output, 0).unwrap(); + hugr.connect(input, 0, lift, 0); + hugr.connect(lift, 0, output, 0); let result = hugr.validate(&PRELUDE_REGISTRY); assert_matches!( @@ -873,7 +839,7 @@ mod extension_tests { } #[test] - fn parent_signature_mismatch() -> Result<(), BuildError> { + fn parent_signature_mismatch() { let main_signature = FunctionType::new(type_row![NAT], type_row![NAT]).with_extension_delta(XA); @@ -885,7 +851,7 @@ mod extension_tests { NodeType::new_pure(ops::Input { types: type_row![NAT], }), - )?; + ); let output = hugr.add_node_with_parent( hugr.root(), NodeType::new( @@ -894,8 +860,8 @@ mod extension_tests { }, Some(XA.into()), ), - )?; - hugr.connect(input, 0, output, 0)?; + ); + hugr.connect(input, 0, output, 0); assert_matches!( hugr.validate(&PRELUDE_REGISTRY), @@ -903,6 +869,5 @@ mod extension_tests { ExtensionError::TgtExceedsSrcExtensionsAtPort { .. } )) ); - Ok(()) } } diff --git a/quantinuum-hugr/src/hugr/views.rs b/quantinuum-hugr/src/hugr/views.rs index 000aac4c9..0a0cf7d1b 100644 --- a/quantinuum-hugr/src/hugr/views.rs +++ b/quantinuum-hugr/src/hugr/views.rs @@ -86,33 +86,25 @@ pub trait HugrView: sealed::HugrInternals { fn contains_node(&self, node: Node) -> bool; /// 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)), - } + fn valid_node(&self, node: Node) -> bool { + self.contains_node(node) } /// Validates that a node is a valid root descendant in the graph. /// /// To include the root node use [`HugrView::valid_node`] instead. - /// - /// Returns a [`HugrError::InvalidNode`] otherwise. #[inline] - fn valid_non_root(&self, node: Node) -> Result<(), HugrError> { - match self.root() == node { - true => Err(HugrError::InvalidNode(node)), - false => self.valid_node(node), - } + fn valid_non_root(&self, node: Node) -> bool { + self.root() != node && self.valid_node(node) } /// Returns the parent of a node. #[inline] fn get_parent(&self, node: Node) -> Option { - self.valid_non_root(node).ok()?; + if !self.valid_non_root(node) { + return None; + }; self.base_hugr() .hierarchy .parent(node.pg_index()) @@ -145,7 +137,9 @@ pub trait HugrView: sealed::HugrInternals { /// Retrieve the complete metadata map for a node. fn get_node_metadata(&self, node: Node) -> Option<&NodeMetadataMap> { - self.valid_node(node).ok()?; + if !self.valid_node(node) { + return None; + } self.base_hugr().metadata.get(node.pg_index()).as_ref() } @@ -356,10 +350,7 @@ pub trait HugrView: sealed::HugrInternals { /// /// For a more detailed representation, use the [`HugrView::dot_string`] /// format instead. - fn mermaid_string(&self) -> String - where - Self: Sized, - { + fn mermaid_string(&self) -> String { self.mermaid_string_with_config(RenderConfig { node_indices: true, port_offsets_in_edges: true, @@ -374,10 +365,7 @@ pub trait HugrView: sealed::HugrInternals { /// /// For a more detailed representation, use the [`HugrView::dot_string`] /// format instead. - fn mermaid_string_with_config(&self, config: RenderConfig) -> String - where - Self: Sized, - { + fn mermaid_string_with_config(&self, config: RenderConfig) -> String { let hugr = self.base_hugr(); let graph = self.portgraph(); graph @@ -479,7 +467,6 @@ pub trait HierarchyView<'a>: RootTagged + Sized { } fn check_tag(hugr: &impl HugrView, node: Node) -> Result<(), HugrError> { - hugr.valid_node(node)?; let actual = hugr.get_optype(node).tag(); let required = Required::TAG; if !required.is_superset(actual) { diff --git a/quantinuum-hugr/src/hugr/views/render.rs b/quantinuum-hugr/src/hugr/views/render.rs index 3e40efd71..e876ac6d2 100644 --- a/quantinuum-hugr/src/hugr/views/render.rs +++ b/quantinuum-hugr/src/hugr/views/render.rs @@ -31,7 +31,7 @@ impl Default for RenderConfig { } /// Formatter method to compute a node style. -pub(super) fn node_style( +pub(super) fn node_style( h: &H, config: RenderConfig, ) -> Box NodeStyle + '_> { @@ -49,7 +49,7 @@ pub(super) fn node_style( } /// Formatter method to compute a port style. -pub(super) fn port_style( +pub(super) fn port_style( h: &H, _config: RenderConfig, ) -> Box PortStyle + '_> { @@ -72,7 +72,7 @@ pub(super) fn port_style( /// Formatter method to compute an edge style. #[allow(clippy::type_complexity)] -pub(super) fn edge_style( +pub(super) fn edge_style( h: &H, config: RenderConfig, ) -> Box< diff --git a/quantinuum-hugr/src/hugr/views/root_checked.rs b/quantinuum-hugr/src/hugr/views/root_checked.rs index 242ac8ee7..9e5c41dd2 100644 --- a/quantinuum-hugr/src/hugr/views/root_checked.rs +++ b/quantinuum-hugr/src/hugr/views/root_checked.rs @@ -130,6 +130,6 @@ mod test { // And it's a HugrMut: let nodetype = NodeType::new_pure(LeafOp::MakeTuple { tys: type_row![] }); - bb_v.add_node_with_parent(bb_v.root(), nodetype).unwrap(); + bb_v.add_node_with_parent(bb_v.root(), nodetype); } } diff --git a/quantinuum-hugr/src/hugr/views/sibling.rs b/quantinuum-hugr/src/hugr/views/sibling.rs index a39131502..e07897ac1 100644 --- a/quantinuum-hugr/src/hugr/views/sibling.rs +++ b/quantinuum-hugr/src/hugr/views/sibling.rs @@ -195,6 +195,11 @@ where Root: NodeHandle, { fn try_new(hugr: &'a impl HugrView, root: Node) -> Result { + assert!( + hugr.valid_node(root), + "Cannot create a sibling graph from an invalid node {}.", + root + ); check_tag::(hugr, root)?; Ok(Self::new_unchecked(hugr, root)) } @@ -373,7 +378,7 @@ mod test { use crate::builder::{Container, Dataflow, DataflowSubContainer, HugrBuilder, ModuleBuilder}; use crate::extension::PRELUDE_REGISTRY; use crate::hugr::NodeType; - use crate::ops::handle::{CfgID, DataflowParentID, DfgID, FuncID, ModuleRootID}; + use crate::ops::handle::{CfgID, DataflowParentID, DfgID, FuncID}; use crate::ops::{dataflow::IOTrait, Input, OpTag, Output}; use crate::type_row; use crate::types::{FunctionType, Type}; @@ -427,14 +432,6 @@ mod test { ); } - // But cannot create a view directly as a grandchild of another SiblingGraph - let root_view: SiblingGraph<'_, ModuleRootID> = - SiblingGraph::try_new(&h, h.root()).unwrap(); - assert_eq!( - SiblingGraph::<'_, DfgID>::try_new(&root_view, sub_dfg.node()).err(), - Some(HugrError::InvalidNode(sub_dfg.node())) - ); - Ok(()) } diff --git a/quantinuum-hugr/src/hugr/views/sibling_subgraph.rs b/quantinuum-hugr/src/hugr/views/sibling_subgraph.rs index 23cbfa812..9741b08c7 100644 --- a/quantinuum-hugr/src/hugr/views/sibling_subgraph.rs +++ b/quantinuum-hugr/src/hugr/views/sibling_subgraph.rs @@ -18,7 +18,7 @@ use portgraph::{view::Subgraph, Direction, PortView}; use thiserror::Error; use crate::builder::{Container, FunctionBuilder}; -use crate::hugr::{HugrError, HugrMut, HugrView, RootTagged}; +use crate::hugr::{HugrMut, HugrView, RootTagged}; use crate::ops::dataflow::DataflowOpTrait; use crate::ops::handle::{ContainerHandle, DataflowOpID}; use crate::ops::{OpTag, OpTrait}; @@ -405,31 +405,27 @@ impl SiblingSubgraph { /// /// The new Hugr will contain a [FuncDefn][crate::ops::FuncDefn] root /// with the same signature as the subgraph and the specified `name` - pub fn extract_subgraph( - &self, - hugr: &impl HugrView, - name: impl Into, - ) -> Result { + pub fn extract_subgraph(&self, hugr: &impl HugrView, name: impl Into) -> Hugr { let mut builder = FunctionBuilder::new(name, self.signature(hugr).into()).unwrap(); // Take the unfinished Hugr from the builder, to avoid unnecessary // validation checks that require connecting the inputs and outputs. let mut extracted = mem::take(builder.hugr_mut()); - let node_map = extracted.insert_subgraph(extracted.root(), hugr, self)?; + let node_map = extracted.insert_subgraph(extracted.root(), hugr, self); // Connect the inserted nodes in-between the input and output nodes. let [inp, out] = extracted.get_io(extracted.root()).unwrap(); for (inp_port, repl_ports) in extracted.node_outputs(inp).zip(self.inputs.iter()) { for (repl_node, repl_port) in repl_ports { - extracted.connect(inp, inp_port, node_map[repl_node], *repl_port)?; + extracted.connect(inp, inp_port, node_map[repl_node], *repl_port); } } for (out_port, (repl_node, repl_port)) in extracted.node_inputs(out).zip(self.outputs.iter()) { - extracted.connect(node_map[repl_node], *repl_port, out, out_port)?; + extracted.connect(node_map[repl_node], *repl_port, out, out_port); } - Ok(extracted) + extracted } } @@ -968,13 +964,12 @@ mod tests { #[test] fn extract_subgraph() -> Result<(), Box> { - let (hugr, func_root) = build_hugr().unwrap(); - let func_graph: SiblingGraph<'_, FuncID> = - SiblingGraph::try_new(&hugr, func_root).unwrap(); - let subgraph = SiblingSubgraph::try_new_dataflow_subgraph(&func_graph).unwrap(); - let extracted = subgraph.extract_subgraph(&hugr, "region")?; + let (hugr, func_root) = build_hugr()?; + let func_graph: SiblingGraph<'_, FuncID> = SiblingGraph::try_new(&hugr, func_root)?; + let subgraph = SiblingSubgraph::try_new_dataflow_subgraph(&func_graph)?; + let extracted = subgraph.extract_subgraph(&hugr, "region"); - extracted.validate(&PRELUDE_REGISTRY).unwrap(); + extracted.validate(&PRELUDE_REGISTRY)?; Ok(()) }