Make HugrMut a trait

src/algorithm/nest_cfgs.rs

@@ -399,7 +399,7 @@ impl<T: Copy + Clone + PartialEq + Eq + Hash> EdgeClassifier<T> {
 pub(crate) mod test {
     use super::*;
     use crate::builder::{
-        BuildError, CFGBuilder, Container, Dataflow, DataflowSubContainer, HugrBuilder, HugrMutRef,
+        BuildError, CFGBuilder, Container, Dataflow, DataflowSubContainer, HugrBuilder,
         ModuleBuilder, SubContainer,
     };
     use crate::ops::{
@@ -611,7 +611,7 @@ pub(crate) mod test {
         dataflow_builder.finish_with_outputs([u].into_iter().chain(w))
     }
 
-    fn build_if_then_else_merge<T: HugrMutRef>(
+    fn build_if_then_else_merge<T: AsMut<Hugr> + AsRef<Hugr>>(
         cfg: &mut CFGBuilder<T>,
         const_pred: &ConstID,
         unit_const: &ConstID,
@@ -624,7 +624,7 @@ pub(crate) mod test {
         Ok((split, merge))
     }
 
-    fn build_then_else_merge_from_if<T: HugrMutRef>(
+    fn build_then_else_merge_from_if<T: AsMut<Hugr> + AsRef<Hugr>>(
         cfg: &mut CFGBuilder<T>,
         unit_const: &ConstID,
         split: BasicBlockID,
@@ -649,7 +649,7 @@ pub(crate) mod test {
     }
 
     // Returns loop tail - caller must link header to tail, and provide 0th successor of tail
-    fn build_loop_from_header<T: HugrMutRef>(
+    fn build_loop_from_header<T: AsMut<Hugr> + AsRef<Hugr>>(
         cfg: &mut CFGBuilder<T>,
         const_pred: &ConstID,
         header: BasicBlockID,
@@ -663,7 +663,7 @@ pub(crate) mod test {
     }
 
     // Result is header and tail. Caller must provide 0th successor of header (linking to tail), and 0th successor of tail.
-    fn build_loop<T: HugrMutRef>(
+    fn build_loop<T: AsMut<Hugr> + AsRef<Hugr>>(
         cfg: &mut CFGBuilder<T>,
         const_pred: &ConstID,
         unit_const: &ConstID,

src/builder.rs

@@ -2,7 +2,7 @@
 //!
 use thiserror::Error;
 
-use crate::hugr::{Hugr, HugrError, HugrMut, Node, ValidationError, Wire};
+use crate::hugr::{HugrError, Node, ValidationError, Wire};
 use crate::ops::handle::{BasicBlockID, CfgID, ConditionalID, DfgID, FuncID, TailLoopID};
 
 use crate::types::LinearType;
@@ -70,11 +70,6 @@ pub enum BuildError {
     CircuitError(#[from] circuit_builder::CircuitBuildError),
 }
 
-/// Trait allowing treating type as (im)mutable reference to [`Hugr`]
-pub trait HugrMutRef: AsMut<Hugr> + AsRef<Hugr> {}
-impl<H: HugrMut> HugrMutRef for H {}
-impl HugrMutRef for &mut Hugr {}
-
 #[cfg(test)]
 mod test {
     use crate::types::{ClassicType, LinearType, Signature, SimpleType};

src/builder/build_traits.rs

@@ -35,22 +35,22 @@ use crate::hugr::HugrMut;
 pub trait Container {
     /// The container node.
     fn container_node(&self) -> Node;
-    /// The underlying [`Hugr`] being built...TODO: should we just require `AsMut<Hugr>`?
-    fn base(&mut self) -> &mut Hugr;
-    /// Immutable reference to HUGR being built...TODO: should we just require `AsRef<Hugr>`? Or combine with previous?
+    /// The underlying [`Hugr`] being built
+    fn hugr_mut(&mut self) -> &mut Hugr;
+    /// Immutable reference to HUGR being built
     fn hugr(&self) -> &Hugr;
     /// Add an [`OpType`] as the final child of the container.
     fn add_child_op(&mut self, op: impl Into<OpType>) -> Result<Node, BuildError> {
         let parent = self.container_node();
-        Ok(self.base().add_op_with_parent(parent, op)?)
+        Ok(self.hugr_mut().add_op_with_parent(parent, op)?)
     }
 
     /// Adds a non-dataflow edge between two nodes. The kind is given by the operation's [`other_inputs`] or  [`other_outputs`]
     ///
     /// [`other_inputs`]: crate::ops::OpTrait::other_inputs
     /// [`other_outputs`]: crate::ops::OpTrait::other_outputs
     fn add_other_wire(&mut self, src: Node, dst: Node) -> Result<Wire, BuildError> {
-        let (src_port, _) = self.base().add_other_edge(src, dst)?;
+        let (src_port, _) = self.hugr_mut().add_other_edge(src, dst)?;
         Ok(Wire::new(src, Port::new_outgoing(src_port)))
     }
 
@@ -166,7 +166,7 @@ pub trait Dataflow: Container {
             input_wires,
         )?;
 
-        DFGBuilder::create_with_io(self.base(), dfg_n, input_types.into(), output_types)
+        DFGBuilder::create_with_io(self.hugr_mut(), dfg_n, input_types.into(), output_types)
     }
 
     /// Return a builder for a [`crate::ops::CFG`] node,
@@ -196,7 +196,7 @@ pub trait Dataflow: Container {
             },
             input_wires,
         )?;
-        CFGBuilder::create(self.base(), cfg_node, inputs, output_types)
+        CFGBuilder::create(self.hugr_mut(), cfg_node, inputs, output_types)
     }
 
     /// Load a static constant and return the local dataflow wire for that constant.
@@ -208,7 +208,7 @@ pub trait Dataflow: Container {
         let cn = cid.node();
         let c_out = self.hugr().num_outputs(cn);
 
-        self.base().add_ports(cn, Direction::Outgoing, 1);
+        self.hugr_mut().add_ports(cn, Direction::Outgoing, 1);
 
         let load_n = self.add_dataflow_op(
             ops::LoadConstant {
@@ -271,7 +271,7 @@ pub trait Dataflow: Container {
         };
         let (loop_node, _) = add_op_with_wires(self, tail_loop.clone(), input_wires)?;
 
-        TailLoopBuilder::create_with_io(self.base(), loop_node, &tail_loop)
+        TailLoopBuilder::create_with_io(self.hugr_mut(), loop_node, &tail_loop)
     }
 
     /// Return a builder for a [`crate::ops::Conditional`] node.
@@ -312,7 +312,7 @@ pub trait Dataflow: Container {
         )?;
 
         Ok(ConditionalBuilder {
-            base: self.base(),
+            base: self.hugr_mut(),
             conditional_node: conditional_id.node(),
             n_out_wires,
             case_nodes: vec![None; n_cases],
@@ -450,11 +450,11 @@ pub trait Dataflow: Container {
         let const_in_port = signature.output.len();
         let op_id = self.add_dataflow_op(ops::Call { signature }, input_wires)?;
         let src_port: usize = self
-            .base()
+            .hugr_mut()
             .add_ports(function.node(), Direction::Outgoing, 1)
             .collect_vec()[0];
 
-        self.base()
+        self.hugr_mut()
             .connect(function.node(), src_port, op_id.node(), const_in_port)?;
         Ok(op_id)
     }
@@ -475,7 +475,7 @@ fn add_op_with_wires<T: Dataflow + ?Sized>(
 
     let op: OpType = op.into();
     let sig = op.signature();
-    let op_node = data_builder.base().add_op_before(out, op)?;
+    let op_node = data_builder.hugr_mut().add_op_before(out, op)?;
 
     wire_up_inputs(inputs, op_node, data_builder, inp)?;
 
@@ -498,7 +498,7 @@ fn wire_up_inputs<T: Dataflow + ?Sized>(
             dst_port,
         )?;
     }
-    let base = data_builder.base();
+    let base = data_builder.hugr_mut();
     let op = base.get_optype(op_node);
     let some_df_outputs = !op.signature().output.is_empty();
     if !any_local_df_inputs && some_df_outputs {
@@ -517,76 +517,50 @@ fn wire_up<T: Dataflow + ?Sized>(
     dst: Node,
     dst_port: usize,
 ) -> Result<bool, BuildError> {
-    let base = data_builder.base();
+    let base = data_builder.hugr_mut();
     let src_offset = Port::new_outgoing(src_port);
 
     let src_parent = base.get_parent(src);
     let dst_parent = base.get_parent(dst);
     let local_source = src_parent == dst_parent;
-    if !local_source {
-        if let Some(copy_port) = if_copy_add_port(base, src) {
-            src_port = copy_port;
-        } else if let Some(typ) = check_classical_value(base, src, src_offset)? {
-            let src_parent = base.get_parent(src).expect("Node has no parent");
-
-            let final_child = base
-                .children(src_parent)
-                .next_back()
-                .expect("Parent must have at least one child.");
-            let copy_node = base.add_op_before(final_child, LeafOp::Copy { n_copies: 1, typ })?;
+    // Non-local value sources require a state edge to an ancestor of dst
+    if !local_source && get_value_kind(base, src, src_offset) == ValueKind::Classic {
+        let src_parent = src_parent.expect("Node has no parent");
+        let Some(src_sibling) =
+                iter::successors(dst_parent, |&p| base.get_parent(p))
+                    .tuple_windows()
+                    .find_map(|(ancestor, ancestor_parent)| {
+                        (ancestor_parent == src_parent).then_some(ancestor)
+                    })
+            else {
+                let val_err: ValidationError = InterGraphEdgeError::NoRelation {
+                    from: src,
+                    from_offset: Port::new_outgoing(src_port),
+                    to: dst,
+                    to_offset: Port::new_incoming(dst_port),
+                }.into();
+                return Err(val_err.into());
+            };
 
         // TODO: Avoid adding duplicate edges
         // This should be easy with https://github.com/CQCL-DEV/hugr/issues/130
         base.add_other_edge(src, src_sibling)?;
     }
 
-            // Copy node has to have state edge to an ancestor of dst
-            let Some(src_sibling) = iter::successors(dst_parent, |&p| base.get_parent(p))
-                .tuple_windows()
-                .find_map(|(ancestor, ancestor_parent)| {
-                    (ancestor_parent == src_parent).then_some(ancestor)
-                }) else {
-                    let val_err: ValidationError = InterGraphEdgeError::NoRelation {
-                        from: src,
-                        from_offset: Port::new_outgoing(src_port),
-                        to: dst,
-                        to_offset: Port::new_incoming(dst_port),
-                    }.into();
-                    return Err(val_err.into());
-                };
-
-            base.add_other_edge(copy_node, src_sibling)?;
-
-            src = copy_node;
-            src_port = 0;
+    // Don't copy linear edges.
+    if base.linked_ports(src, src_offset).next().is_some() {
+        if let ValueKind::Linear(typ) = get_value_kind(base, src, src_offset) {
+            return Err(BuildError::NoCopyLinear(typ));
         }
     }
 
-    if let Some((connected, connected_offset)) = base.linked_port(src, src_offset) {
-        if let Some(copy_port) = if_copy_add_port(base, src) {
-            src_port = copy_port;
-            src = connected;
-        }
-        // Need to insert a copy - first check can be copied
-        else if let Some(typ) = check_classical_value(base, src, src_offset)? {
-            base.disconnect(src, Port::new_outgoing(src_port))?;
-
-            let copy = data_builder.add_dataflow_op(
-                LeafOp::Copy { n_copies: 2, typ },
-                [Wire::new(src, Port::new_outgoing(src_port))],
-            )?;
-
-            let base = data_builder.base();
-            base.connect(copy.node(), 0, connected, connected_offset.index())?;
-            src = copy.node();
-            src_port = 1;
-        }
-    }
-    data_builder.base().connect(src, src_port, dst, dst_port)?;
+    data_builder
+        .hugr_mut()
+        .connect(src, src_port, dst, dst_port)?;
     Ok(local_source
         && matches!(
             data_builder
-                .base()
+                .hugr_mut()
                 .get_optype(dst)
                 .port_kind(Port::new_incoming(dst_port))
                 .unwrap(),
@@ -605,42 +579,17 @@ enum ValueKind {
 /// Check the kind of a port is a classical Value and return it
 /// Return None if Const kind
 /// Panics if port not valid for Op or port is not Const/Value
-fn check_classical_value(
-    base: &impl AsRef<Hugr>,
-    src: Node,
-    src_offset: Port,
-) -> Result<Option<ClassicType>, BuildError> {
-    let wire_kind = base.as_ref().get_optype(src).port_kind(src_offset).unwrap();
-    let typ = match wire_kind {
-        EdgeKind::Const(_) => None,
+fn get_value_kind(base: &Hugr, src: Node, src_offset: Port) -> ValueKind {
+    let wire_kind = base.get_optype(src).port_kind(src_offset).unwrap();
+    match wire_kind {
+        EdgeKind::Const(_) => ValueKind::Const,
         EdgeKind::Value(simple_type) => match simple_type {
             SimpleType::Classic(_) => ValueKind::Classic,
             SimpleType::Linear(typ) => ValueKind::Linear(typ),
         },
         _ => {
             panic!("Wires can only be Const or Value kind")
         }
-    };
-
-    Ok(typ)
-}
-
-// Return newly added port to copy node if src node is a copy
-fn if_copy_add_port(base: &mut impl HugrMut, src: Node) -> Option<usize> {
-    let src_op: Result<&LeafOp, ()> = base.as_ref().get_optype(src).try_into();
-    if let Ok(LeafOp::Copy { n_copies, typ }) = src_op {
-        let copy_node = src;
-        // If already connected to a copy node, add wire to the copy
-        base.replace_op(
-            copy_node,
-            LeafOp::Copy {
-                n_copies: n_copies + 1,
-                typ: typ.clone(),
-            },
-        );
-        base.add_ports(copy_node, Direction::Outgoing, 1).next()
-    } else {
-        None
     }
 }