From 0de995ff1eefccdecbf7d6fb00b9ee1394283bfc Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 18:34:15 +0000 Subject: [PATCH 01/26] FuncDefn/FuncDecl store PolyFuncType - some horrendous hacks with // ALAN --- src/builder/build_traits.rs | 6 ++++-- src/builder/dataflow.rs | 3 ++- src/builder/module.rs | 6 +++--- src/hugr/hugrmut.rs | 2 +- src/hugr/validate/test.rs | 6 +++--- src/hugr/views.rs | 10 +++++----- src/hugr/views/descendants.rs | 4 ++-- src/hugr/views/sibling.rs | 6 +++++- src/hugr/views/sibling_subgraph.rs | 2 +- src/ops/module.rs | 6 +++--- src/ops/validate.rs | 10 +++------- src/types/check.rs | 2 +- src/types/poly_func.rs | 12 +++--------- 13 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/builder/build_traits.rs b/src/builder/build_traits.rs index dfdc86488..f1861d760 100644 --- a/src/builder/build_traits.rs +++ b/src/builder/build_traits.rs @@ -95,7 +95,7 @@ pub trait Container { let f_node = self.add_child_node(NodeType::new( ops::FuncDefn { name: name.into(), - signature: signature.signature.clone(), + signature: signature.signature.clone().into(), }, signature.input_extensions.clone(), ))?; @@ -602,7 +602,7 @@ pub trait Dataflow: Container { ) -> Result, BuildError> { let hugr = self.hugr(); let def_op = hugr.get_optype(function.node()); - let signature = match def_op { + let type_scheme = match def_op { OpType::FuncDefn(ops::FuncDefn { signature, .. }) | OpType::FuncDecl(ops::FuncDecl { signature, .. }) => signature.clone(), _ => { @@ -612,7 +612,9 @@ pub trait Dataflow: Container { }) } }; + let signature = type_scheme.body; let const_in_port = signature.output.len(); + // TODO ALAN this is totally broken for polymorphic functions. Need to TypeApply here. let op_id = self.add_dataflow_op(ops::Call { signature }, input_wires)?; let src_port = self.hugr_mut().num_outputs(function.node()) - 1; diff --git a/src/builder/dataflow.rs b/src/builder/dataflow.rs index 3ef28ac8d..0b59f36bf 100644 --- a/src/builder/dataflow.rs +++ b/src/builder/dataflow.rs @@ -147,8 +147,9 @@ impl FunctionBuilder { /// /// Error in adding DFG child nodes. pub fn new(name: impl Into, signature: Signature) -> Result { + // ALAN this looks dodgy too let op = ops::FuncDefn { - signature: signature.clone().into(), + signature: signature.signature.clone().into(), name: name.into(), }; diff --git a/src/builder/module.rs b/src/builder/module.rs index a78c047d7..4de8e76b2 100644 --- a/src/builder/module.rs +++ b/src/builder/module.rs @@ -96,7 +96,7 @@ impl + AsRef> ModuleBuilder { }), )?; - let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, signature, None)?; + let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, signature.body, None)?; // ALAN Ok(FunctionBuilder::from_dfg_builder(db)) } @@ -109,13 +109,13 @@ impl + AsRef> ModuleBuilder { pub fn declare( &mut self, name: impl Into, - signature: Signature, + signature: Signature, // ALAN ) -> Result, BuildError> { // TODO add param names to metadata let rs = signature.input_extensions.clone(); let declare_n = self.add_child_node(NodeType::new( ops::FuncDecl { - signature: signature.into(), + signature: signature.signature.into(), name: name.into(), }, rs, diff --git a/src/hugr/hugrmut.rs b/src/hugr/hugrmut.rs index 00bef6d18..28a616f8c 100644 --- a/src/hugr/hugrmut.rs +++ b/src/hugr/hugrmut.rs @@ -636,7 +636,7 @@ mod test { module, ops::FuncDefn { name: "main".into(), - signature: FunctionType::new(type_row![NAT], type_row![NAT, NAT]), + signature: FunctionType::new(type_row![NAT], type_row![NAT, NAT]).into(), }, ) .expect("Failed to add function definition node"); diff --git a/src/hugr/validate/test.rs b/src/hugr/validate/test.rs index 2cf5d1812..7eec5015d 100644 --- a/src/hugr/validate/test.rs +++ b/src/hugr/validate/test.rs @@ -27,7 +27,7 @@ const Q: Type = crate::extension::prelude::QB_T; fn make_simple_hugr(copies: usize) -> (Hugr, Node) { let def_op: OpType = ops::FuncDefn { name: "main".into(), - signature: FunctionType::new(type_row![BOOL_T], vec![BOOL_T; copies]), + signature: FunctionType::new(type_row![BOOL_T], vec![BOOL_T; copies]).into(), } .into(); @@ -173,7 +173,7 @@ fn children_restrictions() { .add_node_with_parent( root, ops::FuncDefn { - signature: def_sig, + signature: def_sig.into(), name: "main".into(), }, ) @@ -634,7 +634,7 @@ fn identity_hugr_with_type(t: Type) -> (Hugr, Node) { b.root(), ops::FuncDefn { name: "main".into(), - signature: FunctionType::new(row.clone(), row.clone()), + signature: FunctionType::new(row.clone(), row.clone()).into(), }, ) .unwrap(); diff --git a/src/hugr/views.rs b/src/hugr/views.rs index eafc68986..eff0e4499 100644 --- a/src/hugr/views.rs +++ b/src/hugr/views.rs @@ -25,7 +25,7 @@ use portgraph::{multiportgraph, LinkView, MultiPortGraph, PortView}; use super::{Hugr, HugrError, NodeMetadata, NodeMetadataMap, NodeType, DEFAULT_NODETYPE}; use crate::ops::handle::NodeHandle; use crate::ops::{FuncDecl, FuncDefn, OpName, OpTag, OpTrait, OpType, DFG}; -use crate::types::{EdgeKind, FunctionType}; +use crate::types::{EdgeKind, PolyFuncType}; use crate::{Direction, IncomingPort, Node, OutgoingPort, Port}; /// A trait for inspecting HUGRs. @@ -267,12 +267,12 @@ pub trait HugrView: sealed::HugrInternals { /// For function-like HUGRs (DFG, FuncDefn, FuncDecl), report the function /// type. Otherwise return None. - fn get_function_type(&self) -> Option<&FunctionType> { + fn get_function_type(&self) -> Option { let op = self.get_nodetype(self.root()); match &op.op { - OpType::DFG(DFG { signature }) - | OpType::FuncDecl(FuncDecl { signature, .. }) - | OpType::FuncDefn(FuncDefn { signature, .. }) => Some(signature), + OpType::DFG(DFG { signature }) => Some(signature.clone().into()), + OpType::FuncDecl(FuncDecl { signature, .. }) + | OpType::FuncDefn(FuncDefn { signature, .. }) => Some(signature.clone()), _ => None, } } diff --git a/src/hugr/views/descendants.rs b/src/hugr/views/descendants.rs index bd31107a2..5cfdbd34b 100644 --- a/src/hugr/views/descendants.rs +++ b/src/hugr/views/descendants.rs @@ -261,12 +261,12 @@ pub(super) mod test { assert_eq!( region.get_function_type(), - Some(&FunctionType::new(type_row![NAT, QB], type_row![NAT, QB])) + Some(FunctionType::new(type_row![NAT, QB], type_row![NAT, QB]).into()) ); let inner_region: DescendantsGraph = DescendantsGraph::try_new(&hugr, inner)?; assert_eq!( inner_region.get_function_type(), - Some(&FunctionType::new(type_row![NAT], type_row![NAT])) + Some(FunctionType::new(type_row![NAT], type_row![NAT]).into()) ); Ok(()) diff --git a/src/hugr/views/sibling.rs b/src/hugr/views/sibling.rs index 77f74f39b..6426dcefe 100644 --- a/src/hugr/views/sibling.rs +++ b/src/hugr/views/sibling.rs @@ -442,7 +442,11 @@ mod test { fn flat_mut(mut simple_dfg_hugr: Hugr) { simple_dfg_hugr.update_validate(&PRELUDE_REGISTRY).unwrap(); let root = simple_dfg_hugr.root(); - let signature = simple_dfg_hugr.get_function_type().unwrap().clone(); + let signature = simple_dfg_hugr + .get_function_type() + .unwrap() + .instantiate(&[], &PRELUDE_REGISTRY) + .unwrap(); let sib_mut = SiblingMut::::try_new(&mut simple_dfg_hugr, root); assert_eq!( diff --git a/src/hugr/views/sibling_subgraph.rs b/src/hugr/views/sibling_subgraph.rs index 515392615..616b37ce0 100644 --- a/src/hugr/views/sibling_subgraph.rs +++ b/src/hugr/views/sibling_subgraph.rs @@ -962,7 +962,7 @@ mod tests { let OpType::FuncDefn(func_defn) = hugr.get_optype(func_root) else { panic!() }; - assert_eq!(func_defn.signature, func.signature(&func_graph)) + assert_eq!(func_defn.signature, func.signature(&func_graph).into()) } #[test] diff --git a/src/ops/module.rs b/src/ops/module.rs index 2bdb5d400..571789183 100644 --- a/src/ops/module.rs +++ b/src/ops/module.rs @@ -2,7 +2,7 @@ use smol_str::SmolStr; -use crate::types::{EdgeKind, FunctionType}; +use crate::types::{EdgeKind, PolyFuncType}; use crate::types::{Type, TypeBound}; use super::StaticTag; @@ -36,7 +36,7 @@ pub struct FuncDefn { /// Name of function pub name: String, /// Signature of the function - pub signature: FunctionType, + pub signature: PolyFuncType, } impl_op_name!(FuncDefn); @@ -63,7 +63,7 @@ pub struct FuncDecl { /// Name of function pub name: String, /// Signature of the function - pub signature: FunctionType, + pub signature: PolyFuncType, } impl_op_name!(FuncDecl); diff --git a/src/ops/validate.rs b/src/ops/validate.rs index 6c6ff65cd..73ad6327a 100644 --- a/src/ops/validate.rs +++ b/src/ops/validate.rs @@ -10,7 +10,7 @@ use itertools::Itertools; use portgraph::{NodeIndex, PortOffset}; use thiserror::Error; -use crate::types::{Type, TypeRow}; +use crate::types::{FunctionType, Type, TypeRow}; use crate::Direction; use super::{impl_validate_op, BasicBlock, OpTag, OpTrait, OpType, ValidateOp}; @@ -97,12 +97,8 @@ impl ValidateOp for super::FuncDefn { &self, children: impl DoubleEndedIterator, ) -> Result<(), ChildrenValidationError> { - validate_io_nodes( - &self.signature.input, - &self.signature.output, - "function definition", - children, - ) + let FunctionType { input, output, .. } = &self.signature.body; + validate_io_nodes(input, output, "function definition", children) } } diff --git a/src/types/check.rs b/src/types/check.rs index 496cc2a2d..e17ae98d8 100644 --- a/src/types/check.rs +++ b/src/types/check.rs @@ -65,7 +65,7 @@ impl PrimType { Ok(()) } (PrimType::Function(t), PrimValue::Function { hugr: v }) - if v.get_function_type().is_some_and(|f| &**t == f) => + if v.get_function_type().is_some_and(|f| **t == f) => { // exact signature equality, in future this may need to be // relaxed to be compatibility checks between the signatures. diff --git a/src/types/poly_func.rs b/src/types/poly_func.rs index a8ab29ff7..22a28554a 100644 --- a/src/types/poly_func.rs +++ b/src/types/poly_func.rs @@ -15,7 +15,7 @@ use super::{FunctionType, Substitution}; /// [Graph]: crate::values::PrimValue::Function /// [OpDef]: crate::extension::OpDef #[derive( - Clone, PartialEq, Debug, Eq, derive_more::Display, serde::Serialize, serde::Deserialize, + Clone, PartialEq, Debug, Default, Eq, derive_more::Display, serde::Serialize, serde::Deserialize, )] #[display( fmt = "forall {}. {}", @@ -31,7 +31,7 @@ pub struct PolyFuncType { /// [TypeArg]: super::type_param::TypeArg params: Vec, /// Template for the function. May contain variables up to length of [Self::params] - pub(super) body: FunctionType, + pub(crate) body: FunctionType, } impl From for PolyFuncType { @@ -146,12 +146,6 @@ impl PolyFuncType { } } -impl PartialEq for PolyFuncType { - fn eq(&self, other: &FunctionType) -> bool { - self.params.is_empty() && &self.body == other - } -} - /// A [Substitution] with a finite list of known values. /// (Variables out of the range of the list will result in a panic) struct SubstValues<'a>(&'a [TypeArg], &'a ExtensionRegistry); @@ -479,7 +473,7 @@ pub(crate) mod test { let actual = array_max .instantiate_poly(&[USIZE_TA, TypeArg::BoundedNat { n: 3 }], &PRELUDE_REGISTRY)?; - assert_eq!(actual, concrete); + assert_eq!(actual, concrete.into()); // forall N.(Array -> usize) let partial = PolyFuncType::new_validated( From 6c0edbbe87312b1d3b8c18efb773deddc4c393db Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 19:37:10 +0000 Subject: [PATCH 02/26] build_main/define_function take PolyFuncType not Signature, but tail_loop test --- src/builder.rs | 4 ++-- src/builder/build_traits.rs | 17 +++++++---------- src/builder/cfg.rs | 2 +- src/builder/circuit.rs | 4 ++-- src/builder/conditional.rs | 2 +- src/builder/dataflow.rs | 8 ++++---- src/builder/module.rs | 6 +++--- src/builder/tail_loop.rs | 15 +++++++++++++-- src/hugr/rewrite/outline_cfg.rs | 2 +- src/hugr/rewrite/simple_replace.rs | 2 +- src/hugr/serialize.rs | 4 ++-- src/hugr/validate/test.rs | 6 +++--- src/hugr/views/descendants.rs | 2 +- src/hugr/views/sibling.rs | 2 +- 14 files changed, 42 insertions(+), 34 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 5e437bcbc..d0e743eb9 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -100,7 +100,7 @@ pub(crate) mod test { use crate::hugr::{views::HugrView, HugrMut, NodeType}; use crate::ops; - use crate::types::{FunctionType, Signature, Type}; + use crate::types::{FunctionType, PolyFuncType, Type}; use crate::{type_row, Hugr}; use super::handle::BuildHandle; @@ -123,7 +123,7 @@ pub(crate) mod test { } pub(super) fn build_main( - signature: Signature, + signature: PolyFuncType, f: impl FnOnce(FunctionBuilder<&mut Hugr>) -> Result>, BuildError>, ) -> Result { let mut module_builder = ModuleBuilder::new(); diff --git a/src/builder/build_traits.rs b/src/builder/build_traits.rs index f1861d760..1ab9b8236 100644 --- a/src/builder/build_traits.rs +++ b/src/builder/build_traits.rs @@ -20,7 +20,7 @@ use crate::{ }; use crate::extension::{ExtensionRegistry, ExtensionSet, PRELUDE_REGISTRY}; -use crate::types::{FunctionType, Signature, Type, TypeRow}; +use crate::types::{FunctionType, PolyFuncType, Type, TypeRow}; use itertools::Itertools; @@ -90,22 +90,19 @@ pub trait Container { fn define_function( &mut self, name: impl Into, - signature: Signature, + signature: PolyFuncType, ) -> Result, BuildError> { + let body = signature.body.clone(); let f_node = self.add_child_node(NodeType::new( ops::FuncDefn { name: name.into(), - signature: signature.signature.clone().into(), + signature: signature, }, - signature.input_extensions.clone(), + ExtensionSet::new(), ))?; - let db = DFGBuilder::create_with_io( - self.hugr_mut(), - f_node, - signature.signature, - Some(signature.input_extensions), - )?; + let db = + DFGBuilder::create_with_io(self.hugr_mut(), f_node, body, Some(ExtensionSet::new()))?; Ok(FunctionBuilder::from_dfg_builder(db)) } diff --git a/src/builder/cfg.rs b/src/builder/cfg.rs index fe2954bd5..99781ea2c 100644 --- a/src/builder/cfg.rs +++ b/src/builder/cfg.rs @@ -336,7 +336,7 @@ mod test { let build_result = { let mut module_builder = ModuleBuilder::new(); let mut func_builder = module_builder - .define_function("main", FunctionType::new(vec![NAT], type_row![NAT]).pure())?; + .define_function("main", FunctionType::new(vec![NAT], type_row![NAT]).into())?; let _f_id = { let [int] = func_builder.input_wires_arr(); diff --git a/src/builder/circuit.rs b/src/builder/circuit.rs index 043945ff9..054f839fa 100644 --- a/src/builder/circuit.rs +++ b/src/builder/circuit.rs @@ -147,7 +147,7 @@ mod test { #[test] fn simple_linear() { let build_res = build_main( - FunctionType::new(type_row![QB, QB], type_row![QB, QB]).pure(), + FunctionType::new(type_row![QB, QB], type_row![QB, QB]).into(), |mut f_build| { let wires = f_build.input_wires().collect(); @@ -184,7 +184,7 @@ mod test { .into(), ); let build_res = build_main( - FunctionType::new(type_row![QB, QB, NAT], type_row![QB, QB, BOOL_T]).pure(), + FunctionType::new(type_row![QB, QB, NAT], type_row![QB, QB, BOOL_T]).into(), |mut f_build| { let [q0, q1, angle]: [Wire; 3] = f_build.input_wires_arr(); diff --git a/src/builder/conditional.rs b/src/builder/conditional.rs index e6219f46a..b12b78f4f 100644 --- a/src/builder/conditional.rs +++ b/src/builder/conditional.rs @@ -239,7 +239,7 @@ mod test { let mut module_builder = ModuleBuilder::new(); let mut fbuild = module_builder.define_function( "main", - FunctionType::new(type_row![NAT], type_row![NAT]).pure(), + FunctionType::new(type_row![NAT], type_row![NAT]).into(), )?; let tru_const = fbuild.add_constant(Const::true_val(), ExtensionSet::new())?; let _fdef = { diff --git a/src/builder/dataflow.rs b/src/builder/dataflow.rs index 0b59f36bf..e9d55b8c1 100644 --- a/src/builder/dataflow.rs +++ b/src/builder/dataflow.rs @@ -240,7 +240,7 @@ pub(crate) mod test { let _f_id = { let mut func_builder = module_builder.define_function( "main", - FunctionType::new(type_row![NAT, QB], type_row![NAT, QB]).pure(), + FunctionType::new(type_row![NAT, QB], type_row![NAT, QB]).into(), )?; let [int, qb] = func_builder.input_wires_arr(); @@ -274,7 +274,7 @@ pub(crate) mod test { let f_build = module_builder.define_function( "main", - FunctionType::new(type_row![BOOL_T], type_row![BOOL_T, BOOL_T]).pure(), + FunctionType::new(type_row![BOOL_T], type_row![BOOL_T, BOOL_T]).into(), )?; f(f_build)?; @@ -324,7 +324,7 @@ pub(crate) mod test { let f_build = module_builder.define_function( "main", - FunctionType::new(type_row![QB], type_row![QB, QB]).pure(), + FunctionType::new(type_row![QB], type_row![QB, QB]).into(), )?; let [q1] = f_build.input_wires_arr(); @@ -409,7 +409,7 @@ pub(crate) mod test { let (dfg_node, f_node) = { let mut f_build = module_builder.define_function( "main", - FunctionType::new(type_row![BIT], type_row![BIT]).pure(), + FunctionType::new(type_row![BIT], type_row![BIT]).into(), )?; let [i1] = f_build.input_wires_arr(); diff --git a/src/builder/module.rs b/src/builder/module.rs index 4de8e76b2..bcebc3c5d 100644 --- a/src/builder/module.rs +++ b/src/builder/module.rs @@ -217,7 +217,7 @@ mod test { vec![qubit_state_type.get_alias_type()], vec![qubit_state_type.get_alias_type()], ) - .pure(), + .into(), )?; n_identity(f_build)?; module_builder.finish_hugr(&EMPTY_REG) @@ -233,11 +233,11 @@ mod test { let mut f_build = module_builder.define_function( "main", - FunctionType::new(type_row![NAT], type_row![NAT]).pure(), + FunctionType::new(type_row![NAT], type_row![NAT]).into(), )?; let local_build = f_build.define_function( "local", - FunctionType::new(type_row![NAT], type_row![NAT]).pure(), + FunctionType::new(type_row![NAT], type_row![NAT]).into(), )?; let [wire] = local_build.input_wires_arr(); let f_id = local_build.finish_with_outputs([wire])?; diff --git a/src/builder/tail_loop.rs b/src/builder/tail_loop.rs index 5eb8286e1..a1eb84348 100644 --- a/src/builder/tail_loop.rs +++ b/src/builder/tail_loop.rs @@ -131,10 +131,21 @@ mod test { let mut fbuild = module_builder.define_function( "main", FunctionType::new(type_row![BIT], type_row![NAT]) - .with_input_extensions(ExtensionSet::singleton(&PRELUDE_ID)), + // ALAN note these were the input-extensions before + .with_extension_delta(&ExtensionSet::singleton(&PRELUDE_ID)) + .into(), )?; let _fdef = { - let [b1] = fbuild.input_wires_arr(); + // ALAN added a lift here + let [b1] = fbuild + .add_dataflow_op( + ops::LeafOp::Lift { + type_row: type_row![BIT], + new_extension: PRELUDE_ID, + }, + fbuild.input_wires(), + )? + .outputs_arr(); let loop_id = { let mut loop_b = fbuild.tail_loop_builder(vec![(BIT, b1)], vec![], type_row![NAT])?; diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 179ea486d..495cb430a 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -361,7 +361,7 @@ mod test { let mut fbuild = module_builder .define_function( "main", - FunctionType::new(type_row![USIZE_T], type_row![USIZE_T]).pure(), + FunctionType::new(type_row![USIZE_T], type_row![USIZE_T]).into(), ) .unwrap(); let [i1] = fbuild.input_wires_arr(); diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index 4a376604d..1491e73ed 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -257,7 +257,7 @@ pub(in crate::hugr::rewrite) mod test { let _f_id = { let mut func_builder = module_builder.define_function( "main", - FunctionType::new(type_row![QB, QB, QB], type_row![QB, QB, QB]).pure(), + FunctionType::new(type_row![QB, QB, QB], type_row![QB, QB, QB]).into(), )?; let [qb0, qb1, qb2] = func_builder.input_wires_arr(); diff --git a/src/hugr/serialize.rs b/src/hugr/serialize.rs index a96d1abca..b3f527a06 100644 --- a/src/hugr/serialize.rs +++ b/src/hugr/serialize.rs @@ -364,7 +364,7 @@ pub mod test { let t_row = vec![Type::new_sum(vec![NAT, QB])]; let mut f_build = module_builder - .define_function("main", FunctionType::new(t_row.clone(), t_row).pure()) + .define_function("main", FunctionType::new(t_row.clone(), t_row).into()) .unwrap(); let outputs = f_build @@ -399,7 +399,7 @@ pub mod test { let mut module_builder = ModuleBuilder::new(); let t_row = vec![Type::new_sum(vec![NAT, QB])]; let mut f_build = module_builder - .define_function("main", FunctionType::new(t_row.clone(), t_row).pure()) + .define_function("main", FunctionType::new(t_row.clone(), t_row).into()) .unwrap(); let outputs = f_build diff --git a/src/hugr/validate/test.rs b/src/hugr/validate/test.rs index 7eec5015d..07ce72aa3 100644 --- a/src/hugr/validate/test.rs +++ b/src/hugr/validate/test.rs @@ -443,7 +443,7 @@ fn missing_lift_node() -> Result<(), BuildError> { let mut module_builder = ModuleBuilder::new(); let mut main = module_builder.define_function( "main", - FunctionType::new(type_row![NAT], type_row![NAT]).pure(), + FunctionType::new(type_row![NAT], type_row![NAT]).into(), )?; let [main_input] = main.input_wires_arr(); @@ -479,7 +479,7 @@ fn missing_lift_node() -> Result<(), BuildError> { fn too_many_extension() -> Result<(), BuildError> { let mut module_builder = ModuleBuilder::new(); - let main_sig = FunctionType::new(type_row![NAT], type_row![NAT]).pure(); + let main_sig = FunctionType::new(type_row![NAT], type_row![NAT]).into(); let mut main = module_builder.define_function("main", main_sig)?; let [main_input] = main.input_wires_arr(); @@ -519,7 +519,7 @@ fn extensions_mismatch() -> Result<(), BuildError> { let main_sig = FunctionType::new(type_row![], type_row![NAT]) .with_extension_delta(&all_rs) - .with_input_extensions(ExtensionSet::new()); + .into(); let mut main = module_builder.define_function("main", main_sig)?; diff --git a/src/hugr/views/descendants.rs b/src/hugr/views/descendants.rs index 5cfdbd34b..f0c515457 100644 --- a/src/hugr/views/descendants.rs +++ b/src/hugr/views/descendants.rs @@ -222,7 +222,7 @@ pub(super) mod test { let (f_id, inner_id) = { let mut func_builder = module_builder.define_function( "main", - FunctionType::new(type_row![NAT, QB], type_row![NAT, QB]).pure(), + FunctionType::new(type_row![NAT, QB], type_row![NAT, QB]).into(), )?; let [int, qb] = func_builder.input_wires_arr(); diff --git a/src/hugr/views/sibling.rs b/src/hugr/views/sibling.rs index 6426dcefe..fec31036f 100644 --- a/src/hugr/views/sibling.rs +++ b/src/hugr/views/sibling.rs @@ -401,7 +401,7 @@ mod test { fn nested_flat() -> Result<(), Box> { let mut module_builder = ModuleBuilder::new(); let fty = FunctionType::new(type_row![NAT], type_row![NAT]); - let mut fbuild = module_builder.define_function("main", fty.clone().pure())?; + let mut fbuild = module_builder.define_function("main", fty.clone().into())?; let dfg = fbuild.dfg_builder(fty, None, fbuild.input_wires())?; let ins = dfg.input_wires(); let sub_dfg = dfg.finish_with_outputs(ins)?; From 3078259d024b0c3fa3d184079084cfbfe2e0726d Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 21:52:11 +0000 Subject: [PATCH 03/26] extract_subgraph and FunctionBuilder::new take PolyFuncType not Signature --- src/builder/dataflow.rs | 21 ++++++++------------- src/hugr/views/sibling_subgraph.rs | 15 ++++----------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/builder/dataflow.rs b/src/builder/dataflow.rs index e9d55b8c1..1db768b52 100644 --- a/src/builder/dataflow.rs +++ b/src/builder/dataflow.rs @@ -7,7 +7,7 @@ use std::marker::PhantomData; use crate::hugr::{HugrView, NodeType, ValidationError}; use crate::ops; -use crate::types::{FunctionType, Signature}; +use crate::types::{FunctionType, PolyFuncType}; use crate::extension::{ExtensionRegistry, ExtensionSet}; use crate::Node; @@ -146,22 +146,17 @@ impl FunctionBuilder { /// # Errors /// /// Error in adding DFG child nodes. - pub fn new(name: impl Into, signature: Signature) -> Result { - // ALAN this looks dodgy too + pub fn new(name: impl Into, signature: PolyFuncType) -> Result { + let body = signature.body.clone(); let op = ops::FuncDefn { - signature: signature.signature.clone().into(), + signature, name: name.into(), }; - let base = Hugr::new(NodeType::new(op, signature.input_extensions.clone())); + let base = Hugr::new(NodeType::new_pure(op)); let root = base.root(); - let db = DFGBuilder::create_with_io( - base, - root, - signature.signature, - Some(signature.input_extensions), - )?; + let db = DFGBuilder::create_with_io(base, root, body, Some(ExtensionSet::new()))?; Ok(Self::from_dfg_builder(db)) } } @@ -341,7 +336,7 @@ pub(crate) mod test { let builder = || -> Result { let mut f_build = FunctionBuilder::new( "main", - FunctionType::new(type_row![BIT], type_row![BIT]).pure(), + FunctionType::new(type_row![BIT], type_row![BIT]).into(), )?; let [i1] = f_build.input_wires_arr(); @@ -365,7 +360,7 @@ pub(crate) mod test { fn error_on_linear_inter_graph_edge() -> Result<(), BuildError> { let mut f_build = FunctionBuilder::new( "main", - FunctionType::new(type_row![QB], type_row![QB]).pure(), + FunctionType::new(type_row![QB], type_row![QB]).into(), )?; let [i1] = f_build.input_wires_arr(); diff --git a/src/hugr/views/sibling_subgraph.rs b/src/hugr/views/sibling_subgraph.rs index 616b37ce0..c209f43bf 100644 --- a/src/hugr/views/sibling_subgraph.rs +++ b/src/hugr/views/sibling_subgraph.rs @@ -17,11 +17,9 @@ use portgraph::{view::Subgraph, Direction, PortView}; use thiserror::Error; use crate::builder::{Container, FunctionBuilder}; -use crate::extension::ExtensionSet; use crate::hugr::{HugrError, HugrMut, HugrView, RootTagged}; use crate::ops::handle::{ContainerHandle, DataflowOpID}; use crate::ops::{OpTag, OpTrait}; -use crate::types::Signature; use crate::types::{FunctionType, Type}; use crate::{Hugr, IncomingPort, Node, OutgoingPort, Port, SimpleReplacement}; @@ -400,19 +398,14 @@ impl SiblingSubgraph { /// Create a new Hugr containing only the subgraph. /// - /// The new Hugr will contain a function root wth the same signature as the - /// subgraph and the specified `input_extensions`. + /// The new Hugr will contain a [FuncDefn] root wth the same signature as the + /// subgraph and the specified `name` pub fn extract_subgraph( &self, hugr: &impl HugrView, name: impl Into, - input_extensions: ExtensionSet, ) -> Result { - let signature = Signature { - signature: self.signature(hugr), - input_extensions, - }; - let mut builder = FunctionBuilder::new(name, signature).unwrap(); + let mut builder = FunctionBuilder::new(name, self.signature(hugr).into()).unwrap(); // Take the unfinished Hugr from the builder, to avoid unnecessary // validation checks that require connecting the inputs and outputs. let mut extracted = mem::take(builder.hugr_mut()); @@ -971,7 +964,7 @@ mod tests { let func_graph: SiblingGraph<'_, FuncID> = SiblingGraph::try_new(&hugr, func_root).unwrap(); let subgraph = SiblingSubgraph::try_new_dataflow_subgraph(&func_graph).unwrap(); - let extracted = subgraph.extract_subgraph(&hugr, "region", ExtensionSet::new())?; + let extracted = subgraph.extract_subgraph(&hugr, "region")?; extracted.validate(&PRELUDE_REGISTRY).unwrap(); From 271a15afed3c5d040ab3cd86d7eb60523fe19dcd Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 21:55:52 +0000 Subject: [PATCH 04/26] believe define_declaration is correct, but refactor a bit --- src/builder/module.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/builder/module.rs b/src/builder/module.rs index bcebc3c5d..73006b581 100644 --- a/src/builder/module.rs +++ b/src/builder/module.rs @@ -78,25 +78,23 @@ impl + AsRef> ModuleBuilder { f_id: &FuncID, ) -> Result, BuildError> { let f_node = f_id.node(); - let (signature, name) = if let OpType::FuncDecl(ops::FuncDecl { signature, name }) = + let OpType::FuncDecl(ops::FuncDecl { signature, name }) = self.hugr().get_optype(f_node) - { - (signature.clone(), name.clone()) - } else { + else { return Err(BuildError::UnexpectedType { node: f_node, op_desc: "OpType::FuncDecl", }); }; + let signature = signature.clone(); + let name = name.clone(); + let body = signature.body.clone(); self.hugr_mut().replace_op( f_node, - NodeType::new_pure(ops::FuncDefn { - name, - signature: signature.clone(), - }), + NodeType::new_pure(ops::FuncDefn { name, signature }), )?; - let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, signature.body, None)?; // ALAN + let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body, None)?; Ok(FunctionBuilder::from_dfg_builder(db)) } From b7811554cc47bd07ab654a5c603f5ecf68c7c79e Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 21:57:56 +0000 Subject: [PATCH 05/26] declare (FuncDecl) takes PolyFuncType --- src/builder/build_traits.rs | 2 +- src/builder/module.rs | 20 +++++++------------- src/hugr/views/sibling_subgraph.rs | 6 +++--- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/builder/build_traits.rs b/src/builder/build_traits.rs index 1ab9b8236..8c314294d 100644 --- a/src/builder/build_traits.rs +++ b/src/builder/build_traits.rs @@ -96,7 +96,7 @@ pub trait Container { let f_node = self.add_child_node(NodeType::new( ops::FuncDefn { name: name.into(), - signature: signature, + signature, }, ExtensionSet::new(), ))?; diff --git a/src/builder/module.rs b/src/builder/module.rs index 73006b581..c3bb1c3d5 100644 --- a/src/builder/module.rs +++ b/src/builder/module.rs @@ -8,14 +8,12 @@ use crate::{ extension::ExtensionRegistry, hugr::{hugrmut::sealed::HugrMutInternals, views::HugrView, ValidationError}, ops, - types::{Type, TypeBound}, + types::{PolyFuncType, Type, TypeBound}, }; use crate::ops::handle::{AliasID, FuncID, NodeHandle}; use crate::ops::OpType; -use crate::types::Signature; - use crate::Node; use smol_str::SmolStr; @@ -107,17 +105,13 @@ impl + AsRef> ModuleBuilder { pub fn declare( &mut self, name: impl Into, - signature: Signature, // ALAN + signature: PolyFuncType, ) -> Result, BuildError> { // TODO add param names to metadata - let rs = signature.input_extensions.clone(); - let declare_n = self.add_child_node(NodeType::new( - ops::FuncDecl { - signature: signature.signature.into(), - name: name.into(), - }, - rs, - ))?; + let declare_n = self.add_child_node(NodeType::new_pure(ops::FuncDecl { + signature, + name: name.into(), + }))?; Ok(declare_n.into()) } @@ -188,7 +182,7 @@ mod test { let f_id = module_builder.declare( "main", - FunctionType::new(type_row![NAT], type_row![NAT]).pure(), + FunctionType::new(type_row![NAT], type_row![NAT]).into(), )?; let mut f_build = module_builder.define_declaration(&f_id)?; diff --git a/src/hugr/views/sibling_subgraph.rs b/src/hugr/views/sibling_subgraph.rs index c209f43bf..8e2280f0e 100644 --- a/src/hugr/views/sibling_subgraph.rs +++ b/src/hugr/views/sibling_subgraph.rs @@ -724,7 +724,7 @@ mod tests { let mut mod_builder = ModuleBuilder::new(); let func = mod_builder.declare( "test", - FunctionType::new_linear(type_row![QB_T, QB_T, QB_T]).pure(), + FunctionType::new_linear(type_row![QB_T, QB_T, QB_T]).into(), )?; let func_id = { let mut dfg = mod_builder.define_declaration(&func)?; @@ -741,7 +741,7 @@ mod tests { fn build_3not_hugr() -> Result<(Hugr, Node), BuildError> { let mut mod_builder = ModuleBuilder::new(); let func = - mod_builder.declare("test", FunctionType::new_linear(type_row![BOOL_T]).pure())?; + mod_builder.declare("test", FunctionType::new_linear(type_row![BOOL_T]).into())?; let func_id = { let mut dfg = mod_builder.define_declaration(&func)?; let outs1 = dfg.add_dataflow_op(not_op(), dfg.input_wires())?; @@ -760,7 +760,7 @@ mod tests { let mut mod_builder = ModuleBuilder::new(); let func = mod_builder.declare( "test", - FunctionType::new(type_row![BOOL_T], type_row![BOOL_T]).pure(), + FunctionType::new(type_row![BOOL_T], type_row![BOOL_T]).into(), )?; let func_id = { let mut dfg = mod_builder.define_declaration(&func)?; From c56e8c2a8f1ad0129fe34093970a83ecdfb2730f Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 22:07:47 +0000 Subject: [PATCH 06/26] builder fn call takes TypeArgs - and ExtensionRegistry, ugh --- src/builder/build_traits.rs | 15 ++++++++++++--- src/builder/module.rs | 7 ++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/builder/build_traits.rs b/src/builder/build_traits.rs index 8c314294d..8f5341b08 100644 --- a/src/builder/build_traits.rs +++ b/src/builder/build_traits.rs @@ -20,7 +20,7 @@ use crate::{ }; use crate::extension::{ExtensionRegistry, ExtensionSet, PRELUDE_REGISTRY}; -use crate::types::{FunctionType, PolyFuncType, Type, TypeRow}; +use crate::types::{FunctionType, PolyFuncType, Type, TypeArg, TypeRow}; use itertools::Itertools; @@ -595,7 +595,9 @@ pub trait Dataflow: Container { fn call( &mut self, function: &FuncID, + type_args: &[TypeArg], input_wires: impl IntoIterator, + exts: &ExtensionRegistry, // TODO remove? ) -> Result, BuildError> { let hugr = self.hugr(); let def_op = hugr.get_optype(function.node()); @@ -609,9 +611,16 @@ pub trait Dataflow: Container { }) } }; - let signature = type_scheme.body; + // TODO either some way of returning a SignatureError without a node (as its not + // constructed yet) - or, preferably, a way to "instantiate" the PolyFuncType without + // validating against an ExtensionRegistry. + let signature = type_scheme.instantiate(type_args, exts).map_err(|e| { + BuildError::InvalidHUGR(ValidationError::SignatureError { + node: function.node(), + cause: e, + }) + })?; let const_in_port = signature.output.len(); - // TODO ALAN this is totally broken for polymorphic functions. Need to TypeApply here. let op_id = self.add_dataflow_op(ops::Call { signature }, input_wires)?; let src_port = self.hugr_mut().num_outputs(function.node()) - 1; diff --git a/src/builder/module.rs b/src/builder/module.rs index c3bb1c3d5..92b9e396a 100644 --- a/src/builder/module.rs +++ b/src/builder/module.rs @@ -169,7 +169,7 @@ mod test { test::{n_identity, NAT}, Dataflow, DataflowSubContainer, }, - extension::EMPTY_REG, + extension::{EMPTY_REG, PRELUDE_REGISTRY}, type_row, types::FunctionType, }; @@ -186,7 +186,7 @@ mod test { )?; let mut f_build = module_builder.define_declaration(&f_id)?; - let call = f_build.call(&f_id, f_build.input_wires())?; + let call = f_build.call(&f_id, &[], f_build.input_wires(), &PRELUDE_REGISTRY)?; f_build.finish_with_outputs(call.outputs())?; module_builder.finish_prelude_hugr() @@ -234,7 +234,8 @@ mod test { let [wire] = local_build.input_wires_arr(); let f_id = local_build.finish_with_outputs([wire])?; - let call = f_build.call(f_id.handle(), f_build.input_wires())?; + let call = + f_build.call(f_id.handle(), &[], f_build.input_wires(), &PRELUDE_REGISTRY)?; f_build.finish_with_outputs(call.outputs())?; module_builder.finish_prelude_hugr() From 9fd164b5c7381caf849967075bf4f5c32df92d4a Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 22:46:33 +0000 Subject: [PATCH 07/26] validate_children: return early if no children --- src/hugr/validate.rs | 144 ++++++++++++++++++++++--------------------- 1 file changed, 74 insertions(+), 70 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index 0d0724379..af7255d8d 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -303,91 +303,95 @@ impl<'a, 'b> ValidationContext<'a, 'b> { let op_type = &node_type.op; let flags = op_type.validity_flags(); - if self.hugr.hierarchy.child_count(node.pg_index()) > 0 { - if flags.allowed_children.is_empty() { - return Err(ValidationError::NonContainerWithChildren { + if self.hugr.hierarchy.child_count(node.pg_index()) == 0 { + return if flags.requires_children { + Err(ValidationError::ContainerWithoutChildren { node, optype: op_type.clone(), - }); - } + }) + } else { + Ok(()) + }; + } - let all_children = self.hugr.children(node); - let mut first_two_children = all_children.clone().take(2); - let first_child = self.hugr.get_optype(first_two_children.next().unwrap()); - if !flags.allowed_first_child.is_superset(first_child.tag()) { - return Err(ValidationError::InvalidInitialChild { - parent: node, - parent_optype: op_type.clone(), - optype: first_child.clone(), - expected: flags.allowed_first_child, - position: "first", - }); - } + if flags.allowed_children.is_empty() { + return Err(ValidationError::NonContainerWithChildren { + node, + optype: op_type.clone(), + }); + } - if let Some(second_child) = first_two_children - .next() - .map(|child| self.hugr.get_optype(child)) - { - if !flags.allowed_second_child.is_superset(second_child.tag()) { - return Err(ValidationError::InvalidInitialChild { - parent: node, - parent_optype: op_type.clone(), - optype: second_child.clone(), - expected: flags.allowed_second_child, - position: "second", - }); - } - } - // Additional validations running over the full list of children optypes - let children_optypes = all_children.map(|c| (c.pg_index(), self.hugr.get_optype(c))); - if let Err(source) = op_type.validate_op_children(children_optypes) { - return Err(ValidationError::InvalidChildren { + let all_children = self.hugr.children(node); + let mut first_two_children = all_children.clone().take(2); + let first_child = self.hugr.get_optype(first_two_children.next().unwrap()); + if !flags.allowed_first_child.is_superset(first_child.tag()) { + return Err(ValidationError::InvalidInitialChild { + parent: node, + parent_optype: op_type.clone(), + optype: first_child.clone(), + expected: flags.allowed_first_child, + position: "first", + }); + } + + if let Some(second_child) = first_two_children + .next() + .map(|child| self.hugr.get_optype(child)) + { + if !flags.allowed_second_child.is_superset(second_child.tag()) { + return Err(ValidationError::InvalidInitialChild { parent: node, parent_optype: op_type.clone(), - source, + optype: second_child.clone(), + expected: flags.allowed_second_child, + position: "second", }); } + } + // Additional validations running over the full list of children optypes + let children_optypes = all_children.map(|c| (c.pg_index(), self.hugr.get_optype(c))); + if let Err(source) = op_type.validate_op_children(children_optypes) { + return Err(ValidationError::InvalidChildren { + parent: node, + parent_optype: op_type.clone(), + source, + }); + } - // Additional validations running over the edges of the contained graph - if let Some(edge_check) = flags.edge_check { - for source in self.hugr.hierarchy.children(node.pg_index()) { - for target in self.hugr.graph.output_neighbours(source) { - if self.hugr.hierarchy.parent(target) != Some(node.pg_index()) { - continue; - } - let source_op = self.hugr.get_optype(source.into()); - let target_op = self.hugr.get_optype(target.into()); - for (source_port, target_port) in - self.hugr.graph.get_connections(source, target) - { - let edge_data = ChildrenEdgeData { + // Additional validations running over the edges of the contained graph + if let Some(edge_check) = flags.edge_check { + for source in self.hugr.hierarchy.children(node.pg_index()) { + for target in self.hugr.graph.output_neighbours(source) { + if self.hugr.hierarchy.parent(target) != Some(node.pg_index()) { + continue; + } + let source_op = self.hugr.get_optype(source.into()); + let target_op = self.hugr.get_optype(target.into()); + for (source_port, target_port) in + self.hugr.graph.get_connections(source, target) + { + let edge_data = ChildrenEdgeData { + source, + target, + source_port: self.hugr.graph.port_offset(source_port).unwrap(), + target_port: self.hugr.graph.port_offset(target_port).unwrap(), + source_op: source_op.clone(), + target_op: target_op.clone(), + }; + if let Err(source) = edge_check(edge_data) { + return Err(ValidationError::InvalidEdges { + parent: node, + parent_optype: op_type.clone(), source, - target, - source_port: self.hugr.graph.port_offset(source_port).unwrap(), - target_port: self.hugr.graph.port_offset(target_port).unwrap(), - source_op: source_op.clone(), - target_op: target_op.clone(), - }; - if let Err(source) = edge_check(edge_data) { - return Err(ValidationError::InvalidEdges { - parent: node, - parent_optype: op_type.clone(), - source, - }); - } + }); } } } } + } - if flags.requires_dag { - self.validate_children_dag(node, op_type)?; - } - } else if flags.requires_children { - return Err(ValidationError::ContainerWithoutChildren { - node, - optype: op_type.clone(), - }); + if flags.requires_dag { + self.validate_children_dag(node, op_type)?; } Ok(()) From 78316a1191cc8a271fc168794c99963cf6c620fb Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 22:52:55 +0000 Subject: [PATCH 08/26] validate_children=>validate_subtree, breaking more tests (wrong choice among multiple Err's) --- src/hugr/validate.rs | 102 ++++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 41 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index af7255d8d..b656bdd76 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -21,12 +21,12 @@ use crate::extension::{ use crate::ops::custom::CustomOpError; use crate::ops::custom::{resolve_opaque_op, ExtensionOp, ExternalOp}; use crate::ops::validate::{ChildrenEdgeData, ChildrenValidationError, EdgeValidationError}; -use crate::ops::{OpTag, OpTrait, OpType, ValidateOp}; +use crate::ops::{FuncDecl, FuncDefn, OpTag, OpTrait, OpType, ValidateOp}; +use crate::types::type_param::TypeParam; use crate::types::{EdgeKind, Type}; use crate::{Direction, Hugr, Node, Port}; use super::views::{HierarchyView, HugrView, SiblingGraph}; -use super::NodeType; /// Structure keeping track of pre-computed information used in the validation /// process. @@ -93,6 +93,9 @@ impl<'a, 'b> ValidationContext<'a, 'b> { self.validate_node(node)?; } + // Hierarchy and children. No type variables declared outside the root. + self.validate_subtree(self.hugr.root(), &[])?; + Ok(()) } @@ -163,43 +166,6 @@ impl<'a, 'b> ValidationContext<'a, 'b> { } } - // Check operation-specific constraints. - // TODO make a separate method for this (perhaps producing Result<(), SignatureError>) - match op_type { - OpType::LeafOp(crate::ops::LeafOp::CustomOp(b)) => { - // Check TypeArgs are valid (in themselves, not necessarily wrt the TypeParams) - for arg in b.args() { - // Hugrs are monomorphic, so no type variables in scope - arg.validate(self.extension_registry, &[]) - .map_err(|cause| ValidationError::SignatureError { node, cause })?; - } - // Try to resolve serialized names to actual OpDefs in Extensions. - let e: Option; - let ext_op = match &**b { - ExternalOp::Opaque(op) => { - // If resolve_extension_ops has been called first, this would always return Ok(None) - e = resolve_opaque_op(node, op, self.extension_registry)?; - e.as_ref() - } - ExternalOp::Extension(ext) => Some(ext), - }; - // If successful, check TypeArgs are valid for the declared TypeParams - if let Some(ext_op) = ext_op { - ext_op - .def() - .check_args(ext_op.args()) - .map_err(|cause| ValidationError::SignatureError { node, cause })?; - } - } - OpType::LeafOp(crate::ops::LeafOp::TypeApply { ta }) => { - ta.validate(self.extension_registry) - .map_err(|cause| ValidationError::SignatureError { node, cause })?; - } - _ => (), - } - // Secondly that the node has correct children - self.validate_children(node, node_type)?; - // If this is a container with I/O nodes, check that the extension they // define match the extensions of the container. if let Some([input, output]) = self.hugr.get_io(node) { @@ -299,10 +265,48 @@ impl<'a, 'b> ValidationContext<'a, 'b> { /// Check operation-specific constraints. /// /// These are flags defined for each operation type as an [`OpValidityFlags`] object. - fn validate_children(&self, node: Node, node_type: &NodeType) -> Result<(), ValidationError> { + fn validate_subtree( + &mut self, + node: Node, + var_decls: &[TypeParam], + ) -> Result<(), ValidationError> { + let node_type = self.hugr.get_nodetype(node); let op_type = &node_type.op; let flags = op_type.validity_flags(); + // TODO consider turning this match into a trait method? + match op_type { + OpType::LeafOp(crate::ops::LeafOp::CustomOp(b)) => { + // Check TypeArgs are valid (in themselves, not necessarily wrt the TypeParams) + for arg in b.args() { + arg.validate(self.extension_registry, var_decls) + .map_err(|cause| ValidationError::SignatureError { node, cause })?; + } + // Try to resolve serialized names to actual OpDefs in Extensions. + let e: Option; + let ext_op = match &**b { + ExternalOp::Opaque(op) => { + // If resolve_extension_ops has been called first, this would always return Ok(None) + e = resolve_opaque_op(node, op, self.extension_registry)?; + e.as_ref() + } + ExternalOp::Extension(ext) => Some(ext), + }; + // If successful, check TypeArgs are valid for the declared TypeParams + if let Some(ext_op) = ext_op { + ext_op + .def() + .check_args(ext_op.args()) + .map_err(|cause| ValidationError::SignatureError { node, cause })?; + } + } + OpType::LeafOp(crate::ops::LeafOp::TypeApply { ta }) => { + ta.validate(self.extension_registry) + .map_err(|cause| ValidationError::SignatureError { node, cause })?; + } + _ => (), + } + if self.hugr.hierarchy.child_count(node.pg_index()) == 0 { return if flags.requires_children { Err(ValidationError::ContainerWithoutChildren { @@ -349,7 +353,9 @@ impl<'a, 'b> ValidationContext<'a, 'b> { } } // Additional validations running over the full list of children optypes - let children_optypes = all_children.map(|c| (c.pg_index(), self.hugr.get_optype(c))); + let children_optypes = all_children + .clone() + .map(|c| (c.pg_index(), self.hugr.get_optype(c))); if let Err(source) = op_type.validate_op_children(children_optypes) { return Err(ValidationError::InvalidChildren { parent: node, @@ -394,6 +400,20 @@ impl<'a, 'b> ValidationContext<'a, 'b> { self.validate_children_dag(node, op_type)?; } + let mut v: Vec; + let var_decls = if let OpType::FuncDefn(FuncDefn { signature, .. }) + | OpType::FuncDecl(FuncDecl { signature, .. }) = op_type + { + v = signature.params().to_owned(); + v.extend(var_decls.iter().cloned()); + v.as_ref() + } else { + var_decls + }; + + for node in all_children { + self.validate_subtree(node, var_decls)?; + } Ok(()) } From 9c3048a711b904409e044af3b87ed67d92e8c487 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 23:02:30 +0000 Subject: [PATCH 09/26] Separate validate_subtree --- src/hugr/validate.rs | 115 ++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 56 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index b656bdd76..ffa18fca9 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -27,6 +27,7 @@ use crate::types::{EdgeKind, Type}; use crate::{Direction, Hugr, Node, Port}; use super::views::{HierarchyView, HugrView, SiblingGraph}; +use super::NodeType; /// Structure keeping track of pre-computed information used in the validation /// process. @@ -166,6 +167,9 @@ impl<'a, 'b> ValidationContext<'a, 'b> { } } + // Secondly that the node has correct children + self.validate_children(node, node_type)?; + // If this is a container with I/O nodes, check that the extension they // define match the extensions of the container. if let Some([input, output]) = self.hugr.get_io(node) { @@ -265,48 +269,10 @@ impl<'a, 'b> ValidationContext<'a, 'b> { /// Check operation-specific constraints. /// /// These are flags defined for each operation type as an [`OpValidityFlags`] object. - fn validate_subtree( - &mut self, - node: Node, - var_decls: &[TypeParam], - ) -> Result<(), ValidationError> { - let node_type = self.hugr.get_nodetype(node); + fn validate_children(&self, node: Node, node_type: &NodeType) -> Result<(), ValidationError> { let op_type = &node_type.op; let flags = op_type.validity_flags(); - // TODO consider turning this match into a trait method? - match op_type { - OpType::LeafOp(crate::ops::LeafOp::CustomOp(b)) => { - // Check TypeArgs are valid (in themselves, not necessarily wrt the TypeParams) - for arg in b.args() { - arg.validate(self.extension_registry, var_decls) - .map_err(|cause| ValidationError::SignatureError { node, cause })?; - } - // Try to resolve serialized names to actual OpDefs in Extensions. - let e: Option; - let ext_op = match &**b { - ExternalOp::Opaque(op) => { - // If resolve_extension_ops has been called first, this would always return Ok(None) - e = resolve_opaque_op(node, op, self.extension_registry)?; - e.as_ref() - } - ExternalOp::Extension(ext) => Some(ext), - }; - // If successful, check TypeArgs are valid for the declared TypeParams - if let Some(ext_op) = ext_op { - ext_op - .def() - .check_args(ext_op.args()) - .map_err(|cause| ValidationError::SignatureError { node, cause })?; - } - } - OpType::LeafOp(crate::ops::LeafOp::TypeApply { ta }) => { - ta.validate(self.extension_registry) - .map_err(|cause| ValidationError::SignatureError { node, cause })?; - } - _ => (), - } - if self.hugr.hierarchy.child_count(node.pg_index()) == 0 { return if flags.requires_children { Err(ValidationError::ContainerWithoutChildren { @@ -353,9 +319,7 @@ impl<'a, 'b> ValidationContext<'a, 'b> { } } // Additional validations running over the full list of children optypes - let children_optypes = all_children - .clone() - .map(|c| (c.pg_index(), self.hugr.get_optype(c))); + let children_optypes = all_children.map(|c| (c.pg_index(), self.hugr.get_optype(c))); if let Err(source) = op_type.validate_op_children(children_optypes) { return Err(ValidationError::InvalidChildren { parent: node, @@ -400,20 +364,6 @@ impl<'a, 'b> ValidationContext<'a, 'b> { self.validate_children_dag(node, op_type)?; } - let mut v: Vec; - let var_decls = if let OpType::FuncDefn(FuncDefn { signature, .. }) - | OpType::FuncDecl(FuncDecl { signature, .. }) = op_type - { - v = signature.params().to_owned(); - v.extend(var_decls.iter().cloned()); - v.as_ref() - } else { - var_decls - }; - - for node in all_children { - self.validate_subtree(node, var_decls)?; - } Ok(()) } @@ -578,6 +528,59 @@ impl<'a, 'b> ValidationContext<'a, 'b> { } .into()) } + + fn validate_subtree(&self, node: Node, var_decls: &[TypeParam]) -> Result<(), ValidationError> { + let op_type = self.hugr.get_optype(node); + // TODO consider turning this match into a trait method? + match op_type { + OpType::LeafOp(crate::ops::LeafOp::CustomOp(b)) => { + // Check TypeArgs are valid (in themselves, not necessarily wrt the TypeParams) + for arg in b.args() { + arg.validate(self.extension_registry, var_decls) + .map_err(|cause| ValidationError::SignatureError { node, cause })?; + } + // Try to resolve serialized names to actual OpDefs in Extensions. + let e: Option; + let ext_op = match &**b { + ExternalOp::Opaque(op) => { + // If resolve_extension_ops has been called first, this would always return Ok(None) + e = resolve_opaque_op(node, op, self.extension_registry)?; + e.as_ref() + } + ExternalOp::Extension(ext) => Some(ext), + }; + // If successful, check TypeArgs are valid for the declared TypeParams + if let Some(ext_op) = ext_op { + ext_op + .def() + .check_args(ext_op.args()) + .map_err(|cause| ValidationError::SignatureError { node, cause })?; + } + } + OpType::LeafOp(crate::ops::LeafOp::TypeApply { ta }) => { + ta.validate(self.extension_registry) + .map_err(|cause| ValidationError::SignatureError { node, cause })?; + } + _ => (), + } + + let mut v: Vec; + let var_decls = if let OpType::FuncDefn(FuncDefn { signature, .. }) + | OpType::FuncDecl(FuncDecl { signature, .. }) = op_type + { + v = signature.params().to_owned(); + v.extend(var_decls.iter().cloned()); + v.as_ref() + } else { + var_decls + }; + + for child in self.hugr.children(node) { + self.validate_subtree(child, var_decls)?; + } + + Ok(()) + } } /// Errors that can occur while validating a Hugr. From 9cb43a1552ea4be9a206656aa34cacdae06c9a48 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 23:04:04 +0000 Subject: [PATCH 10/26] Revert "validate_children: return early if no children" This reverts commit 9fd164b5c7381caf849967075bf4f5c32df92d4a. --- src/hugr/validate.rs | 144 +++++++++++++++++++++---------------------- 1 file changed, 70 insertions(+), 74 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index ffa18fca9..69a44d5d3 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -273,95 +273,91 @@ impl<'a, 'b> ValidationContext<'a, 'b> { let op_type = &node_type.op; let flags = op_type.validity_flags(); - if self.hugr.hierarchy.child_count(node.pg_index()) == 0 { - return if flags.requires_children { - Err(ValidationError::ContainerWithoutChildren { + if self.hugr.hierarchy.child_count(node.pg_index()) > 0 { + if flags.allowed_children.is_empty() { + return Err(ValidationError::NonContainerWithChildren { node, optype: op_type.clone(), - }) - } else { - Ok(()) - }; - } - - if flags.allowed_children.is_empty() { - return Err(ValidationError::NonContainerWithChildren { - node, - optype: op_type.clone(), - }); - } - - let all_children = self.hugr.children(node); - let mut first_two_children = all_children.clone().take(2); - let first_child = self.hugr.get_optype(first_two_children.next().unwrap()); - if !flags.allowed_first_child.is_superset(first_child.tag()) { - return Err(ValidationError::InvalidInitialChild { - parent: node, - parent_optype: op_type.clone(), - optype: first_child.clone(), - expected: flags.allowed_first_child, - position: "first", - }); - } + }); + } - if let Some(second_child) = first_two_children - .next() - .map(|child| self.hugr.get_optype(child)) - { - if !flags.allowed_second_child.is_superset(second_child.tag()) { + let all_children = self.hugr.children(node); + let mut first_two_children = all_children.clone().take(2); + let first_child = self.hugr.get_optype(first_two_children.next().unwrap()); + if !flags.allowed_first_child.is_superset(first_child.tag()) { return Err(ValidationError::InvalidInitialChild { parent: node, parent_optype: op_type.clone(), - optype: second_child.clone(), - expected: flags.allowed_second_child, - position: "second", + optype: first_child.clone(), + expected: flags.allowed_first_child, + position: "first", }); } - } - // Additional validations running over the full list of children optypes - let children_optypes = all_children.map(|c| (c.pg_index(), self.hugr.get_optype(c))); - if let Err(source) = op_type.validate_op_children(children_optypes) { - return Err(ValidationError::InvalidChildren { - parent: node, - parent_optype: op_type.clone(), - source, - }); - } - // Additional validations running over the edges of the contained graph - if let Some(edge_check) = flags.edge_check { - for source in self.hugr.hierarchy.children(node.pg_index()) { - for target in self.hugr.graph.output_neighbours(source) { - if self.hugr.hierarchy.parent(target) != Some(node.pg_index()) { - continue; - } - let source_op = self.hugr.get_optype(source.into()); - let target_op = self.hugr.get_optype(target.into()); - for (source_port, target_port) in - self.hugr.graph.get_connections(source, target) - { - let edge_data = ChildrenEdgeData { - source, - target, - source_port: self.hugr.graph.port_offset(source_port).unwrap(), - target_port: self.hugr.graph.port_offset(target_port).unwrap(), - source_op: source_op.clone(), - target_op: target_op.clone(), - }; - if let Err(source) = edge_check(edge_data) { - return Err(ValidationError::InvalidEdges { - parent: node, - parent_optype: op_type.clone(), + if let Some(second_child) = first_two_children + .next() + .map(|child| self.hugr.get_optype(child)) + { + if !flags.allowed_second_child.is_superset(second_child.tag()) { + return Err(ValidationError::InvalidInitialChild { + parent: node, + parent_optype: op_type.clone(), + optype: second_child.clone(), + expected: flags.allowed_second_child, + position: "second", + }); + } + } + // Additional validations running over the full list of children optypes + let children_optypes = all_children.map(|c| (c.pg_index(), self.hugr.get_optype(c))); + if let Err(source) = op_type.validate_op_children(children_optypes) { + return Err(ValidationError::InvalidChildren { + parent: node, + parent_optype: op_type.clone(), + source, + }); + } + + // Additional validations running over the edges of the contained graph + if let Some(edge_check) = flags.edge_check { + for source in self.hugr.hierarchy.children(node.pg_index()) { + for target in self.hugr.graph.output_neighbours(source) { + if self.hugr.hierarchy.parent(target) != Some(node.pg_index()) { + continue; + } + let source_op = self.hugr.get_optype(source.into()); + let target_op = self.hugr.get_optype(target.into()); + for (source_port, target_port) in + self.hugr.graph.get_connections(source, target) + { + let edge_data = ChildrenEdgeData { source, - }); + target, + source_port: self.hugr.graph.port_offset(source_port).unwrap(), + target_port: self.hugr.graph.port_offset(target_port).unwrap(), + source_op: source_op.clone(), + target_op: target_op.clone(), + }; + if let Err(source) = edge_check(edge_data) { + return Err(ValidationError::InvalidEdges { + parent: node, + parent_optype: op_type.clone(), + source, + }); + } } } } } - } - if flags.requires_dag { - self.validate_children_dag(node, op_type)?; + if flags.requires_dag { + self.validate_children_dag(node, op_type)?; + } + } else if flags.requires_children { + return Err(ValidationError::ContainerWithoutChildren { + node, + optype: op_type.clone(), + }); } Ok(()) From 41f0970948d57760db628ec2b8e18e7a34c5b071 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 13 Nov 2023 23:08:25 +0000 Subject: [PATCH 11/26] Properly check that args to binary compute_signature have no type vars --- src/extension/op_def.rs | 32 +++++++++++++++++++++++++++++--- src/hugr/validate.rs | 29 ++++++++++++++++------------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 60916914a..cfb5c6b5c 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -160,9 +160,35 @@ impl TypeParametrised for OpDef { } impl OpDef { - /// Check provided type arguments are valid against parameters. - pub fn check_args(&self, args: &[TypeArg]) -> Result<(), SignatureError> { - self.check_args_impl(args) + /// Check provided type arguments are valid against [ExtensionRegistry], + /// against parameters, and that no type variables are used as static arguments + /// (to [compute_signature][CustomSignatureFunc::compute_signature]) + pub fn validate_args( + &self, + args: &[TypeArg], + exts: &ExtensionRegistry, + var_decls: &[TypeParam], + ) -> Result<(), SignatureError> { + let temp: PolyFuncType; // to keep alive + let (pf, args) = match &self.signature_func { + SignatureFunc::TypeScheme(ts) => (ts, args), + SignatureFunc::CustomFunc { + static_params, + func, + } => { + let (static_args, other_args) = args.split_at(min(static_params.len(), args.len())); + static_args + .iter() + .try_for_each(|ta| ta.validate(exts, &[]))?; + check_type_args(static_args, static_params)?; + temp = func.compute_signature(&self.name, static_args, &self.misc, exts)?; + (&temp, other_args) + } + }; + args.iter() + .try_for_each(|ta| ta.validate(exts, var_decls))?; + check_type_args(args, pf.params())?; + Ok(()) } /// Computes the signature of a node, i.e. an instantiation of this diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index 69a44d5d3..95660dbc6 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::iter; -use itertools::Itertools; +use itertools::{Either, Itertools}; use petgraph::algo::dominators::{self, Dominators}; use petgraph::visit::{Topo, Walker}; use portgraph::{LinkView, PortView}; @@ -530,27 +530,30 @@ impl<'a, 'b> ValidationContext<'a, 'b> { // TODO consider turning this match into a trait method? match op_type { OpType::LeafOp(crate::ops::LeafOp::CustomOp(b)) => { - // Check TypeArgs are valid (in themselves, not necessarily wrt the TypeParams) - for arg in b.args() { - arg.validate(self.extension_registry, var_decls) - .map_err(|cause| ValidationError::SignatureError { node, cause })?; - } // Try to resolve serialized names to actual OpDefs in Extensions. let e: Option; - let ext_op = match &**b { + let ext_or_opaq = match &**b { ExternalOp::Opaque(op) => { // If resolve_extension_ops has been called first, this would always return Ok(None) e = resolve_opaque_op(node, op, self.extension_registry)?; - e.as_ref() + e.as_ref().map(Either::Left).unwrap_or(Either::Right(op)) } - ExternalOp::Extension(ext) => Some(ext), + ExternalOp::Extension(ext) => Either::Left(ext), }; // If successful, check TypeArgs are valid for the declared TypeParams - if let Some(ext_op) = ext_op { - ext_op + match ext_or_opaq { + Either::Left(exten) => exten .def() - .check_args(ext_op.args()) - .map_err(|cause| ValidationError::SignatureError { node, cause })?; + .validate_args(exten.args(), self.extension_registry, var_decls) + .map_err(|cause| ValidationError::SignatureError { node, cause })?, + Either::Right(opaq) => { + // Best effort. Just check TypeArgs are valid (in themselves, not necessarily wrt the TypeParams) + // and assuming none are binary params (all may contain type vars) + for arg in opaq.args() { + arg.validate(self.extension_registry, var_decls) + .map_err(|cause| ValidationError::SignatureError { node, cause })?; + } + } } } OpType::LeafOp(crate::ops::LeafOp::TypeApply { ta }) => { From 793a00c55f560b7bae3f7735d5b97c9d56c85d0f Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 14 Nov 2023 00:01:08 +0000 Subject: [PATCH 12/26] Don't implement TypeParametrised for OpDef --- src/extension/op_def.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index cfb5c6b5c..cdb0ceb3a 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -8,10 +8,8 @@ use smol_str::SmolStr; use super::{ Extension, ExtensionBuildError, ExtensionId, ExtensionRegistry, ExtensionSet, SignatureError, - TypeParametrised, }; -use crate::ops::custom::OpaqueOp; use crate::types::type_param::{check_type_args, TypeArg, TypeParam}; use crate::types::{FunctionType, PolyFuncType}; use crate::Hugr; @@ -143,22 +141,6 @@ pub struct OpDef { lower_funcs: Vec, } -impl TypeParametrised for OpDef { - type Concrete = OpaqueOp; - - fn params(&self) -> &[TypeParam] { - self.params() - } - - fn name(&self) -> &SmolStr { - self.name() - } - - fn extension(&self) -> &ExtensionId { - self.extension() - } -} - impl OpDef { /// Check provided type arguments are valid against [ExtensionRegistry], /// against parameters, and that no type variables are used as static arguments From c55019cd62659cbe3985225a4bcaa27095e34d5e Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 14 Nov 2023 19:59:46 +0000 Subject: [PATCH 13/26] Remove comments - those were correct changes --- src/builder/tail_loop.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/builder/tail_loop.rs b/src/builder/tail_loop.rs index a1eb84348..c400c4d65 100644 --- a/src/builder/tail_loop.rs +++ b/src/builder/tail_loop.rs @@ -131,12 +131,10 @@ mod test { let mut fbuild = module_builder.define_function( "main", FunctionType::new(type_row![BIT], type_row![NAT]) - // ALAN note these were the input-extensions before .with_extension_delta(&ExtensionSet::singleton(&PRELUDE_ID)) .into(), )?; let _fdef = { - // ALAN added a lift here let [b1] = fbuild .add_dataflow_op( ops::LeafOp::Lift { From 6f5526f302902c8ee6779e2c86725f3c5d6a84be Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 14 Nov 2023 20:26:12 +0000 Subject: [PATCH 14/26] fix doclink --- src/hugr/views/sibling_subgraph.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hugr/views/sibling_subgraph.rs b/src/hugr/views/sibling_subgraph.rs index 8e2280f0e..8588865ea 100644 --- a/src/hugr/views/sibling_subgraph.rs +++ b/src/hugr/views/sibling_subgraph.rs @@ -398,8 +398,8 @@ impl SiblingSubgraph { /// Create a new Hugr containing only the subgraph. /// - /// The new Hugr will contain a [FuncDefn] root wth the same signature as the - /// subgraph and the specified `name` + /// The new Hugr will contain a [FuncDefn][crate::ops::FuncDefn] root + /// wth the same signature as the subgraph and the specified `name` pub fn extract_subgraph( &self, hugr: &impl HugrView, From 9a656d9e02b13c33d2707daf435e373f98a6ec04 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 15 Nov 2023 22:53:57 +0000 Subject: [PATCH 15/26] Add PolyFuncType::new, make validate public --- src/types/poly_func.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/types/poly_func.rs b/src/types/poly_func.rs index d11bc882d..d4d8c206e 100644 --- a/src/types/poly_func.rs +++ b/src/types/poly_func.rs @@ -49,8 +49,8 @@ impl PolyFuncType { &self.params } - /// Create a new PolyFuncType and validates it. (This will only succeed - /// for outermost PolyFuncTypes i.e. with no free type-variables.) + /// Create a new PolyFuncType and validates it, assuming it has no free + /// type variables (from any enclosing scope). /// The [ExtensionRegistry] should be the same (or a subset) of that which will later /// be used to validate the Hugr; at this point we only need the types. /// @@ -62,12 +62,25 @@ impl PolyFuncType { body: FunctionType, extension_registry: &ExtensionRegistry, ) -> Result { - let params = params.into(); - body.validate(extension_registry, ¶ms)?; - Ok(Self { params, body }) + let res = Self::new(params, body); + res.validate(extension_registry, &[])?; + Ok(res) } - pub(super) fn validate( + /// Create a new PolyFuncType given the kinds of the variables it declares + /// and the underlying [FunctionType]. + pub fn new(params: impl Into>, body: FunctionType) -> Self { + Self { + params: params.into(), + body, + } + } + + /// Validates this instance against an ExtensionRegistry - checking that + /// all type bounds are wellformed instances of declared types - and + /// that all variables are declared (perhaps including those from an + /// enclosing scope, whose kinds are provided). + pub fn validate( &self, reg: &ExtensionRegistry, external_var_decls: &[TypeParam], From 293b663f0efe7c0a75986e2b3d976a33672afddb Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Thu, 16 Nov 2023 17:57:35 +0000 Subject: [PATCH 16/26] validate_port from validate_subtree not validate_node; some tests --- src/hugr/validate.rs | 30 +++++++++++++++++----------- src/hugr/validate/test.rs | 42 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index 66926aa03..a96d5d3ff 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -116,7 +116,7 @@ impl<'a, 'b> ValidationContext<'a, 'b> { /// This includes: /// - Matching the number of ports with the signature /// - Dataflow ports are correct. See `validate_df_port` - fn validate_node(&mut self, node: Node) -> Result<(), ValidationError> { + fn validate_node(&self, node: Node) -> Result<(), ValidationError> { let node_type = self.hugr.get_nodetype(node); let op_type = &node_type.op; @@ -158,12 +158,6 @@ impl<'a, 'b> ValidationContext<'a, 'b> { dir, }); } - - // Check port connections - for (i, port_index) in self.hugr.graph.ports(node.pg_index(), dir).enumerate() { - let port = Port::new(dir, i); - self.validate_port(node, port, port_index, op_type)?; - } } } @@ -193,6 +187,7 @@ impl<'a, 'b> ValidationContext<'a, 'b> { port: Port, port_index: portgraph::PortIndex, op_type: &OpType, + var_decls: &[TypeParam], ) -> Result<(), ValidationError> { let port_kind = op_type.port_kind(port).unwrap(); let dir = port.direction(); @@ -224,7 +219,7 @@ impl<'a, 'b> ValidationContext<'a, 'b> { match &port_kind { EdgeKind::Value(ty) | EdgeKind::Static(ty) => ty - .validate(self.extension_registry, &[]) // no type vars inside the Hugr + .validate(self.extension_registry, var_decls) .map_err(|cause| ValidationError::SignatureError { node, cause })?, _ => (), } @@ -248,9 +243,7 @@ impl<'a, 'b> ValidationContext<'a, 'b> { let other_op = self.hugr.get_optype(other_node); let Some(other_kind) = other_op.port_kind(other_offset) else { - // The number of ports in `other_node` does not match the operation definition. - // This should be caught by `validate_node`. - return Err(self.validate_node(other_node).unwrap_err()); + panic!("The number of ports in {other_node} does not match the operation definition. This should have been caught by `validate_node`."); }; // TODO: We will require some "unifiable" comparison instead of strict equality, to allow for pre-type inference hugrs. if other_kind != port_kind { @@ -529,8 +522,13 @@ impl<'a, 'b> ValidationContext<'a, 'b> { .into()) } - fn validate_subtree(&self, node: Node, var_decls: &[TypeParam]) -> Result<(), ValidationError> { + fn validate_subtree( + &mut self, + node: Node, + var_decls: &[TypeParam], + ) -> Result<(), ValidationError> { let op_type = self.hugr.get_optype(node); + // The op_type must be defined only in terms of type variables defined outside the node // TODO consider turning this match into a trait method? match op_type { OpType::LeafOp(crate::ops::LeafOp::CustomOp(b)) => { @@ -567,6 +565,14 @@ impl<'a, 'b> ValidationContext<'a, 'b> { _ => (), } + // Check port connections. + for dir in Direction::BOTH { + for (i, port_index) in self.hugr.graph.ports(node.pg_index(), dir).enumerate() { + let port = Port::new(dir, i); + self.validate_port(node, port, port_index, op_type, var_decls)?; + } + } + let mut v: Vec; let var_decls = if let OpType::FuncDefn(FuncDefn { signature, .. }) | OpType::FuncDecl(FuncDecl { signature, .. }) = op_type diff --git a/src/hugr/validate/test.rs b/src/hugr/validate/test.rs index a8028f7f8..87e026e32 100644 --- a/src/hugr/validate/test.rs +++ b/src/hugr/validate/test.rs @@ -2,7 +2,10 @@ use cool_asserts::assert_matches; use super::*; use crate::builder::test::closed_dfg_root_hugr; -use crate::builder::{BuildError, Container, Dataflow, DataflowSubContainer, ModuleBuilder}; +use crate::builder::{ + BuildError, Container, Dataflow, DataflowHugr, DataflowSubContainer, FunctionBuilder, + ModuleBuilder, +}; use crate::extension::prelude::{BOOL_T, PRELUDE, USIZE_T}; use crate::extension::{ Extension, ExtensionId, ExtensionSet, TypeDefBound, EMPTY_REG, PRELUDE_REGISTRY, @@ -15,7 +18,7 @@ use crate::ops::{self, LeafOp, OpType}; use crate::std_extensions::logic; use crate::std_extensions::logic::test::{and_op, not_op, or_op}; use crate::types::type_param::{TypeArg, TypeArgError, TypeParam}; -use crate::types::{CustomType, FunctionType, Type, TypeBound, TypeRow}; +use crate::types::{CustomType, FunctionType, PolyFuncType, Type, TypeBound, TypeRow}; use crate::{type_row, Direction, IncomingPort, Node}; const NAT: Type = crate::extension::prelude::USIZE_T; @@ -802,3 +805,38 @@ fn parent_io_mismatch() { )) ); } + +#[test] +fn typevars_declared() -> Result<(), Box> { + // Base case + let f = FunctionBuilder::new( + "myfunc", + PolyFuncType::new( + [TypeParam::Type(TypeBound::Any)], + FunctionType::new_endo(vec![Type::new_var_use(0, TypeBound::Any)]), + ), + )?; + let [w] = f.input_wires_arr(); + f.finish_prelude_hugr_with_outputs([w])?; + // Type refers to undeclared variable + let f = FunctionBuilder::new( + "myfunc", + PolyFuncType::new( + [TypeParam::Type(TypeBound::Any)], + FunctionType::new_endo(vec![Type::new_var_use(1, TypeBound::Any)]), + ), + )?; + let [w] = f.input_wires_arr(); + assert!(f.finish_prelude_hugr_with_outputs([w]).is_err()); + // Variable declaration incorrectly copied to use site + let f = FunctionBuilder::new( + "myfunc", + PolyFuncType::new( + [TypeParam::Type(TypeBound::Any)], + FunctionType::new_endo(vec![Type::new_var_use(1, TypeBound::Copyable)]), + ), + )?; + let [w] = f.input_wires_arr(); + assert!(f.finish_prelude_hugr_with_outputs([w]).is_err()); + Ok(()) +} From d9da4633ff4a2e2294dc654d5b412432eca91d85 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Thu, 16 Nov 2023 18:25:53 +0000 Subject: [PATCH 17/26] Finally give in and add "impl From for TypeParam" --- src/types/type_param.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/types/type_param.rs b/src/types/type_param.rs index d8546b58f..b87e2ad82 100644 --- a/src/types/type_param.rs +++ b/src/types/type_param.rs @@ -91,6 +91,12 @@ impl TypeParam { } } +impl From for TypeParam { + fn from(bound: TypeBound) -> Self { + Self::Type(bound) + } +} + /// A statically-known argument value to an operation. #[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize, serde::Serialize)] #[non_exhaustive] From 3652d564bcc64659ec09717bf615bbef326ffca6 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Thu, 16 Nov 2023 18:28:04 +0000 Subject: [PATCH 18/26] Test (and comment) re. nested FuncDefns --- src/hugr/validate.rs | 2 ++ src/hugr/validate/test.rs | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index a96d5d3ff..97796c2bf 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -573,6 +573,8 @@ impl<'a, 'b> ValidationContext<'a, 'b> { } } + // Compute cumulative TypeParams for nested FuncDefns, i.e. allow inner FuncDefns + // to refer to type parameters from outer FuncDefns. let mut v: Vec; let var_decls = if let OpType::FuncDefn(FuncDefn { signature, .. }) | OpType::FuncDecl(FuncDecl { signature, .. }) = op_type diff --git a/src/hugr/validate/test.rs b/src/hugr/validate/test.rs index 87e026e32..6dbc149e6 100644 --- a/src/hugr/validate/test.rs +++ b/src/hugr/validate/test.rs @@ -840,3 +840,43 @@ fn typevars_declared() -> Result<(), Box> { assert!(f.finish_prelude_hugr_with_outputs([w]).is_err()); Ok(()) } + +/// Test that nested FuncDefns can use Type Variables declared by enclosing FuncDefns +#[test] +fn nested_typevars() -> Result<(), Box> { + const OUTER_BOUND: TypeBound = TypeBound::Any; + const INNER_BOUND: TypeBound = TypeBound::Copyable; + fn build(t: Type) -> Result { + let mut outer = FunctionBuilder::new( + "outer", + PolyFuncType::new( + [OUTER_BOUND.into()], + FunctionType::new_endo(vec![Type::new_var_use(0, TypeBound::Any)]), + ), + )?; + let inner = outer.define_function( + "inner", + PolyFuncType::new([INNER_BOUND.into()], FunctionType::new_endo(vec![t])), + )?; + let [w] = inner.input_wires_arr(); + inner.finish_with_outputs([w])?; + let [w] = outer.input_wires_arr(); + outer.finish_prelude_hugr_with_outputs([w]) + } + assert!(build(Type::new_var_use(0, INNER_BOUND)).is_ok()); + assert!(build(Type::new_var_use(1, OUTER_BOUND)).is_ok()); + assert_matches!(build(Type::new_var_use(0, OUTER_BOUND)).unwrap_err(), + BuildError::InvalidHUGR(ValidationError::SignatureError { cause: SignatureError::TypeVarDoesNotMatchDeclaration { actual, cached }, .. }) => + actual == INNER_BOUND.into() && cached == OUTER_BOUND.into()); + assert_matches!( + build(Type::new_var_use(2, OUTER_BOUND)).unwrap_err(), + BuildError::InvalidHUGR(ValidationError::SignatureError { + cause: SignatureError::FreeTypeVar { + idx: 2, + num_decls: 2 + }, + .. + }) + ); + Ok(()) +} From d2e281b9a3270a34bddc87d614a75d1afb105810 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Thu, 16 Nov 2023 18:31:48 +0000 Subject: [PATCH 19/26] Comments re. Dataflow::call() --- src/builder/build_traits.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/builder/build_traits.rs b/src/builder/build_traits.rs index f090092ef..0e028b427 100644 --- a/src/builder/build_traits.rs +++ b/src/builder/build_traits.rs @@ -597,7 +597,8 @@ pub trait Dataflow: Container { function: &FuncID, type_args: &[TypeArg], input_wires: impl IntoIterator, - exts: &ExtensionRegistry, // TODO remove? + // Sadly required as we substituting in type_args may result in recomputing bounds of types: + exts: &ExtensionRegistry, ) -> Result, BuildError> { let hugr = self.hugr(); let def_op = hugr.get_optype(function.node()); @@ -611,11 +612,10 @@ pub trait Dataflow: Container { }) } }; - // TODO either some way of returning a SignatureError without a node (as its not - // constructed yet) - or, preferably, a way to "instantiate" the PolyFuncType without - // validating against an ExtensionRegistry. let signature = type_scheme.instantiate(type_args, exts).map_err(|e| { BuildError::InvalidHUGR(ValidationError::SignatureError { + // TODO this is rather a horrendous hack. Do we need some way of returning a SignatureError without a node + // (as the call node this refers to is not constructed yet)? Or, pass in an "instantiated" FuncID? node: function.node(), cause: e, }) From de374c57563d7e7478cc8012152026dc1dd2664c Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Thu, 16 Nov 2023 18:56:42 +0000 Subject: [PATCH 20/26] Avoid Itertools::Either - can use ExternalOp --- src/hugr/validate.rs | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index 97796c2bf..c549b6fcb 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::iter; -use itertools::{Either, Itertools}; +use itertools::Itertools; use petgraph::algo::dominators::{self, Dominators}; use petgraph::visit::{Topo, Walker}; use portgraph::{LinkView, PortView}; @@ -19,7 +19,7 @@ use crate::extension::{ }; use crate::ops::custom::CustomOpError; -use crate::ops::custom::{resolve_opaque_op, ExtensionOp, ExternalOp}; +use crate::ops::custom::{resolve_opaque_op, ExternalOp}; use crate::ops::validate::{ChildrenEdgeData, ChildrenValidationError, EdgeValidationError}; use crate::ops::{FuncDecl, FuncDefn, OpTag, OpTrait, OpType, ValidateOp}; use crate::types::type_param::TypeParam; @@ -533,24 +533,29 @@ impl<'a, 'b> ValidationContext<'a, 'b> { match op_type { OpType::LeafOp(crate::ops::LeafOp::CustomOp(b)) => { // Try to resolve serialized names to actual OpDefs in Extensions. - let e: Option; - let ext_or_opaq = match &**b { + let temp: ExternalOp; + let resolved = match &**b { ExternalOp::Opaque(op) => { // If resolve_extension_ops has been called first, this would always return Ok(None) - e = resolve_opaque_op(node, op, self.extension_registry)?; - e.as_ref().map(Either::Left).unwrap_or(Either::Right(op)) + match resolve_opaque_op(node, op, self.extension_registry)? { + Some(exten) => { + temp = ExternalOp::Extension(exten); + &temp + } + None => &**b, + } } - ExternalOp::Extension(ext) => Either::Left(ext), + ExternalOp::Extension(_) => &**b, }; - // If successful, check TypeArgs are valid for the declared TypeParams - match ext_or_opaq { - Either::Left(exten) => exten + // Check TypeArgs are valid, and if we can, fit the declared TypeParams + match resolved { + ExternalOp::Extension(exten) => exten .def() .validate_args(exten.args(), self.extension_registry, var_decls) .map_err(|cause| ValidationError::SignatureError { node, cause })?, - Either::Right(opaq) => { - // Best effort. Just check TypeArgs are valid (in themselves, not necessarily wrt the TypeParams) - // and assuming none are binary params (all may contain type vars) + ExternalOp::Opaque(opaq) => { + // Best effort. Just check TypeArgs are valid in themselves, allowing any of them + // to contain type vars (we don't know how many are binary params, so accept if in doubt) for arg in opaq.args() { arg.validate(self.extension_registry, var_decls) .map_err(|cause| ValidationError::SignatureError { node, cause })?; From 9b235609b86c9182b6dc9e3b71bf159154eebba8 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 20 Nov 2023 10:12:45 +0000 Subject: [PATCH 21/26] Update since-1.75 test --- src/hugr/views/tests.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/hugr/views/tests.rs b/src/hugr/views/tests.rs index 44b4ee8df..6d81ff52a 100644 --- a/src/hugr/views/tests.rs +++ b/src/hugr/views/tests.rs @@ -156,7 +156,7 @@ fn static_targets() { #[test] fn test_dataflow_ports_only() { use crate::builder::DataflowSubContainer; - use crate::extension::prelude::BOOL_T; + use crate::extension::{prelude::BOOL_T, PRELUDE_REGISTRY}; use crate::hugr::views::PortIterator; use crate::std_extensions::logic::test::not_op; use itertools::Itertools; @@ -165,7 +165,7 @@ fn test_dataflow_ports_only() { let local_and = dfg .define_function( "and", - FunctionType::new(type_row![BOOL_T; 2], type_row![BOOL_T]).pure(), + FunctionType::new(type_row![BOOL_T; 2], type_row![BOOL_T]).into(), ) .unwrap(); let first_input = local_and.input().out_wire(0); @@ -174,10 +174,17 @@ fn test_dataflow_ports_only() { let [in_bool] = dfg.input_wires_arr(); let not = dfg.add_dataflow_op(not_op(), [in_bool]).unwrap(); - let call = dfg.call(local_and.handle(), [not.out_wire(0); 2]).unwrap(); + let call = dfg + .call( + local_and.handle(), + &[], + [not.out_wire(0); 2], + &PRELUDE_REGISTRY, + ) + .unwrap(); dfg.add_other_wire(not.node(), call.node()).unwrap(); let h = dfg - .finish_hugr_with_outputs(not.outputs(), &crate::extension::PRELUDE_REGISTRY) + .finish_hugr_with_outputs(not.outputs(), &PRELUDE_REGISTRY) .unwrap(); let filtered_ports = h .all_linked_outputs(call.node()) From 9b0b5b9e3c5800dac576d858de51b66605de4d8f Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 20 Nov 2023 10:17:19 +0000 Subject: [PATCH 22/26] Provide a BuildError::SignatureError variant --- src/builder.rs | 5 +++++ src/builder/build_traits.rs | 9 +-------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index d0e743eb9..c24913303 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -5,6 +5,7 @@ use thiserror::Error; #[cfg(feature = "pyo3")] use pyo3::{create_exception, exceptions::PyException, PyErr}; +use crate::extension::SignatureError; use crate::hugr::{HugrError, ValidationError}; use crate::ops::handle::{BasicBlockID, CfgID, ConditionalID, DfgID, FuncID, TailLoopID}; use crate::types::ConstTypeError; @@ -43,6 +44,10 @@ pub enum BuildError { /// The constructed HUGR is invalid. #[error("The constructed HUGR is invalid: {0}.")] InvalidHUGR(#[from] ValidationError), + /// SignatureError in trying to construct a node (differs from + /// [ValidationError::SignatureError] in that we could not construct a node to report about) + #[error(transparent)] + SignatureError(#[from] SignatureError), /// Tried to add a malformed [Const] /// /// [Const]: crate::ops::constant::Const diff --git a/src/builder/build_traits.rs b/src/builder/build_traits.rs index 0e028b427..9f467426a 100644 --- a/src/builder/build_traits.rs +++ b/src/builder/build_traits.rs @@ -612,14 +612,7 @@ pub trait Dataflow: Container { }) } }; - let signature = type_scheme.instantiate(type_args, exts).map_err(|e| { - BuildError::InvalidHUGR(ValidationError::SignatureError { - // TODO this is rather a horrendous hack. Do we need some way of returning a SignatureError without a node - // (as the call node this refers to is not constructed yet)? Or, pass in an "instantiated" FuncID? - node: function.node(), - cause: e, - }) - })?; + let signature = type_scheme.instantiate(type_args, exts)?; let op: OpType = ops::Call { signature }.into(); let const_in_port = op.static_input_port().unwrap(); let op_id = self.add_dataflow_op(op, input_wires)?; From f069c921168c1132a5f414724670576ae77d0151 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 20 Nov 2023 16:22:15 +0000 Subject: [PATCH 23/26] Comment re FuncDefn::validate_op_children --- src/ops/validate.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ops/validate.rs b/src/ops/validate.rs index e9da6b2a2..ae31b8e28 100644 --- a/src/ops/validate.rs +++ b/src/ops/validate.rs @@ -77,6 +77,8 @@ impl ValidateOp for super::FuncDefn { &self, children: impl DoubleEndedIterator, ) -> Result<(), ChildrenValidationError> { + // We check type-variables are declared in `validate_subtree`, so here + // we can just assume all type variables are valid regardless of binders. let FunctionType { input, output, .. } = &self.signature.body; validate_io_nodes(input, output, "function definition", children) } From c3d4c8352203a494bc2930c31cce32a7e8e672ee Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 21 Nov 2023 12:46:50 +0000 Subject: [PATCH 24/26] CANNOT use type vars from enclosing defn's --- src/hugr/validate.rs | 17 +++++++---------- src/hugr/validate/test.rs | 15 +++++++-------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index c549b6fcb..69eec53f8 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -21,7 +21,7 @@ use crate::extension::{ use crate::ops::custom::CustomOpError; use crate::ops::custom::{resolve_opaque_op, ExternalOp}; use crate::ops::validate::{ChildrenEdgeData, ChildrenValidationError, EdgeValidationError}; -use crate::ops::{FuncDecl, FuncDefn, OpTag, OpTrait, OpType, ValidateOp}; +use crate::ops::{FuncDefn, OpTag, OpTrait, OpType, ValidateOp}; use crate::types::type_param::TypeParam; use crate::types::{EdgeKind, Type}; use crate::{Direction, Hugr, Node, Port}; @@ -522,6 +522,8 @@ impl<'a, 'b> ValidationContext<'a, 'b> { .into()) } + /// Validates that TypeArgs are valid wrt the [ExtensionRegistry] and that nodes + /// only refer to type variables declared by the closest enclosing FuncDefn. fn validate_subtree( &mut self, node: Node, @@ -578,15 +580,10 @@ impl<'a, 'b> ValidationContext<'a, 'b> { } } - // Compute cumulative TypeParams for nested FuncDefns, i.e. allow inner FuncDefns - // to refer to type parameters from outer FuncDefns. - let mut v: Vec; - let var_decls = if let OpType::FuncDefn(FuncDefn { signature, .. }) - | OpType::FuncDecl(FuncDecl { signature, .. }) = op_type - { - v = signature.params().to_owned(); - v.extend(var_decls.iter().cloned()); - v.as_ref() + // For FuncDefn's, only the type variables declared by the FuncDefn can be referred to by nodes + // inside the function. (The same would be true for FuncDecl's, but they have no child nodes.) + let var_decls = if let OpType::FuncDefn(FuncDefn { signature, .. }) = op_type { + signature.params() } else { var_decls }; diff --git a/src/hugr/validate/test.rs b/src/hugr/validate/test.rs index aa820c687..12c53688f 100644 --- a/src/hugr/validate/test.rs +++ b/src/hugr/validate/test.rs @@ -841,7 +841,7 @@ fn typevars_declared() -> Result<(), Box> { Ok(()) } -/// Test that nested FuncDefns can use Type Variables declared by enclosing FuncDefns +/// Test that nested FuncDefns cannot use Type Variables declared by enclosing FuncDefns #[test] fn nested_typevars() -> Result<(), Box> { const OUTER_BOUND: TypeBound = TypeBound::Any; @@ -864,19 +864,18 @@ fn nested_typevars() -> Result<(), Box> { outer.finish_prelude_hugr_with_outputs([w]) } assert!(build(Type::new_var_use(0, INNER_BOUND)).is_ok()); - assert!(build(Type::new_var_use(1, OUTER_BOUND)).is_ok()); - assert_matches!(build(Type::new_var_use(0, OUTER_BOUND)).unwrap_err(), - BuildError::InvalidHUGR(ValidationError::SignatureError { cause: SignatureError::TypeVarDoesNotMatchDeclaration { actual, cached }, .. }) => - actual == INNER_BOUND.into() && cached == OUTER_BOUND.into()); assert_matches!( - build(Type::new_var_use(2, OUTER_BOUND)).unwrap_err(), + build(Type::new_var_use(1, OUTER_BOUND)).unwrap_err(), BuildError::InvalidHUGR(ValidationError::SignatureError { cause: SignatureError::FreeTypeVar { - idx: 2, - num_decls: 2 + idx: 1, + num_decls: 1 }, .. }) ); + assert_matches!(build(Type::new_var_use(0, OUTER_BOUND)).unwrap_err(), + BuildError::InvalidHUGR(ValidationError::SignatureError { cause: SignatureError::TypeVarDoesNotMatchDeclaration { actual, cached }, .. }) => + actual == INNER_BOUND.into() && cached == OUTER_BOUND.into()); Ok(()) } From 95ddb36c6efba71a3fbd40a5211eb02b1757f512 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 21 Nov 2023 13:21:20 +0000 Subject: [PATCH 25/26] Test no_polymorphic_consts, enforce no TVs on Static edges, add ListValue::new --- src/hugr/validate.rs | 7 +++++- src/hugr/validate/test.rs | 40 ++++++++++++++++++++++++++++++- src/std_extensions/collections.rs | 8 +++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index 69eec53f8..bf0021ad8 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -218,9 +218,14 @@ impl<'a, 'b> ValidationContext<'a, 'b> { } match &port_kind { - EdgeKind::Value(ty) | EdgeKind::Static(ty) => ty + EdgeKind::Value(ty) => ty .validate(self.extension_registry, var_decls) .map_err(|cause| ValidationError::SignatureError { node, cause })?, + // Static edges must *not* refer to type variables declared by enclosing FuncDefns + // as these are only types at runtime. + EdgeKind::Static(ty) => ty + .validate(self.extension_registry, &[]) + .map_err(|cause| ValidationError::SignatureError { node, cause })?, _ => (), } diff --git a/src/hugr/validate/test.rs b/src/hugr/validate/test.rs index 12c53688f..8a59d8a45 100644 --- a/src/hugr/validate/test.rs +++ b/src/hugr/validate/test.rs @@ -14,11 +14,12 @@ use crate::hugr::hugrmut::sealed::HugrMutInternals; use crate::hugr::{HugrError, HugrMut, NodeType}; use crate::macros::const_extension_ids; use crate::ops::dataflow::IOTrait; -use crate::ops::{self, LeafOp, OpType}; +use crate::ops::{self, Const, LeafOp, OpType}; use crate::std_extensions::logic; use crate::std_extensions::logic::test::{and_op, not_op, or_op}; use crate::types::type_param::{TypeArg, TypeArgError, TypeParam}; use crate::types::{CustomType, FunctionType, PolyFuncType, Type, TypeBound, TypeRow}; +use crate::values::Value; use crate::{type_row, Direction, IncomingPort, Node}; const NAT: Type = crate::extension::prelude::USIZE_T; @@ -879,3 +880,40 @@ fn nested_typevars() -> Result<(), Box> { actual == INNER_BOUND.into() && cached == OUTER_BOUND.into()); Ok(()) } + +#[test] +fn no_polymorphic_consts() -> Result<(), Box> { + use crate::std_extensions::collections; + const BOUND: TypeParam = TypeParam::Type(TypeBound::Copyable); + let list_of_var = Type::new_extension( + collections::EXTENSION + .get_type(&collections::LIST_TYPENAME) + .unwrap() + .instantiate(vec![TypeArg::new_var_use(0, BOUND)])?, + ); + let reg = ExtensionRegistry::try_new([collections::EXTENSION.to_owned()]).unwrap(); + let just_colns = ExtensionSet::singleton(&collections::EXTENSION_NAME); + let mut def = FunctionBuilder::new( + "myfunc", + PolyFuncType::new( + [BOUND], + FunctionType::new(vec![], vec![list_of_var.clone()]).with_extension_delta(&just_colns), + ), + )?; + let empty_list = Value::Extension { + c: (Box::new(collections::ListValue::new(vec![])),), + }; + let cst = def.add_load_const(Const::new(empty_list, list_of_var)?, just_colns)?; + let res = def.finish_hugr_with_outputs([cst], ®); + assert_matches!( + res.unwrap_err(), + BuildError::InvalidHUGR(ValidationError::SignatureError { + cause: SignatureError::FreeTypeVar { + idx: 0, + num_decls: 0 + }, + .. + }) + ); + Ok(()) +} diff --git a/src/std_extensions/collections.rs b/src/std_extensions/collections.rs index 99d208c63..41939d5eb 100644 --- a/src/std_extensions/collections.rs +++ b/src/std_extensions/collections.rs @@ -27,6 +27,14 @@ pub const EXTENSION_NAME: ExtensionId = ExtensionId::new_unchecked("Collections" /// Dynamically sized list of values, all of the same type. pub struct ListValue(Vec); +impl ListValue { + /// Create a new [CustomConst] for a list of values. + /// (The caller will need these to all be of the same type, but that is not checked here.) + pub fn new(contents: Vec) -> Self { + Self(contents) + } +} + #[typetag::serde] impl CustomConst for ListValue { fn name(&self) -> SmolStr { From 7e8c5152bf3b593873e396d79a81235f5a1500aa Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 21 Nov 2023 16:57:35 +0000 Subject: [PATCH 26/26] wth -> with --- src/hugr/views/sibling_subgraph.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hugr/views/sibling_subgraph.rs b/src/hugr/views/sibling_subgraph.rs index 6f5ed8d04..e8e8b4319 100644 --- a/src/hugr/views/sibling_subgraph.rs +++ b/src/hugr/views/sibling_subgraph.rs @@ -406,7 +406,7 @@ impl SiblingSubgraph { /// Create a new Hugr containing only the subgraph. /// /// The new Hugr will contain a [FuncDefn][crate::ops::FuncDefn] root - /// wth the same signature as the subgraph and the specified `name` + /// with the same signature as the subgraph and the specified `name` pub fn extract_subgraph( &self, hugr: &impl HugrView,