feat!: No nested FuncDefns (or AliasDefns)

hugr-core/src/builder.rs

@@ -261,8 +261,8 @@ pub(crate) mod test {
 
     use super::handle::BuildHandle;
     use super::{
-        BuildError, CFGBuilder, Container, DFGBuilder, Dataflow, DataflowHugr, FuncID,
-        FunctionBuilder, ModuleBuilder,
+        BuildError, CFGBuilder, DFGBuilder, Dataflow, DataflowHugr, FuncID, FunctionBuilder,
+        ModuleBuilder,
     };
     use super::{DataflowSubContainer, HugrBuilder};
 

hugr-core/src/builder/build_traits.rs

@@ -9,7 +9,7 @@ use crate::{Extension, IncomingPort, Node, OutgoingPort};
 use std::iter;
 use std::sync::Arc;
 
-use super::{BuilderWiringError, FunctionBuilder};
+use super::{BuilderWiringError, ModuleBuilder};
 use super::{
     CircuitBuilder,
     handle::{BuildHandle, Outputs},
@@ -21,7 +21,7 @@ use crate::{
 };
 
 use crate::extension::ExtensionRegistry;
-use crate::types::{PolyFuncType, Signature, Type, TypeArg, TypeRow};
+use crate::types::{Signature, Type, TypeArg, TypeRow};
 
 use itertools::Itertools;
 
@@ -82,33 +82,6 @@ pub trait Container {
         self.add_child_node(constant.into()).into()
     }
 
-    /// Add a [`ops::FuncDefn`] node and returns a builder to define the function
-    /// body graph.
-    ///
-    /// # Errors
-    ///
-    /// This function will return an error if there is an error in adding the
-    /// [`ops::FuncDefn`] node.
-    fn define_function(
-        &mut self,
-        name: impl Into<String>,
-        signature: impl Into<PolyFuncType>,
-    ) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
-        let signature: PolyFuncType = signature.into();
-        let body = signature.body().clone();
-        let f_node = self.add_child_node(ops::FuncDefn::new(name, signature));
-
-        // Add the extensions used by the function types.
-        self.use_extensions(
-            body.used_extensions().unwrap_or_else(|e| {
-                panic!("Build-time signatures should have valid extensions. {e}")
-            }),
-        );
-
-        let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body)?;
-        Ok(FunctionBuilder::from_dfg_builder(db))
-    }
-
     /// Insert a HUGR as a child of the container.
     fn add_hugr(&mut self, child: Hugr) -> InsertionResult {
         let parent = self.container_node();
@@ -155,8 +128,19 @@ pub trait Container {
 }
 
 /// Types implementing this trait can be used to build complete HUGRs
-/// (with varying root node types)
+/// (with varying entrypoint node types)
 pub trait HugrBuilder: Container {
+    /// Allows adding definitions to the module root of which
+    /// this builder is building a part
+    fn module_root_builder(&mut self) -> ModuleBuilder<&mut Hugr> {
+        debug_assert!(
+            self.hugr()
+                .get_optype(self.hugr().module_root())
+                .is_module()
+        );
+        ModuleBuilder(self.hugr_mut())
+    }
+
     /// Finish building the HUGR, perform any validation checks and return it.
     fn finish_hugr(self) -> Result<Hugr, ValidationError<Node>>;
 }

hugr-core/src/builder/circuit.rs

@@ -245,7 +245,7 @@ mod test {
     use cool_asserts::assert_matches;
 
     use crate::Extension;
-    use crate::builder::{Container, HugrBuilder, ModuleBuilder};
+    use crate::builder::{HugrBuilder, ModuleBuilder};
     use crate::extension::ExtensionId;
     use crate::extension::prelude::{qb_t, usize_t};
     use crate::std_extensions::arithmetic::float_types::ConstF64;

hugr-core/src/builder/module.rs

@@ -16,13 +16,13 @@ use crate::{Hugr, Node};
 use smol_str::SmolStr;
 
 /// Builder for a HUGR module.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Default, Clone, PartialEq)]
 pub struct ModuleBuilder<T>(pub(super) T);
 
 impl<T: AsMut<Hugr> + AsRef<Hugr>> Container for ModuleBuilder<T> {
     #[inline]
     fn container_node(&self) -> Node {
-        self.0.as_ref().entrypoint()
+        self.0.as_ref().module_root()
     }
 
     #[inline]
@@ -39,13 +39,7 @@ impl ModuleBuilder<Hugr> {
     /// Begin building a new module.
     #[must_use]
     pub fn new() -> Self {
-        Self(Default::default())
-    }
-}
-
-impl Default for ModuleBuilder<Hugr> {
-    fn default() -> Self {
-        Self::new()
+        Self::default()
     }
 }
 
@@ -112,6 +106,33 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
         Ok(declare_n.into())
     }
 
+    /// Add a [`ops::FuncDefn`] node and returns a builder to define the function
+    /// body graph.
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if there is an error in adding the
+    /// [`ops::FuncDefn`] node.
+    pub fn define_function(
+        &mut self,
+        name: impl Into<String>,
+        signature: impl Into<PolyFuncType>,
+    ) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
+        let signature: PolyFuncType = signature.into();
+        let body = signature.body().clone();
+        let f_node = self.add_child_node(ops::FuncDefn::new(name, signature));
+
+        // Add the extensions used by the function types.
+        self.use_extensions(
+            body.used_extensions().unwrap_or_else(|e| {
+                panic!("Build-time signatures should have valid extensions. {e}")
+            }),
+        );
+
+        let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body)?;
+        Ok(FunctionBuilder::from_dfg_builder(db))
+    }
+
     /// Add a [`crate::ops::OpType::AliasDefn`] node and return a handle to the Alias.
     ///
     /// # Errors
@@ -208,29 +229,4 @@ mod test {
         assert_matches!(build_result, Ok(_));
         Ok(())
     }
-
-    #[test]
-    fn local_def() -> Result<(), BuildError> {
-        let build_result = {
-            let mut module_builder = ModuleBuilder::new();
-
-            let mut f_build = module_builder.define_function(
-                "main",
-                Signature::new(vec![usize_t()], vec![usize_t(), usize_t()]),
-            )?;
-            let local_build = f_build.define_function(
-                "local",
-                Signature::new(vec![usize_t()], vec![usize_t(), usize_t()]),
-            )?;
-            let [wire] = local_build.input_wires_arr();
-            let f_id = local_build.finish_with_outputs([wire, wire])?;
-
-            let call = f_build.call(f_id.handle(), &[], f_build.input_wires())?;
-
-            f_build.finish_with_outputs(call.outputs())?;
-            module_builder.finish_hugr()
-        };
-        assert_matches!(build_result, Ok(_));
-        Ok(())
-    }
 }

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

@@ -291,29 +291,32 @@ mod test {
     fn test_polymorphic() -> Result<(), Box<dyn std::error::Error>> {
         let tuple_ty = Type::new_tuple(vec![usize_t(); 2]);
         let mut fb = FunctionBuilder::new("mkpair", Signature::new(usize_t(), tuple_ty.clone()))?;
-        let inner = fb.define_function(
-            "id",
-            PolyFuncType::new(
-                [TypeBound::Copyable.into()],
-                Signature::new_endo(Type::new_var_use(0, TypeBound::Copyable)),
-            ),
-        )?;
-        let inps = inner.input_wires();
-        let inner = inner.finish_with_outputs(inps)?;
-        let call1 = fb.call(inner.handle(), &[usize_t().into()], fb.input_wires())?;
+        let helper = {
+            let mut mb = fb.module_root_builder();
+            let fb2 = mb.define_function(
+                "id",
+                PolyFuncType::new(
+                    [TypeBound::Copyable.into()],
+                    Signature::new_endo(Type::new_var_use(0, TypeBound::Copyable)),
+                ),
+            )?;
+            let inps = fb2.input_wires();
+            fb2.finish_with_outputs(inps)?
+        };
+        let call1 = fb.call(helper.handle(), &[usize_t().into()], fb.input_wires())?;
         let [call1_out] = call1.outputs_arr();
         let tup = fb.make_tuple([call1_out, call1_out])?;
-        let call2 = fb.call(inner.handle(), &[tuple_ty.into()], [tup])?;
+        let call2 = fb.call(helper.handle(), &[tuple_ty.into()], [tup])?;
         let mut hugr = fb.finish_hugr_with_outputs(call2.outputs()).unwrap();
 
         assert_eq!(
-            hugr.output_neighbours(inner.node()).collect::<Vec<_>>(),
+            hugr.output_neighbours(helper.node()).collect::<Vec<_>>(),
             [call1.node(), call2.node()]
         );
         hugr.apply_patch(InlineCall::new(call1.node()))?;
 
         assert_eq!(
-            hugr.output_neighbours(inner.node()).collect::<Vec<_>>(),
+            hugr.output_neighbours(helper.node()).collect::<Vec<_>>(),
             [call2.node()]
         );
         assert!(hugr.get_optype(call1.node()).is_dfg());

hugr-core/src/hugr/patch/simple_replace.rs

@@ -641,8 +641,8 @@ pub(in crate::hugr::patch) mod test {
 
     use crate::builder::test::n_identity;
     use crate::builder::{
-        BuildError, Container, DFGBuilder, Dataflow, DataflowHugr, DataflowSubContainer,
-        HugrBuilder, ModuleBuilder, endo_sig, inout_sig,
+        BuildError, DFGBuilder, Dataflow, DataflowHugr, DataflowSubContainer, HugrBuilder,
+        ModuleBuilder, endo_sig, inout_sig,
     };
     use crate::extension::prelude::{bool_t, qb_t};
     use crate::hugr::patch::simple_replace::Outcome;

hugr-core/src/hugr/validate.rs

@@ -443,28 +443,12 @@ impl<'a, H: HugrView> ValidationContext<'a, H> {
         //
         // This search could be sped-up with a pre-computed LCA structure, but
         // for valid Hugrs this search should be very short.
-        //
-        // For Value edges only, we record any FuncDefn we went through; if there is
-        // any such, then that is an error, but we report that only if the dom/ext
-        // relation was otherwise ok (an error about an edge "entering" some ancestor
-        // node could be misleading if the source isn't where it's expected)
-        let mut err_entered_func = None;
         let from_parent_parent = self.hugr.get_parent(from_parent);
         for (ancestor, ancestor_parent) in
             iter::successors(to_parent, |&p| self.hugr.get_parent(p)).tuple_windows()
         {
-            if !is_static && self.hugr.get_optype(ancestor).is_func_defn() {
-                err_entered_func.get_or_insert(InterGraphEdgeError::ValueEdgeIntoFunc {
-                    to,
-                    to_offset,
-                    from,
-                    from_offset,
-                    func: ancestor,
-                });
-            }
             if ancestor_parent == from_parent {
                 // External edge.
-                err_entered_func.map_or(Ok(()), Err)?;
                 if !is_static {
                     // Must have an order edge.
                     self.hugr
@@ -491,7 +475,7 @@ impl<'a, H: HugrView> ValidationContext<'a, H> {
                         ancestor_parent_op: ancestor_parent_op.clone(),
                     });
                 }
-                err_entered_func.map_or(Ok(()), Err)?;
+
                 // Check domination
                 let (dominator_tree, node_map) =
                     if let Some(tree) = self.dominators.get(&ancestor_parent) {
@@ -758,17 +742,6 @@ pub enum InterGraphEdgeError<N: HugrNode> {
         to_offset: Port,
         ty: EdgeKind,
     },
-    /// Inter-Graph edges may not enter into `FuncDefns` unless they are static
-    #[error(
-        "Inter-graph Value edges cannot enter into FuncDefns. Inter-graph edge from {from} ({from_offset}) to {to} ({to_offset} enters FuncDefn {func}"
-    )]
-    ValueEdgeIntoFunc {
-        from: N,
-        from_offset: Port,
-        to: N,
-        to_offset: Port,
-        func: N,
-    },
     /// The grandparent of a dominator inter-graph edge must be a CFG container.
     #[error(
         "The grandparent of a dominator inter-graph edge must be a CFG container. Found operation {ancestor_parent_op}. In a dominator inter-graph edge from {from} ({from_offset}) to {to} ({to_offset})."