From 51f3a026254642063713d4838e17b8808a7071c4 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 14:05:40 +0100 Subject: [PATCH 01/13] add_op_with_parent uses open_extensions; some test fixes --- src/algorithm/nest_cfgs.rs | 4 ++-- src/hugr/hugrmut.rs | 26 ++++++++++++-------------- src/hugr/rewrite/outline_cfg.rs | 4 ++-- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/algorithm/nest_cfgs.rs b/src/algorithm/nest_cfgs.rs index b7a9a1ec2..15154d4f2 100644 --- a/src/algorithm/nest_cfgs.rs +++ b/src/algorithm/nest_cfgs.rs @@ -646,7 +646,7 @@ pub(crate) mod test { ]) ); transform_cfg_to_nested(&mut IdentityCfgMap::new(rc)); - h.validate(&PRELUDE_REGISTRY).unwrap(); + h.update_validate(&PRELUDE_REGISTRY).unwrap(); assert_eq!(1, depth(&h, entry)); assert_eq!(1, depth(&h, exit)); for n in [split, left, right, merge, head, tail] { @@ -753,7 +753,7 @@ pub(crate) mod test { let root = h.root(); let m = SiblingMut::::try_new(&mut h, root).unwrap(); transform_cfg_to_nested(&mut IdentityCfgMap::new(m)); - h.validate(&PRELUDE_REGISTRY).unwrap(); + h.update_validate(&PRELUDE_REGISTRY).unwrap(); assert_eq!(1, depth(&h, entry)); assert_eq!(3, depth(&h, head)); for n in [split, left, right, merge] { diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 7f91752b5..c3cb0e3eb 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -40,7 +40,7 @@ pub trait HugrMut: HugrMutInternals { op: impl Into, ) -> Result { // TODO: Default to `NodeType::open_extensions` once we can infer extensions - self.add_node_with_parent(parent, NodeType::pure(op)) + self.add_node_with_parent(parent, NodeType::open_extensions(op)) } /// Add a node to the graph with a parent in the hierarchy. @@ -584,16 +584,15 @@ mod test { #[test] fn simple_function() { - // Starts an empty builder - let mut builder = Hugr::default(); + let mut hugr = Hugr::default(); // Create the root module definition - let module: Node = builder.root(); + let module: Node = hugr.root(); // Start a main function with two nat inputs. // // `add_op` is equivalent to `add_root_op` followed by `set_parent` - let f: Node = builder + let f: Node = hugr .add_op_with_parent( module, ops::FuncDefn { @@ -604,22 +603,21 @@ mod test { .expect("Failed to add function definition node"); { - let f_in = builder - .add_op_with_parent(f, ops::Input::new(type_row![NAT])) + let f_in = hugr + .add_node_with_parent(f, NodeType::pure(ops::Input::new(type_row![NAT]))) .unwrap(); - let f_out = builder + let f_out = hugr .add_op_with_parent(f, ops::Output::new(type_row![NAT, NAT])) .unwrap(); - let noop = builder + let noop = hugr .add_op_with_parent(f, LeafOp::Noop { ty: NAT }) .unwrap(); - assert!(builder.connect(f_in, 0, noop, 0).is_ok()); - assert!(builder.connect(noop, 0, f_out, 0).is_ok()); - assert!(builder.connect(noop, 0, f_out, 1).is_ok()); + hugr.connect(f_in, 0, noop, 0).unwrap(); + hugr.connect(noop, 0, f_out, 0).unwrap(); + hugr.connect(noop, 0, f_out, 1).unwrap(); } - // Finish the construction and create the HUGR - builder.validate(&PRELUDE_REGISTRY).unwrap(); + hugr.update_validate(&PRELUDE_REGISTRY).unwrap(); } } diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 910ea4fcc..2be2e8901 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -303,7 +303,7 @@ mod test { let (mut h, head, tail) = build_conditional_in_loop_cfg(false).unwrap(); h.update_validate(&PRELUDE_REGISTRY).unwrap(); do_outline_cfg_test(&mut h, head, tail, 1); - h.validate(&PRELUDE_REGISTRY).unwrap(); + h.update_validate(&PRELUDE_REGISTRY).unwrap(); } fn do_outline_cfg_test( @@ -405,7 +405,7 @@ mod test { let (new_block, new_cfg) = h .apply_rewrite(OutlineCfg::new(blocks_to_move.iter().copied())) .unwrap(); - h.validate(&PRELUDE_REGISTRY).unwrap(); + h.update_validate(&PRELUDE_REGISTRY).unwrap(); assert_eq!(new_block, h.children(h.root()).next().unwrap()); assert_matches!( h.get_optype(new_block), From 15074dc5132cc9fda8b65952a3d839a7c8c2ce67 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 14:17:58 +0100 Subject: [PATCH 02/13] Use open_extensions in other add_op methods too --- src/hugr.rs | 3 +-- src/hugr/hugrmut.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index ec44afa46..322a123c6 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -305,8 +305,7 @@ impl Hugr { /// Add a node to the graph, with the default conversion from OpType to NodeType pub(crate) fn add_op(&mut self, op: impl Into) -> Node { - // TODO: Default to `NodeType::open_extensions` once we can infer extensions - self.add_node(NodeType::pure(op)) + self.add_node(NodeType::open_extensions(op)) } /// Add a node to the graph. diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index c3cb0e3eb..2baca50af 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -39,7 +39,6 @@ pub trait HugrMut: HugrMutInternals { parent: Node, op: impl Into, ) -> Result { - // TODO: Default to `NodeType::open_extensions` once we can infer extensions self.add_node_with_parent(parent, NodeType::open_extensions(op)) } @@ -216,7 +215,7 @@ impl + AsMut> HugrMut for T { } fn add_op_before(&mut self, sibling: Node, op: impl Into) -> Result { - self.add_node_before(sibling, NodeType::pure(op)) + self.add_node_before(sibling, NodeType::open_extensions(op)) } fn add_node_before(&mut self, sibling: Node, nodetype: NodeType) -> Result { From c92a5e255c9f43e87cfe9d5f92448e1ca6a62a5a Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 14:34:14 +0100 Subject: [PATCH 03/13] Fix simple_alias: define unconstrained Metas as variables --- src/extension/infer.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/extension/infer.rs b/src/extension/infer.rs index ae59d9127..f1a03534a 100644 --- a/src/extension/infer.rs +++ b/src/extension/infer.rs @@ -268,11 +268,6 @@ impl UnificationContext { where T: HugrView, { - if hugr.root_type().signature().is_none() { - let m_input = self.make_or_get_meta(hugr.root(), Direction::Incoming); - self.variables.insert(m_input); - } - for node in hugr.nodes() { let m_input = self.make_or_get_meta(node, Direction::Incoming); let m_output = self.make_or_get_meta(node, Direction::Outgoing); @@ -317,6 +312,14 @@ impl UnificationContext { m_output, node_type.op_signature().extension_reqs, ); + let sig: &OpType = hugr.get_nodetype(node).into(); + if hugr.all_node_ports(node).all(|p| { + sig.port_kind(p) == Some(EdgeKind::StateOrder) + || hugr.linked_ports(node, p).next().is_none() + }) { + // Node has no edges that would constrain its extensions + self.variables.insert(m_input); + } } // We have a solution for everything! Some(sig) => { From 584e7f2b9c5df2ab5de15b7dc807200287b2059b Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 14:46:36 +0100 Subject: [PATCH 04/13] Fix most validate tests by doing update_validate --- src/hugr/validate.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index 57b9020c0..0aad1250b 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -884,13 +884,13 @@ mod test { let mut b = Hugr::new(NodeType::pure(dfg_op)); let root = b.root(); add_df_children(&mut b, root, 1); - assert_eq!(b.validate(&EMPTY_REG), Ok(())); + assert_eq!(b.update_validate(&EMPTY_REG), Ok(())); } #[test] fn simple_hugr() { - let b = make_simple_hugr(2).0; - assert_eq!(b.validate(&EMPTY_REG), Ok(())); + let mut b = make_simple_hugr(2).0; + assert_eq!(b.update_validate(&EMPTY_REG), Ok(())); } #[test] @@ -917,7 +917,7 @@ mod test { ) .unwrap(); assert_matches!( - b.validate(&EMPTY_REG), + b.update_validate(&EMPTY_REG), Err(ValidationError::ContainerWithoutChildren { node, .. }) => assert_eq!(node, new_def) ); @@ -925,7 +925,7 @@ mod test { add_df_children(&mut b, new_def, 2); b.set_parent(new_def, copy).unwrap(); assert_matches!( - b.validate(&EMPTY_REG), + b.update_validate(&EMPTY_REG), Err(ValidationError::NonContainerWithChildren { node, .. }) => assert_eq!(node, copy) ); b.set_parent(new_def, root).unwrap(); @@ -936,7 +936,7 @@ mod test { .add_op_with_parent(root, ops::Input::new(type_row![])) .unwrap(); assert_matches!( - b.validate(&EMPTY_REG), + b.update_validate(&EMPTY_REG), Err(ValidationError::InvalidParentOp { parent, child, .. }) => {assert_eq!(parent, root); assert_eq!(child, new_input)} ); } @@ -1006,7 +1006,7 @@ mod test { ) .unwrap(); assert_matches!( - b.validate(&EMPTY_REG), + b.update_validate(&EMPTY_REG), Err(ValidationError::ContainerWithoutChildren { .. }) ); let cfg = copy; @@ -1375,7 +1375,7 @@ mod test { } #[test] fn unregistered_extension() { - let (h, def) = identity_hugr_with_type(USIZE_T); + let (mut h, def) = identity_hugr_with_type(USIZE_T); assert_eq!( h.validate(&EMPTY_REG), Err(ValidationError::SignatureError { @@ -1383,7 +1383,7 @@ mod test { cause: SignatureError::ExtensionNotFound(PRELUDE.name.clone()) }) ); - h.validate(&PRELUDE_REGISTRY).unwrap(); + h.update_validate(&PRELUDE_REGISTRY).unwrap(); } #[test] @@ -1414,7 +1414,9 @@ mod test { TypeBound::Any, )); assert_eq!( - identity_hugr_with_type(valid.clone()).0.validate(®), + identity_hugr_with_type(valid.clone()) + .0 + .update_validate(®), Ok(()) ); From b5c6ffbd67572981a8090b7df8f6f81dfa6992cc Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 14:56:35 +0100 Subject: [PATCH 05/13] Fix cfg_children_restrictions --- src/hugr/validate.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index 0aad1250b..cea7425bd 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -997,7 +997,8 @@ mod test { .map_into() .collect_tuple() .unwrap(); - + let mut closure = b.infer_extensions().unwrap(); + assert!(closure.remove(©).is_some()); b.replace_op( copy, NodeType::pure(ops::CFG { @@ -1005,8 +1006,11 @@ mod test { }), ) .unwrap(); + // We feed in the closure from above because we can't compute the closure + // (i.e. update_and_validate) on such a malformed Hugr, and pure `validate(&EMPTY_REG)` + // fails because of missing extension annotations (contained in the closure) assert_matches!( - b.update_validate(&EMPTY_REG), + b.validate_with_extension_closure(closure, &EMPTY_REG), Err(ValidationError::ContainerWithoutChildren { .. }) ); let cfg = copy; @@ -1033,7 +1037,7 @@ mod test { ) .unwrap(); b.add_other_edge(block, exit).unwrap(); - assert_eq!(b.validate(&EMPTY_REG), Ok(())); + assert_eq!(b.update_validate(&EMPTY_REG), Ok(())); // Test malformed errors From 85f23211976d2c67d1d0951cb0462dffef431c1e Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 14:59:39 +0100 Subject: [PATCH 06/13] Lift m_tgt invariant --- src/extension/infer.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/extension/infer.rs b/src/extension/infer.rs index f1a03534a..6eaa778f3 100644 --- a/src/extension/infer.rs +++ b/src/extension/infer.rs @@ -341,16 +341,16 @@ impl UnificationContext { | Some(EdgeKind::ControlFlow) ) }) { + let m_tgt = *self + .extensions + .get(&(tgt_node, Direction::Incoming)) + .unwrap(); for (src_node, _) in hugr.linked_ports(tgt_node, port) { let m_src = self .extensions .get(&(src_node, Direction::Outgoing)) .unwrap(); - let m_tgt = self - .extensions - .get(&(tgt_node, Direction::Incoming)) - .unwrap(); - self.add_constraint(*m_src, Constraint::Equal(*m_tgt)); + self.add_constraint(*m_src, Constraint::Equal(m_tgt)); } } } From 46e567c48540dd692afabb001567f73b9c057cd4 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 15:04:01 +0100 Subject: [PATCH 07/13] Change some callers of add_node_before to use add_op_before --- src/builder/conditional.rs | 3 +-- src/extension/infer.rs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/builder/conditional.rs b/src/builder/conditional.rs index f1af1ad0d..7b196d3a9 100644 --- a/src/builder/conditional.rs +++ b/src/builder/conditional.rs @@ -126,8 +126,7 @@ impl + AsRef> ConditionalBuilder { let case_node = // add case before any existing subsequent cases if let Some(&sibling_node) = self.case_nodes[case + 1..].iter().flatten().next() { - // TODO: Allow this to be non-pure - self.hugr_mut().add_node_before(sibling_node, NodeType::open_extensions(case_op))? + self.hugr_mut().add_op_before(sibling_node, case_op)? } else { self.add_child_node(NodeType::open_extensions(case_op))? }; diff --git a/src/extension/infer.rs b/src/extension/infer.rs index 6eaa778f3..236d3aeb4 100644 --- a/src/extension/infer.rs +++ b/src/extension/infer.rs @@ -1220,7 +1220,7 @@ mod test { }), )?; - let entry = hugr.add_node_before(exit, NodeType::open_extensions(dfb))?; + let entry = hugr.add_op_before(exit,dfb)?; let entry_in = hugr.add_node_with_parent( entry, NodeType::open_extensions(ops::Input { types: inputs }), From ed4afa1b3865581eb35c731f1a190708de16b9b6 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 15:04:31 +0100 Subject: [PATCH 08/13] Remove add_node_before(?!) --- src/hugr/hugrmut.rs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 2baca50af..360011bb5 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -65,15 +65,6 @@ pub trait HugrMut: HugrMutInternals { self.hugr_mut().add_op_before(sibling, op) } - /// A generalisation of [`HugrMut::add_op_before`], needed temporarily until - /// add_op type methods all default to creating nodes with open extensions. - /// See issue #424 - #[inline] - fn add_node_before(&mut self, sibling: Node, nodetype: NodeType) -> Result { - self.valid_non_root(sibling)?; - self.hugr_mut().add_node_before(sibling, nodetype) - } - /// Add a node to the graph as the next sibling of another node. /// /// The sibling node's parent becomes the new node's parent. @@ -215,11 +206,7 @@ impl + AsMut> HugrMut for T { } fn add_op_before(&mut self, sibling: Node, op: impl Into) -> Result { - self.add_node_before(sibling, NodeType::open_extensions(op)) - } - - fn add_node_before(&mut self, sibling: Node, nodetype: NodeType) -> Result { - let node = self.as_mut().add_node(nodetype); + let node = self.as_mut().add_node(NodeType::open_extensions(op)); self.as_mut() .hierarchy .insert_before(node.index, sibling.index)?; From 9bb1b4991bc5d5f0dd9f2b9f560d18f93fc276da Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 15:25:53 +0100 Subject: [PATCH 09/13] No, keep add_node_before, maintain the parallel; remove TODO --- src/hugr/hugrmut.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 360011bb5..ff0aef0e7 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -65,6 +65,15 @@ pub trait HugrMut: HugrMutInternals { self.hugr_mut().add_op_before(sibling, op) } + /// Add a node to the graph as the previous sibling of another node. + /// + /// The sibling node's parent becomes the new node's parent. + #[inline] + fn add_node_before(&mut self, sibling: Node, nodetype: NodeType) -> Result { + self.valid_non_root(sibling)?; + self.hugr_mut().add_node_before(sibling, nodetype) + } + /// Add a node to the graph as the next sibling of another node. /// /// The sibling node's parent becomes the new node's parent. @@ -206,7 +215,11 @@ impl + AsMut> HugrMut for T { } fn add_op_before(&mut self, sibling: Node, op: impl Into) -> Result { - let node = self.as_mut().add_node(NodeType::open_extensions(op)); + self.add_node_before(sibling, NodeType::open_extensions(op)) + } + + fn add_node_before(&mut self, sibling: Node, nodetype: NodeType) -> Result { + let node = self.as_mut().add_node(nodetype); self.as_mut() .hierarchy .insert_before(node.index, sibling.index)?; From bb79165e373162bb7742a8a9350835e1f38ec182 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 24 Oct 2023 15:26:55 +0100 Subject: [PATCH 10/13] Change many add_node+open_extensions to use add_op --- src/builder.rs | 12 ++-- src/builder/conditional.rs | 2 +- src/extension/infer.rs | 140 +++++++++++++++++-------------------- src/hugr.rs | 8 +-- 4 files changed, 75 insertions(+), 87 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index c8c0cdc55..51b90f8a2 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -148,18 +148,18 @@ pub(crate) mod test { let mut hugr = Hugr::new(NodeType::pure(ops::DFG { signature: signature.clone(), })); - hugr.add_node_with_parent( + hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::Input { + ops::Input { types: signature.input, - }), + }, ) .unwrap(); - hugr.add_node_with_parent( + hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::Output { + ops::Output { types: signature.output, - }), + }, ) .unwrap(); hugr diff --git a/src/builder/conditional.rs b/src/builder/conditional.rs index 7b196d3a9..da8808eea 100644 --- a/src/builder/conditional.rs +++ b/src/builder/conditional.rs @@ -128,7 +128,7 @@ impl + AsRef> ConditionalBuilder { if let Some(&sibling_node) = self.case_nodes[case + 1..].iter().flatten().next() { self.hugr_mut().add_op_before(sibling_node, case_op)? } else { - self.add_child_node(NodeType::open_extensions(case_op))? + self.add_child_op(case_op)? }; self.case_nodes[case] = Some(case_node); diff --git a/src/extension/infer.rs b/src/extension/infer.rs index 236d3aeb4..88efb5cfe 100644 --- a/src/extension/infer.rs +++ b/src/extension/infer.rs @@ -730,11 +730,11 @@ mod test { let root_node = NodeType::open_extensions(op); let mut hugr = Hugr::new(root_node); - let input = NodeType::open_extensions(ops::Input::new(type_row![NAT, NAT])); - let output = NodeType::open_extensions(ops::Output::new(type_row![NAT])); + let input = ops::Input::new(type_row![NAT, NAT]); + let output = ops::Output::new(type_row![NAT]); - let input = hugr.add_node_with_parent(hugr.root(), input)?; - let output = hugr.add_node_with_parent(hugr.root(), output)?; + let input = hugr.add_op_with_parent(hugr.root(), input)?; + let output = hugr.add_op_with_parent(hugr.root(), output)?; assert_matches!(hugr.get_io(hugr.root()), Some(_)); @@ -750,29 +750,29 @@ mod test { let mult_c_sig = FunctionType::new(type_row![NAT, NAT], type_row![NAT]) .with_extension_delta(&ExtensionSet::singleton(&C)); - let add_a = hugr.add_node_with_parent( + let add_a = hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::DFG { + ops::DFG { signature: add_a_sig, - }), + }, )?; - let add_b = hugr.add_node_with_parent( + let add_b = hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::DFG { + ops::DFG { signature: add_b_sig, - }), + }, )?; - let add_ab = hugr.add_node_with_parent( + let add_ab = hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::DFG { + ops::DFG { signature: add_ab_sig, - }), + }, )?; - let mult_c = hugr.add_node_with_parent( + let mult_c = hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::DFG { + ops::DFG { signature: mult_c_sig, - }), + }, )?; hugr.connect(input, 0, add_a, 0)?; @@ -906,29 +906,26 @@ mod test { let [input, output] = hugr.get_io(hugr.root()).unwrap(); let add_r_sig = FunctionType::new(type_row![NAT], type_row![NAT]).with_extension_delta(&rs); - let add_r = hugr.add_node_with_parent( + let add_r = hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::DFG { + ops::DFG { signature: add_r_sig, - }), + }, )?; // Dangling thingy let src_sig = FunctionType::new(type_row![], type_row![NAT]) .with_extension_delta(&ExtensionSet::new()); - let src = hugr.add_node_with_parent( - hugr.root(), - NodeType::open_extensions(ops::DFG { signature: src_sig }), - )?; + let src = hugr.add_op_with_parent(hugr.root(), ops::DFG { signature: src_sig })?; let mult_sig = FunctionType::new(type_row![NAT, NAT], type_row![NAT]); // Mult has open extension requirements, which we should solve to be "R" - let mult = hugr.add_node_with_parent( + let mult = hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::DFG { + ops::DFG { signature: mult_sig, - }), + }, )?; hugr.connect(input, 0, add_r, 0)?; @@ -988,18 +985,18 @@ mod test { ) -> Result<[Node; 3], Box> { let op: OpType = op.into(); - let node = hugr.add_node_with_parent(parent, NodeType::open_extensions(op))?; - let input = hugr.add_node_with_parent( + let node = hugr.add_op_with_parent(parent, op)?; + let input = hugr.add_op_with_parent( node, - NodeType::open_extensions(ops::Input { + ops::Input { types: op_sig.input, - }), + }, )?; - let output = hugr.add_node_with_parent( + let output = hugr.add_op_with_parent( node, - NodeType::open_extensions(ops::Output { + ops::Output { types: op_sig.output, - }), + }, )?; Ok([node, input, output]) } @@ -1020,20 +1017,20 @@ mod test { Into::::into(op).signature(), )?; - let lift1 = hugr.add_node_with_parent( + let lift1 = hugr.add_op_with_parent( case, - NodeType::open_extensions(ops::LeafOp::Lift { + ops::LeafOp::Lift { type_row: type_row![NAT], new_extension: first_ext, - }), + }, )?; - let lift2 = hugr.add_node_with_parent( + let lift2 = hugr.add_op_with_parent( case, - NodeType::open_extensions(ops::LeafOp::Lift { + ops::LeafOp::Lift { type_row: type_row![NAT], new_extension: second_ext, - }), + }, )?; hugr.connect(case_in, 0, lift1, 0)?; @@ -1098,17 +1095,17 @@ mod test { })); let root = hugr.root(); - let input = hugr.add_node_with_parent( + let input = hugr.add_op_with_parent( root, - NodeType::open_extensions(ops::Input { + ops::Input { types: type_row![NAT], - }), + }, )?; - let output = hugr.add_node_with_parent( + let output = hugr.add_op_with_parent( root, - NodeType::open_extensions(ops::Output { + ops::Output { types: type_row![NAT], - }), + }, )?; // Make identical dataflow nodes which add extension requirement "A" or "B" @@ -1129,12 +1126,12 @@ mod test { .unwrap(); let lift = hugr - .add_node_with_parent( + .add_op_with_parent( node, - NodeType::open_extensions(ops::LeafOp::Lift { + ops::LeafOp::Lift { type_row: type_row![NAT], new_extension: ext, - }), + }, ) .unwrap(); @@ -1181,7 +1178,7 @@ mod test { let [bb, bb_in, bb_out] = create_with_io(hugr, bb_parent, dfb, dfb_sig)?; - let dfg = hugr.add_node_with_parent(bb, NodeType::open_extensions(op))?; + let dfg = hugr.add_op_with_parent(bb, op)?; hugr.connect(bb_in, 0, dfg, 0)?; hugr.connect(dfg, 0, bb_out, 0)?; @@ -1213,23 +1210,20 @@ mod test { extension_delta: entry_extensions, }; - let exit = hugr.add_node_with_parent( + let exit = hugr.add_op_with_parent( root, - NodeType::open_extensions(ops::BasicBlock::Exit { + ops::BasicBlock::Exit { cfg_outputs: exit_types.into(), - }), + }, )?; - let entry = hugr.add_op_before(exit,dfb)?; - let entry_in = hugr.add_node_with_parent( - entry, - NodeType::open_extensions(ops::Input { types: inputs }), - )?; - let entry_out = hugr.add_node_with_parent( + let entry = hugr.add_op_before(exit, dfb)?; + let entry_in = hugr.add_op_with_parent(entry, ops::Input { types: inputs })?; + let entry_out = hugr.add_op_with_parent( entry, - NodeType::open_extensions(ops::Output { + ops::Output { types: vec![entry_tuple_sum].into(), - }), + }, )?; Ok(([entry, entry_in, entry_out], exit)) @@ -1280,12 +1274,12 @@ mod test { type_row![NAT], )?; - let mkpred = hugr.add_node_with_parent( + let mkpred = hugr.add_op_with_parent( entry, - NodeType::open_extensions(make_opaque( + make_opaque( A, FunctionType::new(vec![NAT], twoway(NAT)).with_extension_delta(&a), - )), + ), )?; // Internal wiring for DFGs @@ -1376,12 +1370,9 @@ mod test { type_row![NAT], )?; - let entry_mid = hugr.add_node_with_parent( + let entry_mid = hugr.add_op_with_parent( entry, - NodeType::open_extensions(make_opaque( - UNKNOWN_EXTENSION, - FunctionType::new(vec![NAT], twoway(NAT)), - )), + make_opaque(UNKNOWN_EXTENSION, FunctionType::new(vec![NAT], twoway(NAT))), )?; hugr.connect(entry_in, 0, entry_mid, 0)?; @@ -1465,12 +1456,12 @@ mod test { type_row![NAT], )?; - let entry_dfg = hugr.add_node_with_parent( + let entry_dfg = hugr.add_op_with_parent( entry, - NodeType::open_extensions(make_opaque( + make_opaque( UNKNOWN_EXTENSION, FunctionType::new(vec![NAT], oneway(NAT)).with_extension_delta(&entry_ext), - )), + ), )?; hugr.connect(entry_in, 0, entry_dfg, 0)?; @@ -1546,12 +1537,9 @@ mod test { type_row![NAT], )?; - let entry_mid = hugr.add_node_with_parent( + let entry_mid = hugr.add_op_with_parent( entry, - NodeType::open_extensions(make_opaque( - UNKNOWN_EXTENSION, - FunctionType::new(vec![NAT], oneway(NAT)), - )), + make_opaque(UNKNOWN_EXTENSION, FunctionType::new(vec![NAT], oneway(NAT))), )?; hugr.connect(entry_in, 0, entry_mid, 0)?; diff --git a/src/hugr.rs b/src/hugr.rs index 322a123c6..903706e68 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -609,7 +609,7 @@ impl From for PyErr { #[cfg(test)] mod test { - use super::{Hugr, HugrView, NodeType}; + use super::{Hugr, HugrView}; use crate::builder::test::closed_dfg_root_hugr; use crate::extension::ExtensionSet; use crate::hugr::HugrMut; @@ -645,12 +645,12 @@ mod test { FunctionType::new(type_row![BIT], type_row![BIT]).with_extension_delta(&r), ); let [input, output] = hugr.get_io(hugr.root()).unwrap(); - let lift = hugr.add_node_with_parent( + let lift = hugr.add_op_with_parent( hugr.root(), - NodeType::open_extensions(ops::LeafOp::Lift { + ops::LeafOp::Lift { type_row: type_row![BIT], new_extension: "R".try_into().unwrap(), - }), + }, )?; hugr.connect(input, 0, lift, 0)?; hugr.connect(lift, 0, output, 0)?; From 923415479183d54d94d753dc55059aac136ac965 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 25 Oct 2023 09:07:23 +0100 Subject: [PATCH 11/13] Fix cfg_children_restrictions test in a different way (instantiate_extensions) --- src/hugr/validate.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index cea7425bd..4d505418a 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -997,8 +997,11 @@ mod test { .map_into() .collect_tuple() .unwrap(); - let mut closure = b.infer_extensions().unwrap(); - assert!(closure.remove(©).is_some()); + // Write Extension annotations into the Hugr while it's still well-formed + // enough for us to compute them + let closure = b.infer_extensions().unwrap(); + b.instantiate_extensions(closure); + b.validate(&EMPTY_REG).unwrap(); b.replace_op( copy, NodeType::pure(ops::CFG { @@ -1006,11 +1009,8 @@ mod test { }), ) .unwrap(); - // We feed in the closure from above because we can't compute the closure - // (i.e. update_and_validate) on such a malformed Hugr, and pure `validate(&EMPTY_REG)` - // fails because of missing extension annotations (contained in the closure) assert_matches!( - b.validate_with_extension_closure(closure, &EMPTY_REG), + b.validate(&EMPTY_REG), Err(ValidationError::ContainerWithoutChildren { .. }) ); let cfg = copy; From 59eb2604f4303555797149fa8925142c009dcf28 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 25 Oct 2023 18:40:49 +0100 Subject: [PATCH 12/13] Better fix to gen_constraints. Fix children_restrictions other way to cfg_children_restrictions. --- src/extension/infer.rs | 16 ++++++++-------- src/hugr/validate.rs | 3 ++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/extension/infer.rs b/src/extension/infer.rs index 88efb5cfe..32227a554 100644 --- a/src/extension/infer.rs +++ b/src/extension/infer.rs @@ -307,19 +307,19 @@ impl UnificationContext { match node_type.signature() { // Input extensions are open None => { + if node == hugr.root() + || matches!( + node_type.tag(), + OpTag::Alias | OpTag::FuncDefn | OpTag::Function + ) + { + self.variables.insert(m_input); + } self.gen_union_constraint( m_input, m_output, node_type.op_signature().extension_reqs, ); - let sig: &OpType = hugr.get_nodetype(node).into(); - if hugr.all_node_ports(node).all(|p| { - sig.port_kind(p) == Some(EdgeKind::StateOrder) - || hugr.linked_ports(node, p).next().is_none() - }) { - // Node has no edges that would constrain its extensions - self.variables.insert(m_input); - } } // We have a solution for everything! Some(sig) => { diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index 4d505418a..617959989 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -928,6 +928,7 @@ mod test { b.update_validate(&EMPTY_REG), Err(ValidationError::NonContainerWithChildren { node, .. }) => assert_eq!(node, copy) ); + let closure = b.infer_extensions().unwrap(); b.set_parent(new_def, root).unwrap(); // After moving the previous definition to a valid place, @@ -936,7 +937,7 @@ mod test { .add_op_with_parent(root, ops::Input::new(type_row![])) .unwrap(); assert_matches!( - b.update_validate(&EMPTY_REG), + b.validate_with_extension_closure(closure, &EMPTY_REG), Err(ValidationError::InvalidParentOp { parent, child, .. }) => {assert_eq!(parent, root); assert_eq!(child, new_input)} ); } From dde1ff7e58faeaedaeebd5907a3fe2052a18613b Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 29 Oct 2023 20:10:22 +0000 Subject: [PATCH 13/13] Simpler solution! - add_solution w/ empty ExtensionSet --- src/extension/infer.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/extension/infer.rs b/src/extension/infer.rs index 32227a554..669f8dced 100644 --- a/src/extension/infer.rs +++ b/src/extension/infer.rs @@ -268,6 +268,11 @@ impl UnificationContext { where T: HugrView, { + if hugr.root_type().signature().is_none() { + let m_input = self.make_or_get_meta(hugr.root(), Direction::Incoming); + self.variables.insert(m_input); + } + for node in hugr.nodes() { let m_input = self.make_or_get_meta(node, Direction::Incoming); let m_output = self.make_or_get_meta(node, Direction::Outgoing); @@ -307,19 +312,17 @@ impl UnificationContext { match node_type.signature() { // Input extensions are open None => { - if node == hugr.root() - || matches!( - node_type.tag(), - OpTag::Alias | OpTag::FuncDefn | OpTag::Function - ) - { - self.variables.insert(m_input); - } self.gen_union_constraint( m_input, m_output, node_type.op_signature().extension_reqs, ); + if matches!( + node_type.tag(), + OpTag::Alias | OpTag::Function | OpTag::FuncDefn + ) { + self.add_solution(m_input, ExtensionSet::new()); + } } // We have a solution for everything! Some(sig) => {