HugrMut: validate nodes for set_metadata/get_metadata_mut, too (#531)

...bringing these in line with the other methods. Yes, this involves a
`Result<&mut, ...>` but that works ok!

Also inline `HugrMutInternals::add_node` and `add_op`, these are only
used once and it's fairly trivial (but against, restores uniformity)

src/builder/build_traits.rs

@@ -118,12 +118,16 @@ pub trait Container {
  /// Add metadata to the container node.
  fn set_metadata(&mut self, meta: NodeMetadata) {
  let parent = self.container_node();
- self.hugr_mut().set_metadata(parent, meta);
+ // Implementor's container_node() should be a valid node
+ self.hugr_mut().set_metadata(parent, meta).unwrap();
  }
 
  /// Add metadata to a child node.
- fn set_child_metadata(&mut self, child: Node, meta: NodeMetadata) {
- self.hugr_mut().set_metadata(child, meta);
+ ///
+ /// Returns an error if the specified `child` is not a child of this container
+ fn set_child_metadata(&mut self, child: Node, meta: NodeMetadata) -> Result<(), BuildError> {
+ self.hugr_mut().set_metadata(child, meta)?;
+ Ok(())
  }
 }
 

src/hugr/hugrmut.rs

@@ -17,11 +17,15 @@ use super::{NodeMetadata, Rewrite};
 /// Functions for low-level building of a HUGR.
 pub trait HugrMut: HugrView + HugrMutInternals {
  /// Returns the metadata associated with a node.
- fn get_metadata_mut(&mut self, node: Node) -> &mut NodeMetadata;
+ fn get_metadata_mut(&mut self, node: Node) -> Result<&mut NodeMetadata, HugrError> {
+ self.valid_node(node)?;
+ Ok(self.hugr_mut().metadata.get_mut(node.index))
+ }
 
  /// Sets the metadata associated with a node.
- fn set_metadata(&mut self, node: Node, metadata: NodeMetadata) {
- *self.get_metadata_mut(node) = metadata;
+ fn set_metadata(&mut self, node: Node, metadata: NodeMetadata) -> Result<(), HugrError> {
+ *self.get_metadata_mut(node)? = metadata;
+ Ok(())
  }
 
  /// Add a node to the graph with a parent in the hierarchy.
@@ -159,10 +163,6 @@ impl<T> HugrMut for T
 where
  T: HugrView + AsMut<Hugr>,
 {
- fn get_metadata_mut(&mut self, node: Node) -> &mut NodeMetadata {
- self.as_mut().metadata.get_mut(node.index)
- }
-
  fn add_op_with_parent(
  &mut self,
  parent: Node,
@@ -173,23 +173,23 @@ where
  }
 
  fn add_node_with_parent(&mut self, parent: Node, node: NodeType) -> Result<Node, HugrError> {
- let node = self.add_node(node);
+ let node = self.as_mut().add_node(node);
  self.as_mut()
  .hierarchy
  .push_child(node.index, parent.index)?;
  Ok(node)
  }
 
  fn add_op_before(&mut self, sibling: Node, op: impl Into<OpType>) -> Result<Node, HugrError> {
- let node = self.add_op(op);
+ let node = self.as_mut().add_op(op);
  self.as_mut()
  .hierarchy
  .insert_before(node.index, sibling.index)?;
  Ok(node)
  }
 
  fn add_op_after(&mut self, sibling: Node, op: impl Into<OpType>) -> Result<Node, HugrError> {
- let node = self.add_op(op);
+ let node = self.as_mut().add_op(op);
  self.as_mut()
  .hierarchy
  .insert_after(node.index, sibling.index)?;
@@ -252,7 +252,7 @@ where
  let optype = other.op_types.take(node);
  self.as_mut().op_types.set(new_node, optype);
  let meta = other.metadata.take(node);
- self.as_mut().set_metadata(node.into(), meta);
+ self.as_mut().set_metadata(node.into(), meta).unwrap();
  }
  Ok(other_root)
  }
@@ -264,7 +264,9 @@ where
  let nodetype = other.get_nodetype(node.into());
  self.as_mut().op_types.set(new_node, nodetype.clone());
  let meta = other.get_metadata(node.into());
- self.as_mut().set_metadata(node.into(), meta.clone());
+ self.as_mut()
+ .set_metadata(node.into(), meta.clone())
+ .unwrap();
  }
  Ok(other_root)
  }
@@ -344,16 +346,6 @@ pub(crate) mod sealed {
  }
  }
 
- /// Add a node to the graph, with the default conversion from OpType to NodeType
- fn add_op(&mut self, op: impl Into<OpType>) -> Node {
- self.hugr_mut().add_op(op)
- }
-
- /// Add a node to the graph.
- fn add_node(&mut self, nodetype: NodeType) -> Node {
- self.hugr_mut().add_node(nodetype)
- }
-
  /// Set the number of ports on a node. This may invalidate the node's `PortIndex`.
  fn set_num_ports(&mut self, node: Node, incoming: usize, outgoing: usize) {
  self.valid_node(node).unwrap_or_else(|e| panic!("{}", e));

src/hugr/rewrite/simple_replace.rs

@@ -97,7 +97,7 @@ impl Rewrite for SimpleReplacement {
 
  // Move the metadata
  let meta: &NodeMetadata = self.replacement.get_metadata(node);
- h.set_metadata(node, meta.clone());
+ h.set_metadata(node, meta.clone()).unwrap();
  }
  // Add edges between all newly added nodes matching those in replacement.
  // TODO This will probably change when implicit copies are implemented.

src/hugr/serialize.rs

@@ -227,7 +227,7 @@ impl TryFrom<SerHugrV0> for Hugr {
 
  for (node, metadata) in metadata.into_iter().enumerate() {
  let node = NodeIndex::new(node).into();
- hugr.set_metadata(node, metadata);
+ hugr.set_metadata(node, metadata)?;
  }
 
  let unwrap_offset = |node: Node, offset, dir, hugr: &Hugr| -> Result<usize, Self::Error> {