feat!: Remove RootTagged from the hugr view trait hierarchy

hugr-core/src/builder/dataflow.rs

@@ -174,9 +174,7 @@ impl FunctionBuilder<Hugr> {
 
         // Update the inner input node
         let types = new_optype.signature.body().input.clone();
-        self.hugr_mut()
-            .replace_op(inp_node, Input { types })
-            .unwrap();
+        self.hugr_mut().replace_op(inp_node, Input { types });
         let mut new_port = self.hugr_mut().add_ports(inp_node, Direction::Outgoing, 1);
         let new_port = new_port.next().unwrap();
 
@@ -211,9 +209,7 @@ impl FunctionBuilder<Hugr> {
 
         // Update the inner input node
         let types = new_optype.signature.body().output.clone();
-        self.hugr_mut()
-            .replace_op(out_node, Output { types })
-            .unwrap();
+        self.hugr_mut().replace_op(out_node, Output { types });
         let mut new_port = self.hugr_mut().add_ports(out_node, Direction::Incoming, 1);
         let new_port = new_port.next().unwrap();
 
@@ -250,15 +246,13 @@ impl FunctionBuilder<Hugr> {
             .expect("FunctionBuilder node must be a FuncDefn");
         let signature = old_optype.inner_signature().into_owned();
         let name = old_optype.name.clone();
-        self.hugr_mut()
-            .replace_op(
-                parent,
-                ops::FuncDefn {
-                    signature: f(signature).into(),
-                    name,
-                },
-            )
-            .expect("Could not replace FunctionBuilder operation");
+        self.hugr_mut().replace_op(
+            parent,
+            ops::FuncDefn {
+                signature: f(signature).into(),
+                name,
+            },
+        );
 
         self.hugr().get_optype(parent).as_func_defn().unwrap()
     }

hugr-core/src/builder/module.rs

@@ -83,8 +83,7 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
             .clone();
         let body = signature.body().clone();
         self.hugr_mut()
-            .replace_op(f_node, ops::FuncDefn { name, signature })
-            .expect("Replacing a FuncDecl node with a FuncDefn should always be valid");
+            .replace_op(f_node, ops::FuncDefn { name, signature });
 
         let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body)?;
         Ok(FunctionBuilder::from_dfg_builder(db))

hugr-core/src/hugr.rs

@@ -23,7 +23,7 @@ use portgraph::multiportgraph::MultiPortGraph;
 use portgraph::{Hierarchy, PortMut, PortView, UnmanagedDenseMap};
 use thiserror::Error;
 
-pub use self::views::{HugrView, RootTagged};
+pub use self::views::HugrView;
 use crate::core::NodeIndex;
 use crate::extension::resolution::{
     resolve_op_extensions, resolve_op_types_extensions, ExtensionResolutionError,
@@ -367,13 +367,10 @@ pub struct ExtensionError {
 }
 
 /// Errors that can occur while manipulating a Hugr.
-///
-/// TODO: Better descriptions, not just re-exporting portgraph errors.
 #[derive(Debug, Clone, PartialEq, Eq, Error)]
 #[non_exhaustive]
 pub enum HugrError {
     /// 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}")]
     #[allow(missing_docs)]
     InvalidTag { required: OpTag, actual: OpTag },
@@ -671,12 +668,12 @@ mod test {
                 signature: Signature::new_endo(ty).with_extension_delta(result.clone()),
             };
             let mut expected = backup;
-            expected.replace_op(p, expected_p).unwrap();
+            expected.replace_op(p, expected_p);
             let expected_gp = ops::Conditional {
                 extension_delta: result,
                 ..root_ty
             };
-            expected.replace_op(h.root(), expected_gp).unwrap();
+            expected.replace_op(h.root(), expected_gp);
 
             assert_eq!(h, expected);
         } else {

hugr-core/src/hugr/hugrmut.rs

@@ -156,14 +156,14 @@ pub trait HugrMut: HugrMutInternals {
     /// If the node is not in the graph, or if the port is invalid.
     fn add_other_edge(&mut self, src: Self::Node, dst: Self::Node) -> (OutgoingPort, IncomingPort);
 
-    /// Insert another hugr into this one, under a given root node.
+    /// Insert another hugr into this one, under a given parent node.
     ///
     /// # Panics
     ///
     /// If the root node is not in the graph.
     fn insert_hugr(&mut self, root: Self::Node, other: Hugr) -> InsertionResult<Node, Self::Node>;
 
-    /// Copy another hugr into this one, under a given root node.
+    /// Copy another hugr into this one, under a given parent node.
     ///
     /// # Panics
     ///
@@ -174,7 +174,7 @@ pub trait HugrMut: HugrMutInternals {
         other: &H,
     ) -> InsertionResult<H::Node, Self::Node>;
 
-    /// Copy a subgraph from another hugr into this one, under a given root node.
+    /// Copy a subgraph from another hugr into this one, under a given parent node.
     ///
     /// Sibling order is not preserved.
     ///

hugr-core/src/hugr/internal.rs

@@ -12,7 +12,7 @@ use crate::ops::handle::NodeHandle;
 use crate::{Direction, Hugr, Node};
 
 use super::hugrmut::{panic_invalid_node, panic_invalid_non_root};
-use super::{HugrError, NodeMetadataMap, OpType, RootTagged};
+use super::{HugrView, NodeMetadataMap, OpType};
 
 /// Trait for accessing the internals of a Hugr(View).
 ///
@@ -107,7 +107,7 @@ impl HugrInternals for Hugr {
 ///
 /// Specifically, this trait lets you apply arbitrary modifications that may
 /// invalidate the HUGR.
-pub trait HugrMutInternals: RootTagged {
+pub trait HugrMutInternals: HugrView {
     /// Set root node of the HUGR.
     ///
     /// This should be an existing node in the HUGR. Most operations use the
@@ -189,18 +189,10 @@ pub trait HugrMutInternals: RootTagged {
     ///
     /// Returns the old OpType.
     ///
-    /// If the module root is set to a non-module operation the hugr will
-    /// become invalid.
-    ///
-    /// # Errors
-    ///
-    /// Returns a [`HugrError::InvalidTag`] if this would break the bound
-    /// (`Self::RootHandle`) on the root node's OpTag.
-    ///
     /// # Panics
     ///
     /// If the node is not in the graph.
-    fn replace_op(&mut self, node: Self::Node, op: impl Into<OpType>) -> Result<OpType, HugrError>;
+    fn replace_op(&mut self, node: Self::Node, op: impl Into<OpType>) -> OpType;
 
     /// Gets a mutable reference to the optype.
     ///
@@ -223,9 +215,10 @@ pub trait HugrMutInternals: RootTagged {
     /// If the node is not in the graph.
     fn node_metadata_map_mut(&mut self, node: Self::Node) -> &mut NodeMetadataMap;
 
-    /// Returns a mutable reference to the extension registry for this hugr,
-    /// containing all extensions required to define the operations and types in
-    /// the hugr.
+    /// Returns a mutable reference to the extension registry for this HUGR.
+    ///
+    /// This set contains all extensions required to define the operations and
+    /// types in the HUGR.
     fn extensions_mut(&mut self) -> &mut ExtensionRegistry;
 }
 
@@ -326,10 +319,9 @@ impl HugrMutInternals for Hugr {
             .expect("Inserting a newly-created node into the hierarchy should never fail.");
     }
 
-    fn replace_op(&mut self, node: Node, op: impl Into<OpType>) -> Result<OpType, HugrError> {
+    fn replace_op(&mut self, node: Node, op: impl Into<OpType>) -> OpType {
         panic_invalid_node(self, node);
-        // We know RootHandle=Node here so no need to check
-        Ok(std::mem::replace(self.optype_mut(node), op.into()))
+        std::mem::replace(self.optype_mut(node), op.into())
     }
 
     fn optype_mut(&mut self, node: Self::Node) -> &mut OpType {

hugr-core/src/hugr/rewrite.rs

@@ -82,8 +82,7 @@ impl<R: Rewrite> Rewrite for Transactional<R> {
         let r = self.underlying.apply(h);
         if r.is_err() {
             // Try to restore backup.
-            h.replace_op(h.root(), backup.root_type().clone())
-                .expect("The root replacement should always match the old root type");
+            h.replace_op(h.root(), backup.root_type().clone());
             while let Some(child) = h.first_child(h.root()) {
                 h.remove_node(child);
             }

hugr-core/src/hugr/rewrite/inline_call.rs

@@ -76,7 +76,6 @@ impl Rewrite for InlineCall {
 
         let ty_args = h
             .replace_op(self.0, new_op)
-            .unwrap()
             .as_call()
             .unwrap()
             .type_args
@@ -117,8 +116,7 @@ mod test {
         ModuleBuilder,
     };
     use crate::extension::prelude::usize_t;
-    use crate::hugr::views::RootChecked;
-    use crate::ops::handle::{FuncID, ModuleRootID, NodeHandle};
+    use crate::ops::handle::{FuncID, NodeHandle};
     use crate::ops::{Input, OpType, Value};
     use crate::std_extensions::arithmetic::{
         int_ops::{self, IntOpDef},
@@ -179,10 +177,7 @@ mod test {
             .count(),
             1
         );
-        RootChecked::<_, ModuleRootID>::try_new(&mut hugr)
-            .unwrap()
-            .apply_rewrite(InlineCall(call1.node()))
-            .unwrap();
+        hugr.apply_rewrite(InlineCall(call1.node())).unwrap();
         hugr.validate().unwrap();
         assert_eq!(hugr.output_neighbours(func.node()).collect_vec(), [call2]);
         assert_eq!(calls(&hugr), [call2]);

hugr-core/src/hugr/rewrite/replace.rs

@@ -732,8 +732,7 @@ mod test {
         // Root node type needs to be that of common parent of the removed nodes:
         let mut rep2 = rep.clone();
         rep2.replacement
-            .replace_op(rep2.replacement.root(), h.root_type().clone())
-            .unwrap();
+            .replace_op(rep2.replacement.root(), h.root_type().clone());
         assert_eq!(
             check_same_errors(rep2),
             ReplaceError::WrongRootNodeTag {

hugr-core/src/hugr/validate/test.rs

@@ -191,26 +191,23 @@ fn df_children_restrictions() {
         .unwrap();
 
     // Replace the output operation of the df subgraph with a copy
-    b.replace_op(output, Noop(usize_t())).unwrap();
+    b.replace_op(output, Noop(usize_t()));
     assert_matches!(
         b.validate(),
         Err(ValidationError::InvalidInitialChild { parent, .. }) => assert_eq!(parent, def)
     );
 
     // Revert it back to an output, but with the wrong number of ports
-    b.replace_op(output, ops::Output::new(vec![bool_t()]))
-        .unwrap();
+    b.replace_op(output, ops::Output::new(vec![bool_t()]));
     assert_matches!(
         b.validate(),
         Err(ValidationError::InvalidChildren { parent, source: ChildrenValidationError::IOSignatureMismatch { child, .. }, .. })
             => {assert_eq!(parent, def); assert_eq!(child, output.pg_index())}
     );
-    b.replace_op(output, ops::Output::new(vec![bool_t(), bool_t()]))
-        .unwrap();
+    b.replace_op(output, ops::Output::new(vec![bool_t(), bool_t()]));
 
     // After fixing the output back, replace the copy with an output op
-    b.replace_op(copy, ops::Output::new(vec![bool_t(), bool_t()]))
-        .unwrap();
+    b.replace_op(copy, ops::Output::new(vec![bool_t(), bool_t()]));
     assert_matches!(
         b.validate(),
         Err(ValidationError::InvalidChildren { parent, source: ChildrenValidationError::InternalIOChildren { child, .. }, .. })
@@ -806,8 +803,7 @@ fn cfg_children_restrictions() {
         ops::CFG {
             signature: Signature::new(vec![bool_t()], vec![bool_t()]),
         },
-    )
-    .unwrap();
+    );
     assert_matches!(
         b.validate(),
         Err(ValidationError::ContainerWithoutChildren { .. })
@@ -869,8 +865,7 @@ fn cfg_children_restrictions() {
         ops::CFG {
             signature: Signature::new(vec![qb_t()], vec![bool_t()]),
         },
-    )
-    .unwrap();
+    );
     b.replace_op(
         block,
         ops::DataflowBlock {
@@ -879,18 +874,15 @@ fn cfg_children_restrictions() {
             other_outputs: vec![qb_t()].into(),
             extension_delta: ExtensionSet::new(),
         },
-    )
-    .unwrap();
+    );
     let mut block_children = b.hierarchy.children(block.pg_index());
     let block_input = block_children.next().unwrap().into();
     let block_output = block_children.next_back().unwrap().into();
-    b.replace_op(block_input, ops::Input::new(vec![qb_t()]))
-        .unwrap();
+    b.replace_op(block_input, ops::Input::new(vec![qb_t()]));
     b.replace_op(
         block_output,
         ops::Output::new(vec![Type::new_unit_sum(1), qb_t()]),
-    )
-    .unwrap();
+    );
     assert_matches!(
         b.validate(),
         Err(ValidationError::InvalidEdges { parent, source: EdgeValidationError::CFGEdgeSignatureMismatch { .. }, .. })

hugr-core/src/hugr/views.rs

@@ -16,7 +16,7 @@ use std::borrow::Cow;
 pub use self::petgraph::PetgraphWrapper;
 use self::render::RenderConfig;
 pub use descendants::DescendantsGraph;
-pub use root_checked::RootChecked;
+pub use root_checked::{check_tag, RootCheckable, RootChecked};
 pub use sibling::SiblingGraph;
 pub use sibling_subgraph::SiblingSubgraph;
 
@@ -29,7 +29,6 @@ use super::{
     Hugr, HugrError, HugrMut, Node, NodeMetadata, NodeMetadataMap, ValidationError, DEFAULT_OPTYPE,
 };
 use crate::extension::ExtensionRegistry;
-use crate::ops::handle::NodeHandle;
 use crate::ops::{OpParent, OpTag, OpTrait, OpType};
 
 use crate::types::{EdgeKind, PolyFuncType, Signature, Type};
@@ -479,17 +478,8 @@ pub trait HugrView: HugrInternals {
     }
 }
 
-/// Trait for views that provides a guaranteed bound on the type of the root node.
-pub trait RootTagged: HugrView {
-    /// The kind of handle that can be used to refer to the root node.
-    ///
-    /// The handle is guaranteed to be able to contain the operation returned by
-    /// [`HugrView::root_type`].
-    type RootHandle: NodeHandle<Self::Node>;
-}
-
 /// A common trait for views of a HUGR hierarchical subgraph.
-pub trait HierarchyView<'a>: RootTagged + Sized {
+pub trait HierarchyView<'a>: HugrView + Sized {
     /// Create a hierarchical view of a HUGR given a root node.
     ///
     /// # Errors
@@ -515,19 +505,6 @@ pub trait ExtractHugr: HugrView + Sized {
     }
 }
 
-/// Check that the node in a HUGR can be represented by the required tag.
-fn check_tag<Required: NodeHandle<N>, N>(
-    hugr: &impl HugrView<Node = N>,
-    node: N,
-) -> Result<(), HugrError> {
-    let actual = hugr.get_optype(node).tag();
-    let required = Required::TAG;
-    if !required.is_superset(actual) {
-        return Err(HugrError::InvalidTag { required, actual });
-    }
-    Ok(())
-}
-
 // Explicit implementation to avoid cloning the Hugr.
 impl ExtractHugr for Hugr {
     fn extract_hugr(self) -> Hugr {

hugr-core/src/hugr/views/descendants.rs

@@ -8,7 +8,7 @@ use crate::hugr::HugrError;
 use crate::ops::handle::NodeHandle;
 use crate::{Direction, Hugr, Node, Port};
 
-use super::{check_tag, ExtractHugr, HierarchyView, HugrInternals, HugrView, RootTagged};
+use super::{check_tag, ExtractHugr, HierarchyView, HugrInternals, HugrView};
 
 type RegionGraph<'g> = portgraph::view::Region<'g, &'g MultiPortGraph>;
 
@@ -131,16 +131,13 @@ impl<Root: NodeHandle> HugrView for DescendantsGraph<'_, Root> {
             .map(|index| self.get_node(index))
     }
 }
-impl<Root: NodeHandle> RootTagged for DescendantsGraph<'_, Root> {
-    type RootHandle = Root;
-}
 
 impl<'a, Root> HierarchyView<'a> for DescendantsGraph<'a, Root>
 where
     Root: NodeHandle,
 {
     fn try_new(hugr: &'a impl HugrView<Node = Node>, root: Node) -> Result<Self, HugrError> {
-        check_tag::<Root, Node>(hugr, root)?;
+        check_tag::<Root, _>(hugr, root)?;
         let hugr = hugr.base_hugr();
         Ok(Self {
             root,