From 098d6232d74538da2237dc2dfcea92ad1359b4da Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 21 Nov 2023 18:22:50 +0000 Subject: [PATCH 1/6] Test binary compute_signature returning a PolyFuncType with binders --- src/extension/op_def.rs | 94 +++++++++++++++++++++++++++++++++++++++-- src/types/type_param.rs | 6 +++ 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 9dc2fcd18..608c85d04 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -180,9 +180,6 @@ impl OpDef { args: &[TypeArg], exts: &ExtensionRegistry, ) -> Result { - // Hugr's are monomorphic, so check the args have no free variables - args.iter().try_for_each(|ta| ta.validate(exts, &[]))?; - let temp: PolyFuncType; // to keep alive let (pf, args) = match &self.signature_func { SignatureFunc::TypeScheme(ts) => (ts, args), @@ -369,11 +366,15 @@ impl Extension { #[cfg(test)] mod test { + use std::num::NonZeroU64; + use smol_str::SmolStr; use crate::builder::{DFGBuilder, Dataflow, DataflowHugr}; use crate::extension::prelude::USIZE_T; - use crate::extension::{ExtensionRegistry, PRELUDE}; + use crate::extension::{ + CustomSignatureFunc, ExtensionRegistry, SignatureError, PRELUDE, PRELUDE_REGISTRY, + }; use crate::ops::custom::ExternalOp; use crate::ops::LeafOp; use crate::std_extensions::collections::{EXTENSION, LIST_TYPENAME}; @@ -413,4 +414,89 @@ mod test { Ok(()) } + + #[test] + fn binary_polyfunc() -> Result<(), Box> { + struct SigFun(); + impl CustomSignatureFunc for SigFun { + fn compute_signature( + &self, + _name: &SmolStr, + arg_values: &[TypeArg], + _misc: &std::collections::HashMap, + _exts: &ExtensionRegistry, + ) -> Result { + const TP: TypeParam = TypeParam::Type(TypeBound::Any); + let [TypeArg::BoundedNat {n}] = arg_values else { return Err(SignatureError::InvalidTypeArgs) }; + let n = *n as usize; + let tvs: Vec = (0..n) + .map(|_| Type::new_var_use(0, TypeBound::Any)) + .collect(); + Ok(PolyFuncType::new( + vec![TP], + FunctionType::new(tvs.clone(), vec![Type::new_tuple(tvs)]), + )) + } + } + let mut e = Extension::new(EXT_ID); + let def = e.add_op_custom_sig_simple( + "MyOp".into(), + "".to_string(), + vec![TypeParam::max_nat()], + SigFun(), + )?; + + // Base case, no type variables: + let args = [TypeArg::BoundedNat { n: 3 }, USIZE_T.into()]; + assert_eq!( + def.compute_signature(&args, &PRELUDE_REGISTRY), + Ok(FunctionType::new( + vec![USIZE_T; 3], + vec![Type::new_tuple(vec![USIZE_T; 3])] + )) + ); + assert_eq!(def.validate_args(&args, &PRELUDE_REGISTRY, &[]), Ok(())); + + // Second arg may be a variable (substitutable) + let tyvar = Type::new_var_use(0, TypeBound::Eq); + let tyvars: Vec = (0..3).map(|_| tyvar.clone()).collect(); + let args = [TypeArg::BoundedNat { n: 3 }, tyvar.clone().into()]; + assert_eq!( + def.compute_signature(&args, &PRELUDE_REGISTRY), + Ok(FunctionType::new( + tyvars.clone(), + vec![Type::new_tuple(tyvars)] + )) + ); + def.validate_args(&args, &PRELUDE_REGISTRY, &[TypeParam::Type(TypeBound::Eq)]) + .unwrap(); + + // quick sanity check that we are validating the args - note changed bound: + assert_eq!( + def.validate_args(&args, &PRELUDE_REGISTRY, &[TypeParam::Type(TypeBound::Any)]), + Err(SignatureError::TypeVarDoesNotMatchDeclaration { + actual: TypeBound::Any.into(), + cached: TypeBound::Eq.into() + }) + ); + + // First arg must be concrete, not a variable + let kind = TypeParam::bounded_nat(NonZeroU64::new(5).unwrap()); + let args = [TypeArg::new_var_use(0, kind.clone()), USIZE_T.into()]; + // We can't prevent this from getting into our compute_signature implementation: + assert_eq!( + def.compute_signature(&args, &PRELUDE_REGISTRY), + Err(SignatureError::InvalidTypeArgs) + ); + // But validation rules it out, even when the variable is declared: + assert_eq!( + def.validate_args(&args, &PRELUDE_REGISTRY, &[kind]), + Err(SignatureError::FreeTypeVar { + idx: 0, + num_decls: 0 + }) + ); + + Ok(()) + } } diff --git a/src/types/type_param.rs b/src/types/type_param.rs index b87e2ad82..16d9e0a5a 100644 --- a/src/types/type_param.rs +++ b/src/types/type_param.rs @@ -97,6 +97,12 @@ impl From for TypeParam { } } +impl From for TypeArg { + fn from(ty: Type) -> Self { + Self::Type { ty } + } +} + /// A statically-known argument value to an operation. #[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize, serde::Serialize)] #[non_exhaustive] From d97390908a1c212ceb9600f6ee601592b4dcb14c Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 21 Nov 2023 18:40:03 +0000 Subject: [PATCH 2/6] Bonus: check simple TypeSchemes can be instantiated with type vars too --- src/extension/op_def.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 608c85d04..4ae8cc369 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -373,7 +373,8 @@ mod test { use crate::builder::{DFGBuilder, Dataflow, DataflowHugr}; use crate::extension::prelude::USIZE_T; use crate::extension::{ - CustomSignatureFunc, ExtensionRegistry, SignatureError, PRELUDE, PRELUDE_REGISTRY, + CustomSignatureFunc, ExtensionRegistry, SignatureError, EMPTY_REG, PRELUDE, + PRELUDE_REGISTRY, }; use crate::ops::custom::ExternalOp; use crate::ops::LeafOp; @@ -499,4 +500,26 @@ mod test { Ok(()) } + + #[test] + fn type_scheme_instantiate_var() -> Result<(), Box> { + let mut e = Extension::new(EXT_ID); + let def = e.add_op_type_scheme_simple( + "SimpleOp".into(), + "".into(), + PolyFuncType::new( + vec![TypeParam::Type(TypeBound::Any)], + FunctionType::new_endo(vec![Type::new_var_use(0, TypeBound::Any)]), + ), + )?; + let tv = Type::new_var_use(1, TypeBound::Eq); + let args = [TypeArg::Type { ty: tv.clone() }]; + let decls = [TypeParam::Extensions, TypeBound::Eq.into()]; + def.validate_args(&args, &EMPTY_REG, &decls).unwrap(); + assert_eq!( + def.compute_signature(&args, &EMPTY_REG), + Ok(FunctionType::new_endo(vec![tv])) + ); + Ok(()) + } } From 21625523817f476a9c0117641b483f07e097b618 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 21 Nov 2023 21:17:43 +0000 Subject: [PATCH 3/6] ci fmt --- src/extension/op_def.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 4ae8cc369..0ea0628d4 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -428,7 +428,9 @@ mod test { _exts: &ExtensionRegistry, ) -> Result { const TP: TypeParam = TypeParam::Type(TypeBound::Any); - let [TypeArg::BoundedNat {n}] = arg_values else { return Err(SignatureError::InvalidTypeArgs) }; + let [TypeArg::BoundedNat {n}] = arg_values else { + return Err(SignatureError::InvalidTypeArgs) + }; let n = *n as usize; let tvs: Vec = (0..n) .map(|_| Type::new_var_use(0, TypeBound::Any)) From fb766977e6cad091e52f16c6c255c8aa7985a58a Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 21 Nov 2023 21:28:23 +0000 Subject: [PATCH 4/6] Ah-ha, use correct fmt --- src/extension/op_def.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 0ea0628d4..4dee85bc2 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -428,8 +428,8 @@ mod test { _exts: &ExtensionRegistry, ) -> Result { const TP: TypeParam = TypeParam::Type(TypeBound::Any); - let [TypeArg::BoundedNat {n}] = arg_values else { - return Err(SignatureError::InvalidTypeArgs) + let [TypeArg::BoundedNat { n }] = arg_values else { + return Err(SignatureError::InvalidTypeArgs); }; let n = *n as usize; let tvs: Vec = (0..n) From d0d9da4e5fb1ec90bf14d4333470ca9076d786b4 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 22 Nov 2023 16:11:36 +0000 Subject: [PATCH 5/6] Use `vec![...;3]` syntax --- src/extension/op_def.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 4dee85bc2..5c71080cd 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -462,7 +462,7 @@ mod test { // Second arg may be a variable (substitutable) let tyvar = Type::new_var_use(0, TypeBound::Eq); - let tyvars: Vec = (0..3).map(|_| tyvar.clone()).collect(); + let tyvars: Vec = vec![tyvar.clone(); 3]; let args = [TypeArg::BoundedNat { n: 3 }, tyvar.clone().into()]; assert_eq!( def.compute_signature(&args, &PRELUDE_REGISTRY), From 5f4e71aea8b0c4516fab2e673c635c65c4510131 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 22 Nov 2023 16:11:49 +0000 Subject: [PATCH 6/6] comments --- src/extension/op_def.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 5c71080cd..d1062a3ce 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -418,6 +418,10 @@ mod test { #[test] fn binary_polyfunc() -> Result<(), Box> { + // Test a custom binary `compute_signature` that returns a PolyFuncType + // where the latter declares more type params itself. In particular, + // we should be able to substitute (external) type variables into the latter, + // but not pass them into the former (custom binary function). struct SigFun(); impl CustomSignatureFunc for SigFun { fn compute_signature( @@ -505,6 +509,8 @@ mod test { #[test] fn type_scheme_instantiate_var() -> Result<(), Box> { + // Check that we can instantiate a PolyFuncType-scheme with an (external) + // type variable let mut e = Extension::new(EXT_ID); let def = e.add_op_type_scheme_simple( "SimpleOp".into(),