diff --git a/hugr/src/hugr/validate.rs b/hugr/src/hugr/validate.rs index 40de5d805..5f7ea0511 100644 --- a/hugr/src/hugr/validate.rs +++ b/hugr/src/hugr/validate.rs @@ -475,12 +475,28 @@ impl<'a, 'b> ValidationContext<'a, 'b> { // // 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 @@ -511,7 +527,7 @@ impl<'a, 'b> ValidationContext<'a, 'b> { ancestor_parent_op: ancestor_parent_op.clone(), }); } - + err_entered_func.map_or(Ok(()), Err)?; // Check domination let dominator_tree = match self.dominators.get(&ancestor_parent) { Some(tree) => tree, @@ -755,6 +771,15 @@ pub enum InterGraphEdgeError { 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: Node, + from_offset: Port, + to: Node, + to_offset: Port, + func: Node, + }, /// 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:?}).")] NonCFGAncestor { diff --git a/hugr/src/hugr/validate/test.rs b/hugr/src/hugr/validate/test.rs index a1518fcb2..556c5a092 100644 --- a/hugr/src/hugr/validate/test.rs +++ b/hugr/src/hugr/validate/test.rs @@ -257,6 +257,39 @@ fn test_ext_edge() { h.update_validate(&EMPTY_REG).unwrap(); } +#[test] +fn no_ext_edge_into_func() -> Result<(), Box> { + let b2b = FunctionType::new_endo(BOOL_T); + let mut h = DFGBuilder::new(FunctionType::new(BOOL_T, Type::new_function(b2b.clone())))?; + let [input] = h.input_wires_arr(); + + let mut dfg = h.dfg_builder( + FunctionType::new(vec![], Type::new_function(b2b.clone())), + None, + [], + )?; + let mut func = dfg.define_function("AndWithOuter", b2b.clone().into())?; + let [fn_input] = func.input_wires_arr(); + let and_op = func.add_dataflow_op(and_op(), [fn_input, input])?; // 'ext' edge + let func = func.finish_with_outputs(and_op.outputs())?; + let loadfn = dfg.load_func(func.handle(), &[], &EMPTY_REG)?; + let dfg = dfg.finish_with_outputs([loadfn])?; + let res = h.finish_hugr_with_outputs(dfg.outputs(), &EMPTY_REG); + assert_eq!( + res, + Err(BuildError::InvalidHUGR( + ValidationError::InterGraphEdgeError(InterGraphEdgeError::ValueEdgeIntoFunc { + from: input.node(), + from_offset: input.source().into(), + to: and_op.node(), + to_offset: IncomingPort::from(1).into(), + func: func.node() + }) + )) + ); + Ok(()) +} + #[test] fn test_local_const() { let mut h = closed_dfg_root_hugr(FunctionType::new(type_row![BOOL_T], type_row![BOOL_T])); diff --git a/specification/hugr.md b/specification/hugr.md index d4ebc9309..5f07fb276 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -554,8 +554,11 @@ parent(n2) when the edge's locality is: Each of these localities have additional constraints as follows: 1. For Ext edges, we require parent(n1) == - parenti(n2) for some i\>1, *and* for Value edges only there must be a order edge from n1 to - parenti-1(n2). + parenti(n2) for some i\>1, *and* for Value edges only: + * there must be a order edge from n1 to + parenti-1(n2). + * None of the parentj(n2), for i\>j\>=1, + may be a FuncDefn node The order edge records the ordering requirement that results, i.e. it must be possible to @@ -568,6 +571,9 @@ Each of these localities have additional constraints as follows: For Static edges this order edge is not required since the source is guaranteed to causally precede the target. + The FuncDefn restriction means that FuncDefn really are static, + and do not capture runtime values from their environment. + 2. For Dom edges, we must have that parent2(n1) == parenti(n2) is a CFG-node, for some i\>1, **and** parent(n1) strictly dominates @@ -576,6 +582,8 @@ Each of these localities have additional constraints as follows: i\>1 allows the node to target an arbitrarily-deep descendant of the dominated block, similar to an Ext edge.) + The same FuncDefn restriction also applies here, on the parent(j)(n2) for i\>j\>=1 (of course j=i is the CFG and j=i-1 is the basic block). + Specifically, these rules allow for edges where in a given execution of the HUGR the source of the edge executes once, but the target may execute \>=0 times.