From d0a71a6bb27e9cddb10e600746df6bc5c430959d Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Fri, 24 Nov 2023 16:17:16 +0000 Subject: [PATCH 01/14] feat!: `OpEnum` trait for common opdef functionality BREAKING_CHANGES: const logic op names removd --- Cargo.toml | 2 + src/extension.rs | 6 +- src/extension/op_def.rs | 8 +- src/extension/simple_op.rs | 160 ++++++++++++++++++++++++++++++++++++ src/std_extensions/logic.rs | 104 ++++++++++++++--------- 5 files changed, 235 insertions(+), 45 deletions(-) create mode 100644 src/extension/simple_op.rs diff --git a/Cargo.toml b/Cargo.toml index 3a2f70bcc..012a80749 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,6 +49,8 @@ serde_json = "1.0.97" delegate = "0.10.0" rustversion = "1.0.14" paste = "1.0" +strum = "0.25.0" +strum_macros = "0.25.3" [dev-dependencies] criterion = { version = "0.5.1", features = ["html_reports"] } diff --git a/src/extension.rs b/src/extension.rs index dfdfc5acc..f18834baa 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -23,14 +23,14 @@ pub use infer::{infer_extensions, ExtensionSolution, InferExtensionError}; mod op_def; pub use op_def::{ - CustomSignatureFunc, CustomValidator, OpDef, SignatureFromArgs, ValidateJustArgs, - ValidateTypeArgs, + CustomSignatureFunc, CustomValidator, OpDef, SignatureFromArgs, SignatureFunc, + ValidateJustArgs, ValidateTypeArgs, }; mod type_def; pub use type_def::{TypeDef, TypeDefBound}; pub mod prelude; +pub mod simple_op; pub mod validate; - pub use prelude::{PRELUDE, PRELUDE_REGISTRY}; /// Extension Registries store extensions to be looked up e.g. during validation. diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index b8d43dff9..284478652 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -148,18 +148,18 @@ impl CustomValidator { } /// The two ways in which an OpDef may compute the Signature of each operation node. -/// Either as a TypeScheme (polymorphic function type), with optional custom -/// validation for provided type arguments, -/// or a custom binary which computes a polymorphic function type given values -/// for its static type parameters. #[derive(serde::Deserialize, serde::Serialize)] pub enum SignatureFunc { // Note: except for serialization, we could have type schemes just implement the same // CustomSignatureFunc trait too, and replace this enum with Box. // However instead we treat all CustomFunc's as non-serializable. + /// A TypeScheme (polymorphic function type), with optional custom + /// validation for provided type arguments, #[serde(rename = "signature")] TypeScheme(CustomValidator), #[serde(skip)] + /// A custom binary which computes a polymorphic function type given values + /// for its static type parameters. CustomFunc(Box), } struct NoValidate; diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs new file mode 100644 index 000000000..1bc4dc08e --- /dev/null +++ b/src/extension/simple_op.rs @@ -0,0 +1,160 @@ +//! A trait that enum for op definitions that gathers up some shared functionality. +use std::str::FromStr; + +use strum::IntoEnumIterator; + +use crate::Extension; + +use super::{op_def::SignatureFunc, ExtensionBuildError, ExtensionId, OpDef}; +use thiserror::Error; + +/// Error when definition extension does not match that of the [OpEnum] +#[derive(Debug, Error, PartialEq)] +#[error("Expected extension ID {expected} but found {provided}.")] +pub struct WrongExtension { + expected: ExtensionId, + provided: ExtensionId, +} + +/// Error loading [OpEnum] +#[derive(Debug, Error, PartialEq)] +#[error("{0}")] +#[allow(missing_docs)] +pub enum OpLoadError { + WrongExtension(#[from] WrongExtension), + LoadError(T), +} + +/// A trait that operation sets defined by simple (C-style) enums can implement +/// to simplify interactions with the extension. +/// Relies on `strum_macros::{EnumIter, EnumString, IntoStaticStr}` +pub trait OpEnum: Into<&'static str> + FromStr + Copy + IntoEnumIterator { + /// The name of the extension these ops belong to. + const EXTENSION_ID: ExtensionId; + + // TODO can be removed after rust 1.75 + /// Error thrown when loading from string fails. + type LoadError: std::error::Error; + /// Description type. + type Description: ToString; + + /// Return the signature (polymorphic function type) of the operation. + fn signature(&self) -> SignatureFunc; + + /// Description of the operation. + fn description(&self) -> Self::Description; + + /// Edit the opdef before finalising. + fn post_opdef(&self, _def: &mut OpDef) {} + + /// Name of the operation - derived from strum serialization. + fn name(&self) -> &str { + (*self).into() + } + + /// Load an operation from the name of the operation. + fn from_extension_name(op_name: &str) -> Result; + + /// Try to load one of the operations of this set from an [OpDef]. + fn try_from_op_def(op_def: &OpDef) -> Result> { + if op_def.extension() != &Self::EXTENSION_ID { + return Err(WrongExtension { + expected: Self::EXTENSION_ID.clone(), + provided: op_def.extension().clone(), + } + .into()); + } + Self::from_extension_name(op_def.name()).map_err(OpLoadError::LoadError) + } + + /// Add an operation to an extension. + fn add_to_extension<'e>( + &self, + ext: &'e mut Extension, + ) -> Result<&'e OpDef, ExtensionBuildError> { + let def = ext.add_op( + self.name().into(), + self.description().to_string(), + self.signature(), + )?; + + self.post_opdef(def); + + Ok(def) + } + + /// Iterator over all operations in the set. + fn all_variants() -> ::Iterator { + ::iter() + } + + /// load all variants of a `SimpleOpEnum` in to an extension as op defs. + fn load_all_ops(extension: &mut Extension) -> Result<(), ExtensionBuildError> { + for op in Self::all_variants() { + op.add_to_extension(extension)?; + } + Ok(()) + } +} + +#[cfg(test)] +mod test { + use crate::{type_row, types::FunctionType}; + + use super::*; + use strum_macros::{EnumIter, EnumString, IntoStaticStr}; + #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, EnumIter, IntoStaticStr, EnumString)] + enum DummyEnum { + Dumb, + } + #[derive(Debug, thiserror::Error, PartialEq)] + #[error("Dummy")] + struct DummyError; + impl OpEnum for DummyEnum { + const EXTENSION_ID: ExtensionId = ExtensionId::new_unchecked("dummy"); + + type LoadError = DummyError; + + type Description = &'static str; + + fn signature(&self) -> SignatureFunc { + FunctionType::new_endo(type_row![]).into() + } + + fn description(&self) -> Self::Description { + "dummy" + } + + fn from_extension_name(_op_name: &str) -> Result { + Ok(Self::Dumb) + } + } + + #[test] + fn test_dummy_enum() { + let o = DummyEnum::Dumb; + + let good_name = ExtensionId::new("dummy").unwrap(); + let mut e = Extension::new(good_name.clone()); + + o.add_to_extension(&mut e).unwrap(); + + assert_eq!( + DummyEnum::try_from_op_def(e.get_op(o.name()).unwrap()).unwrap(), + o + ); + + let bad_name = ExtensionId::new("not_dummy").unwrap(); + let mut e = Extension::new(bad_name.clone()); + + o.add_to_extension(&mut e).unwrap(); + + assert_eq!( + DummyEnum::try_from_op_def(e.get_op(o.name()).unwrap()), + Err(OpLoadError::WrongExtension(WrongExtension { + expected: good_name, + provided: bad_name + })) + ); + } +} diff --git a/src/std_extensions/logic.rs b/src/std_extensions/logic.rs index 710d29b1b..255ff221b 100644 --- a/src/std_extensions/logic.rs +++ b/src/std_extensions/logic.rs @@ -1,9 +1,12 @@ //! Basic logical operations. -use smol_str::SmolStr; +use strum_macros::{EnumIter, EnumString, IntoStaticStr}; use crate::{ - extension::{prelude::BOOL_T, ExtensionId, SignatureError, SignatureFromArgs}, + extension::{ + prelude::BOOL_T, simple_op::OpEnum, ExtensionId, SignatureError, SignatureFromArgs, + SignatureFunc, + }, ops, type_row, types::{ type_param::{TypeArg, TypeParam}, @@ -12,18 +15,51 @@ use crate::{ Extension, }; use lazy_static::lazy_static; - +use std::str::FromStr; +use thiserror::Error; /// Name of extension false value. pub const FALSE_NAME: &str = "FALSE"; /// Name of extension true value. pub const TRUE_NAME: &str = "TRUE"; -/// Name of the "not" operation. -pub const NOT_NAME: &str = "Not"; -/// Name of the "and" operation. -pub const AND_NAME: &str = "And"; -/// Name of the "or" operation. -pub const OR_NAME: &str = "Or"; +/// Logic extension operations. +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, EnumIter, IntoStaticStr, EnumString)] +#[allow(missing_docs)] +pub enum LogicOp { + And, + Or, + Not, +} + +/// Error in trying to load logic operation. +#[derive(Debug, Error)] +#[error("Not a logic extension operation.")] +pub struct NotLogicOp; + +impl OpEnum for LogicOp { + const EXTENSION_ID: ExtensionId = EXTENSION_ID; + type LoadError = NotLogicOp; + type Description = &'static str; + + fn from_extension_name(op_name: &str) -> Result { + Self::from_str(op_name).map_err(|_| NotLogicOp) + } + + fn signature(&self) -> SignatureFunc { + match self { + LogicOp::Or | LogicOp::And => logic_op_sig().into(), + LogicOp::Not => FunctionType::new_endo(type_row![BOOL_T]).into(), + } + } + + fn description(&self) -> &'static str { + match self { + LogicOp::And => "logical 'and'", + LogicOp::Or => "logical 'or'", + LogicOp::Not => "logical 'not'", + } + } +} /// The extension identifier. pub const EXTENSION_ID: ExtensionId = ExtensionId::new_unchecked("logic"); @@ -53,30 +89,7 @@ fn logic_op_sig() -> impl SignatureFromArgs { /// Extension for basic logical operations. fn extension() -> Extension { let mut extension = Extension::new(EXTENSION_ID); - - extension - .add_op( - SmolStr::new_inline(NOT_NAME), - "logical 'not'".into(), - FunctionType::new(type_row![BOOL_T], type_row![BOOL_T]), - ) - .unwrap(); - - extension - .add_op( - SmolStr::new_inline(AND_NAME), - "logical 'and'".into(), - logic_op_sig(), - ) - .unwrap(); - - extension - .add_op( - SmolStr::new_inline(OR_NAME), - "logical 'or'".into(), - logic_op_sig(), - ) - .unwrap(); + LogicOp::load_all_ops(&mut extension).unwrap(); extension .add_value(FALSE_NAME, ops::Const::unit_sum(0, 2)) @@ -95,19 +108,26 @@ lazy_static! { #[cfg(test)] pub(crate) mod test { use crate::{ - extension::{prelude::BOOL_T, EMPTY_REG}, + extension::{prelude::BOOL_T, simple_op::OpEnum, EMPTY_REG}, ops::LeafOp, types::type_param::TypeArg, Extension, }; - use super::{extension, AND_NAME, EXTENSION, FALSE_NAME, NOT_NAME, OR_NAME, TRUE_NAME}; + use super::{extension, LogicOp, EXTENSION, FALSE_NAME, TRUE_NAME}; #[test] fn test_logic_extension() { let r: Extension = extension(); assert_eq!(r.name() as &str, "logic"); assert_eq!(r.operations().count(), 3); + + for op in LogicOp::all_variants() { + assert_eq!( + LogicOp::try_from_op_def(r.get_op(op.name()).unwrap()).unwrap(), + op + ); + } } #[test] @@ -125,7 +145,11 @@ pub(crate) mod test { /// Generate a logic extension and "and" operation over [`crate::prelude::BOOL_T`] pub(crate) fn and_op() -> LeafOp { EXTENSION - .instantiate_extension_op(AND_NAME, [TypeArg::BoundedNat { n: 2 }], &EMPTY_REG) + .instantiate_extension_op( + LogicOp::And.name(), + [TypeArg::BoundedNat { n: 2 }], + &EMPTY_REG, + ) .unwrap() .into() } @@ -133,7 +157,11 @@ pub(crate) mod test { /// Generate a logic extension and "or" operation over [`crate::prelude::BOOL_T`] pub(crate) fn or_op() -> LeafOp { EXTENSION - .instantiate_extension_op(OR_NAME, [TypeArg::BoundedNat { n: 2 }], &EMPTY_REG) + .instantiate_extension_op( + LogicOp::Or.name(), + [TypeArg::BoundedNat { n: 2 }], + &EMPTY_REG, + ) .unwrap() .into() } @@ -141,7 +169,7 @@ pub(crate) mod test { /// Generate a logic extension and "not" operation over [`crate::prelude::BOOL_T`] pub(crate) fn not_op() -> LeafOp { EXTENSION - .instantiate_extension_op(NOT_NAME, [], &EMPTY_REG) + .instantiate_extension_op(LogicOp::Not.name(), [], &EMPTY_REG) .unwrap() .into() } From 2bb5a62cdd54d27cee3583d1e150b478fcdb1dac Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Fri, 24 Nov 2023 17:01:35 +0000 Subject: [PATCH 02/14] feat: add `as_extension_op` to LeafOp and use to define conversion of OpEnum to and from OpType. --- src/extension/simple_op.rs | 35 ++++++++++++++++++++++++++++++++--- src/ops/leaf.rs | 16 +++++++++++++++- src/std_extensions/logic.rs | 31 +++++++++---------------------- 3 files changed, 56 insertions(+), 26 deletions(-) diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index 1bc4dc08e..1363e621b 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -3,9 +3,13 @@ use std::str::FromStr; use strum::IntoEnumIterator; -use crate::Extension; +use crate::{ + ops::{custom::ExtensionOp, LeafOp, OpType}, + types::TypeArg, + Extension, +}; -use super::{op_def::SignatureFunc, ExtensionBuildError, ExtensionId, OpDef}; +use super::{op_def::SignatureFunc, ExtensionBuildError, ExtensionId, ExtensionRegistry, OpDef}; use thiserror::Error; /// Error when definition extension does not match that of the [OpEnum] @@ -95,11 +99,32 @@ pub trait OpEnum: Into<&'static str> + FromStr + Copy + IntoEnumIterator { } Ok(()) } + + /// Generate an [OpType]. + fn to_optype( + &self, + extension: &Extension, + args: &[TypeArg], + exts: &ExtensionRegistry, + ) -> Option { + let leaf: LeafOp = ExtensionOp::new(extension.get_op(self.name())?.clone(), args, exts) + .ok()? + .into(); + + Some(leaf.into()) + } + + /// Try to instantiate a variant from an [OpType]. Default behaviour assumes + /// an [ExtensionOp] and loads from the name. + fn from_optype(op: &OpType) -> Option { + let ext: &ExtensionOp = op.as_leaf_op()?.as_extension_op()?; + Self::try_from_op_def(ext.def()).ok() + } } #[cfg(test)] mod test { - use crate::{type_row, types::FunctionType}; + use crate::{extension::EMPTY_REG, type_row, types::FunctionType}; use super::*; use strum_macros::{EnumIter, EnumString, IntoStaticStr}; @@ -144,6 +169,10 @@ mod test { o ); + assert_eq!( + DummyEnum::from_optype(&o.to_optype(&e, &[], &EMPTY_REG).unwrap()).unwrap(), + o + ); let bad_name = ExtensionId::new("not_dummy").unwrap(); let mut e = Extension::new(bad_name.clone()); diff --git a/src/ops/leaf.rs b/src/ops/leaf.rs index 432790cf8..6a1e5ac62 100644 --- a/src/ops/leaf.rs +++ b/src/ops/leaf.rs @@ -2,7 +2,7 @@ use smol_str::SmolStr; -use super::custom::ExternalOp; +use super::custom::{ExtensionOp, ExternalOp}; use super::dataflow::DataflowOpTrait; use super::{OpName, OpTag}; @@ -62,6 +62,20 @@ pub enum LeafOp { }, } +impl LeafOp { + /// If instance of [ExtensionOp] return a reference to it. + pub fn as_extension_op(&self) -> Option<&ExtensionOp> { + let LeafOp::CustomOp(ext) = self else { + return None; + }; + + match ext.as_ref() { + ExternalOp::Extension(e) => Some(e), + ExternalOp::Opaque(_) => None, + } + } +} + /// Records details of an application of a [PolyFuncType] to some [TypeArg]s /// and the result (a less-, but still potentially-, polymorphic type). #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] diff --git a/src/std_extensions/logic.rs b/src/std_extensions/logic.rs index 255ff221b..2738d1455 100644 --- a/src/std_extensions/logic.rs +++ b/src/std_extensions/logic.rs @@ -109,7 +109,7 @@ lazy_static! { pub(crate) mod test { use crate::{ extension::{prelude::BOOL_T, simple_op::OpEnum, EMPTY_REG}, - ops::LeafOp, + ops::OpType, types::type_param::TypeArg, Extension, }; @@ -143,34 +143,21 @@ pub(crate) mod test { } /// Generate a logic extension and "and" operation over [`crate::prelude::BOOL_T`] - pub(crate) fn and_op() -> LeafOp { - EXTENSION - .instantiate_extension_op( - LogicOp::And.name(), - [TypeArg::BoundedNat { n: 2 }], - &EMPTY_REG, - ) + pub(crate) fn and_op() -> OpType { + LogicOp::And + .to_optype(&EXTENSION, &[TypeArg::BoundedNat { n: 2 }], &EMPTY_REG) .unwrap() - .into() } /// Generate a logic extension and "or" operation over [`crate::prelude::BOOL_T`] - pub(crate) fn or_op() -> LeafOp { - EXTENSION - .instantiate_extension_op( - LogicOp::Or.name(), - [TypeArg::BoundedNat { n: 2 }], - &EMPTY_REG, - ) + pub(crate) fn or_op() -> OpType { + LogicOp::Or + .to_optype(&EXTENSION, &[TypeArg::BoundedNat { n: 2 }], &EMPTY_REG) .unwrap() - .into() } /// Generate a logic extension and "not" operation over [`crate::prelude::BOOL_T`] - pub(crate) fn not_op() -> LeafOp { - EXTENSION - .instantiate_extension_op(LogicOp::Not.name(), [], &EMPTY_REG) - .unwrap() - .into() + pub(crate) fn not_op() -> OpType { + LogicOp::Not.to_optype(&EXTENSION, &[], &EMPTY_REG).unwrap() } } From 8ebaf86c47a7c8faae40340ddf1273434b014971 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 12:29:42 +0000 Subject: [PATCH 03/14] move registry-based methods to wrapper struct --- src/extension/op_def.rs | 50 ++++++----- src/extension/simple_op.rs | 163 +++++++++++++++++++++++------------- src/std_extensions/logic.rs | 83 ++++++++++++------ 3 files changed, 190 insertions(+), 106 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 284478652..639224933 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -211,6 +211,34 @@ impl SignatureFunc { SignatureFunc::CustomFunc(func) => func.static_params(), } } + pub fn compute_signature( + &self, + def: &OpDef, + args: &[TypeArg], + exts: &ExtensionRegistry, + ) -> Result { + let temp: PolyFuncType; + let (pf, args) = match &self { + SignatureFunc::TypeScheme(custom) => { + custom.validate.validate(args, def, exts)?; + (&custom.poly_func, args) + } + SignatureFunc::CustomFunc(func) => { + let static_params = func.static_params(); + let (static_args, other_args) = args.split_at(min(static_params.len(), args.len())); + + check_type_args(static_args, static_params)?; + temp = func.compute_signature(static_args, def, exts)?; + (&temp, other_args) + } + }; + + let res = pf.instantiate(args, exts)?; + // TODO bring this assert back once resource inference is done? + // https://github.com/CQCL-DEV/hugr/issues/425 + // assert!(res.contains(self.extension())); + Ok(res) + } } impl Debug for SignatureFunc { @@ -306,27 +334,7 @@ impl OpDef { args: &[TypeArg], exts: &ExtensionRegistry, ) -> Result { - let temp: PolyFuncType; - let (pf, args) = match &self.signature_func { - SignatureFunc::TypeScheme(custom) => { - custom.validate.validate(args, self, exts)?; - (&custom.poly_func, args) - } - SignatureFunc::CustomFunc(func) => { - let static_params = func.static_params(); - let (static_args, other_args) = args.split_at(min(static_params.len(), args.len())); - - check_type_args(static_args, static_params)?; - temp = func.compute_signature(static_args, self, exts)?; - (&temp, other_args) - } - }; - - let res = pf.instantiate(args, exts)?; - // TODO bring this assert back once resource inference is done? - // https://github.com/CQCL-DEV/hugr/issues/425 - // assert!(res.contains(self.extension())); - Ok(res) + self.signature_func.compute_signature(self, args, exts) } pub(crate) fn should_serialize_signature(&self) -> bool { diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index 1363e621b..72649def2 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -5,13 +5,16 @@ use strum::IntoEnumIterator; use crate::{ ops::{custom::ExtensionOp, LeafOp, OpType}, - types::TypeArg, + types::{FunctionType, TypeArg}, Extension, }; -use super::{op_def::SignatureFunc, ExtensionBuildError, ExtensionId, ExtensionRegistry, OpDef}; +use super::{ + op_def::SignatureFunc, ExtensionBuildError, ExtensionId, ExtensionRegistry, OpDef, + SignatureError, +}; +use delegate::delegate; use thiserror::Error; - /// Error when definition extension does not match that of the [OpEnum] #[derive(Debug, Error, PartialEq)] #[error("Expected extension ID {expected} but found {provided}.")] @@ -29,13 +32,22 @@ pub enum OpLoadError { LoadError(T), } +trait IntoStaticSt { + fn to_static_str(&self) -> &str; +} + +impl IntoStaticSt for T +where + for<'a> &'a T: Into<&'static str>, +{ + fn to_static_str(&self) -> &str { + self.into() + } +} /// A trait that operation sets defined by simple (C-style) enums can implement /// to simplify interactions with the extension. /// Relies on `strum_macros::{EnumIter, EnumString, IntoStaticStr}` -pub trait OpEnum: Into<&'static str> + FromStr + Copy + IntoEnumIterator { - /// The name of the extension these ops belong to. - const EXTENSION_ID: ExtensionId; - +pub trait OpEnum: FromStr + IntoEnumIterator + IntoStaticSt { // TODO can be removed after rust 1.75 /// Error thrown when loading from string fails. type LoadError: std::error::Error; @@ -43,33 +55,26 @@ pub trait OpEnum: Into<&'static str> + FromStr + Copy + IntoEnumIterator { type Description: ToString; /// Return the signature (polymorphic function type) of the operation. - fn signature(&self) -> SignatureFunc; + fn def_signature(&self) -> SignatureFunc; /// Description of the operation. fn description(&self) -> Self::Description; - /// Edit the opdef before finalising. + /// Any type args which define this operation. Default is no type arguments. + fn type_args(&self) -> Vec { + vec![] + } + + /// Edit the opdef before finalising. By default does nothing. fn post_opdef(&self, _def: &mut OpDef) {} /// Name of the operation - derived from strum serialization. fn name(&self) -> &str { - (*self).into() + self.to_static_str() } - /// Load an operation from the name of the operation. - fn from_extension_name(op_name: &str) -> Result; - /// Try to load one of the operations of this set from an [OpDef]. - fn try_from_op_def(op_def: &OpDef) -> Result> { - if op_def.extension() != &Self::EXTENSION_ID { - return Err(WrongExtension { - expected: Self::EXTENSION_ID.clone(), - provided: op_def.extension().clone(), - } - .into()); - } - Self::from_extension_name(op_def.name()).map_err(OpLoadError::LoadError) - } + fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result; /// Add an operation to an extension. fn add_to_extension<'e>( @@ -79,7 +84,7 @@ pub trait OpEnum: Into<&'static str> + FromStr + Copy + IntoEnumIterator { let def = ext.add_op( self.name().into(), self.description().to_string(), - self.signature(), + self.def_signature(), )?; self.post_opdef(def); @@ -87,7 +92,8 @@ pub trait OpEnum: Into<&'static str> + FromStr + Copy + IntoEnumIterator { Ok(def) } - /// Iterator over all operations in the set. + /// Iterator over all operations in the set. Non-trivial variants will have + /// default values used for the members. fn all_variants() -> ::Iterator { ::iter() } @@ -100,31 +106,73 @@ pub trait OpEnum: Into<&'static str> + FromStr + Copy + IntoEnumIterator { Ok(()) } + /// Try to instantiate a variant from an [OpType]. Default behaviour assumes + /// an [ExtensionOp] and loads from the name. + fn from_optype(op: &OpType) -> Option { + let ext: &ExtensionOp = op.as_leaf_op()?.as_extension_op()?; + Self::from_op_def(ext.def(), ext.args()).ok() + } + + fn to_registered<'r>( + self, + extension_id: ExtensionId, + registry: &'r ExtensionRegistry, + ) -> RegisteredEnum<'r, Self> { + RegisteredEnum { + extension_id, + registry, + op_enum: self, + } + } +} + +pub struct RegisteredEnum<'r, T> { + /// The name of the extension these ops belong to. + extension_id: ExtensionId, + registry: &'r ExtensionRegistry, + op_enum: T, +} + +impl<'a, T: OpEnum> RegisteredEnum<'a, T> { /// Generate an [OpType]. - fn to_optype( - &self, - extension: &Extension, - args: &[TypeArg], - exts: &ExtensionRegistry, - ) -> Option { - let leaf: LeafOp = ExtensionOp::new(extension.get_op(self.name())?.clone(), args, exts) - .ok()? - .into(); + pub fn to_optype(&self) -> Option { + let leaf: LeafOp = ExtensionOp::new( + self.registry + .get(&self.extension_id)? + .get_op(self.name())? + .clone(), + self.type_args(), + self.registry, + ) + .ok()? + .into(); Some(leaf.into()) } - /// Try to instantiate a variant from an [OpType]. Default behaviour assumes - /// an [ExtensionOp] and loads from the name. - fn from_optype(op: &OpType) -> Option { - let ext: &ExtensionOp = op.as_leaf_op()?.as_extension_op()?; - Self::try_from_op_def(ext.def()).ok() + pub fn function_type(&self) -> Result { + self.op_enum.def_signature().compute_signature( + self.registry + .get(&self.extension_id) + .expect("should return 'Extension not in registry' error here.") + .get_op(self.name()) + .expect("should return 'Op not in extension' error here."), + &self.type_args(), + self.registry, + ) + } + delegate! { + to self.op_enum { + pub fn name(&self) -> &str; + pub fn type_args(&self) -> Vec; + pub fn description(&self) -> T::Description; + } } } #[cfg(test)] mod test { - use crate::{extension::EMPTY_REG, type_row, types::FunctionType}; + use crate::{type_row, types::FunctionType}; use super::*; use strum_macros::{EnumIter, EnumString, IntoStaticStr}; @@ -136,13 +184,11 @@ mod test { #[error("Dummy")] struct DummyError; impl OpEnum for DummyEnum { - const EXTENSION_ID: ExtensionId = ExtensionId::new_unchecked("dummy"); - type LoadError = DummyError; type Description = &'static str; - fn signature(&self) -> SignatureFunc { + fn def_signature(&self) -> SignatureFunc { FunctionType::new_endo(type_row![]).into() } @@ -150,7 +196,7 @@ mod test { "dummy" } - fn from_extension_name(_op_name: &str) -> Result { + fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result { Ok(Self::Dumb) } } @@ -159,31 +205,28 @@ mod test { fn test_dummy_enum() { let o = DummyEnum::Dumb; - let good_name = ExtensionId::new("dummy").unwrap(); - let mut e = Extension::new(good_name.clone()); + let ext_name = ExtensionId::new("dummy").unwrap(); + let mut e = Extension::new(ext_name.clone()); o.add_to_extension(&mut e).unwrap(); assert_eq!( - DummyEnum::try_from_op_def(e.get_op(o.name()).unwrap()).unwrap(), + DummyEnum::from_op_def(e.get_op(o.name()).unwrap(), &[]).unwrap(), o ); assert_eq!( - DummyEnum::from_optype(&o.to_optype(&e, &[], &EMPTY_REG).unwrap()).unwrap(), + DummyEnum::from_optype( + &o.clone() + .to_registered( + ext_name, + &ExtensionRegistry::try_new([e.to_owned()]).unwrap() + ) + .to_optype() + .unwrap() + ) + .unwrap(), o ); - let bad_name = ExtensionId::new("not_dummy").unwrap(); - let mut e = Extension::new(bad_name.clone()); - - o.add_to_extension(&mut e).unwrap(); - - assert_eq!( - DummyEnum::try_from_op_def(e.get_op(o.name()).unwrap()), - Err(OpLoadError::WrongExtension(WrongExtension { - expected: good_name, - provided: bad_name - })) - ); } } diff --git a/src/std_extensions/logic.rs b/src/std_extensions/logic.rs index 2738d1455..b9c923934 100644 --- a/src/std_extensions/logic.rs +++ b/src/std_extensions/logic.rs @@ -4,7 +4,7 @@ use strum_macros::{EnumIter, EnumString, IntoStaticStr}; use crate::{ extension::{ - prelude::BOOL_T, simple_op::OpEnum, ExtensionId, SignatureError, SignatureFromArgs, + prelude::BOOL_T, simple_op::OpEnum, ExtensionId, OpDef, SignatureError, SignatureFromArgs, SignatureFunc, }, ops, type_row, @@ -23,42 +23,62 @@ pub const FALSE_NAME: &str = "FALSE"; pub const TRUE_NAME: &str = "TRUE"; /// Logic extension operations. -#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, EnumIter, IntoStaticStr, EnumString)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, EnumIter, IntoStaticStr, EnumString)] #[allow(missing_docs)] pub enum LogicOp { - And, - Or, + And(u64), + Or(u64), Not, } /// Error in trying to load logic operation. #[derive(Debug, Error)] -#[error("Not a logic extension operation.")] -pub struct NotLogicOp; +pub enum LogicOpLoadError { + #[error("Not a logic extension operation.")] + NotLogicOp, + #[error("Type args invalid: {0}.")] + InvalidArgs(#[from] SignatureError), +} impl OpEnum for LogicOp { - const EXTENSION_ID: ExtensionId = EXTENSION_ID; - type LoadError = NotLogicOp; + type LoadError = LogicOpLoadError; type Description = &'static str; - fn from_extension_name(op_name: &str) -> Result { - Self::from_str(op_name).map_err(|_| NotLogicOp) + fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result { + let mut out = Self::from_str(op_def.name()).map_err(|_| LogicOpLoadError::NotLogicOp)?; + match &mut out { + LogicOp::And(i) | LogicOp::Or(i) => { + let [TypeArg::BoundedNat { n }] = *args else { + return Err(SignatureError::InvalidTypeArgs.into()); + }; + *i = n; + } + LogicOp::Not => (), + } + + Ok(out) } - fn signature(&self) -> SignatureFunc { + fn def_signature(&self) -> SignatureFunc { match self { - LogicOp::Or | LogicOp::And => logic_op_sig().into(), + LogicOp::Or(_) | LogicOp::And(_) => logic_op_sig().into(), LogicOp::Not => FunctionType::new_endo(type_row![BOOL_T]).into(), } } fn description(&self) -> &'static str { match self { - LogicOp::And => "logical 'and'", - LogicOp::Or => "logical 'or'", + LogicOp::And(_) => "logical 'and'", + LogicOp::Or(_) => "logical 'or'", LogicOp::Not => "logical 'not'", } } + fn type_args(&self) -> Vec { + match self { + LogicOp::And(n) | LogicOp::Or(n) => vec![TypeArg::BoundedNat { n: *n }], + LogicOp::Not => vec![], + } + } } /// The extension identifier. pub const EXTENSION_ID: ExtensionId = ExtensionId::new_unchecked("logic"); @@ -107,15 +127,18 @@ lazy_static! { #[cfg(test)] pub(crate) mod test { + use super::{extension, LogicOp, EXTENSION, EXTENSION_ID, FALSE_NAME, TRUE_NAME}; use crate::{ - extension::{prelude::BOOL_T, simple_op::OpEnum, EMPTY_REG}, + extension::{prelude::BOOL_T, simple_op::OpEnum, ExtensionRegistry}, ops::OpType, - types::type_param::TypeArg, + types::TypeArg, Extension, }; - - use super::{extension, LogicOp, EXTENSION, FALSE_NAME, TRUE_NAME}; - + use lazy_static::lazy_static; + lazy_static! { + pub(crate) static ref LOGIC_REG: ExtensionRegistry = + ExtensionRegistry::try_new([EXTENSION.to_owned()]).unwrap(); + } #[test] fn test_logic_extension() { let r: Extension = extension(); @@ -124,7 +147,12 @@ pub(crate) mod test { for op in LogicOp::all_variants() { assert_eq!( - LogicOp::try_from_op_def(r.get_op(op.name()).unwrap()).unwrap(), + LogicOp::from_op_def( + r.get_op(op.name()).unwrap(), + // `all_variants` will set default type arg values. + &[TypeArg::BoundedNat { n: 0 }] + ) + .unwrap(), op ); } @@ -144,20 +172,25 @@ pub(crate) mod test { /// Generate a logic extension and "and" operation over [`crate::prelude::BOOL_T`] pub(crate) fn and_op() -> OpType { - LogicOp::And - .to_optype(&EXTENSION, &[TypeArg::BoundedNat { n: 2 }], &EMPTY_REG) + LogicOp::And(2) + .to_registered(EXTENSION_ID.to_owned(), &LOGIC_REG) + .to_optype() .unwrap() } /// Generate a logic extension and "or" operation over [`crate::prelude::BOOL_T`] pub(crate) fn or_op() -> OpType { - LogicOp::Or - .to_optype(&EXTENSION, &[TypeArg::BoundedNat { n: 2 }], &EMPTY_REG) + LogicOp::Or(2) + .to_registered(EXTENSION_ID.to_owned(), &LOGIC_REG) + .to_optype() .unwrap() } /// Generate a logic extension and "not" operation over [`crate::prelude::BOOL_T`] pub(crate) fn not_op() -> OpType { - LogicOp::Not.to_optype(&EXTENSION, &[], &EMPTY_REG).unwrap() + LogicOp::Not + .to_registered(EXTENSION_ID.to_owned(), &LOGIC_REG) + .to_optype() + .unwrap() } } From e6a3ce60a7374858e41d76b10209ad00ef2e9d4c Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 12:44:00 +0000 Subject: [PATCH 04/14] move op adding to extension --- src/extension/op_def.rs | 15 +++++++++++++++ src/extension/simple_op.rs | 38 +++++++++++--------------------------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 639224933..335ef14be 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -435,6 +435,21 @@ impl Extension { Entry::Vacant(ve) => Ok(Arc::get_mut(ve.insert(Arc::new(op))).unwrap()), } } + + pub fn add_op_enum( + &mut self, + op: &impl super::simple_op::OpEnum, + ) -> Result<&mut OpDef, ExtensionBuildError> { + let def = self.add_op( + op.name().into(), + op.description().to_string(), + op.def_signature(), + )?; + + op.post_opdef(def); + + Ok(def) + } } #[cfg(test)] diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index 72649def2..ed8584133 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -76,36 +76,12 @@ pub trait OpEnum: FromStr + IntoEnumIterator + IntoStaticSt { /// Try to load one of the operations of this set from an [OpDef]. fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result; - /// Add an operation to an extension. - fn add_to_extension<'e>( - &self, - ext: &'e mut Extension, - ) -> Result<&'e OpDef, ExtensionBuildError> { - let def = ext.add_op( - self.name().into(), - self.description().to_string(), - self.def_signature(), - )?; - - self.post_opdef(def); - - Ok(def) - } - /// Iterator over all operations in the set. Non-trivial variants will have /// default values used for the members. fn all_variants() -> ::Iterator { ::iter() } - /// load all variants of a `SimpleOpEnum` in to an extension as op defs. - fn load_all_ops(extension: &mut Extension) -> Result<(), ExtensionBuildError> { - for op in Self::all_variants() { - op.add_to_extension(extension)?; - } - Ok(()) - } - /// Try to instantiate a variant from an [OpType]. Default behaviour assumes /// an [ExtensionOp] and loads from the name. fn from_optype(op: &OpType) -> Option { @@ -124,6 +100,14 @@ pub trait OpEnum: FromStr + IntoEnumIterator + IntoStaticSt { op_enum: self, } } + + /// load all variants of a [OpEnum] in to an extension as op defs. + fn load_all_ops(extension: &mut Extension) -> Result<(), ExtensionBuildError> { + for op in Self::all_variants() { + extension.add_op_enum(&op)?; + } + Ok(()) + } } pub struct RegisteredEnum<'r, T> { @@ -176,7 +160,7 @@ mod test { use super::*; use strum_macros::{EnumIter, EnumString, IntoStaticStr}; - #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, EnumIter, IntoStaticStr, EnumString)] + #[derive(Clone, Debug, Hash, PartialEq, Eq, EnumIter, IntoStaticStr, EnumString)] enum DummyEnum { Dumb, } @@ -196,7 +180,7 @@ mod test { "dummy" } - fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result { + fn from_op_def(_op_def: &OpDef, _args: &[TypeArg]) -> Result { Ok(Self::Dumb) } } @@ -208,7 +192,7 @@ mod test { let ext_name = ExtensionId::new("dummy").unwrap(); let mut e = Extension::new(ext_name.clone()); - o.add_to_extension(&mut e).unwrap(); + e.add_op_enum(&o).unwrap(); assert_eq!( DummyEnum::from_op_def(e.get_op(o.name()).unwrap(), &[]).unwrap(), From 53a39917c223002c2ff2944508e4fac1b486a94b Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 14:13:02 +0000 Subject: [PATCH 05/14] add some docstrings --- src/extension/simple_op.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index ed8584133..d90244e72 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -110,10 +110,14 @@ pub trait OpEnum: FromStr + IntoEnumIterator + IntoStaticSt { } } +/// Wrap an [OpEnum] with an extension registry to allow type computation. +/// Generate from [OpEnum::to_registered] pub struct RegisteredEnum<'r, T> { /// The name of the extension these ops belong to. extension_id: ExtensionId, + /// A registry of all extensions, used for type computation. registry: &'r ExtensionRegistry, + /// The inner [OpEnum] op_enum: T, } @@ -134,6 +138,7 @@ impl<'a, T: OpEnum> RegisteredEnum<'a, T> { Some(leaf.into()) } + /// Compute the [FunctionType] for this operation, instantiating with type arguments. pub fn function_type(&self) -> Result { self.op_enum.def_signature().compute_signature( self.registry @@ -147,8 +152,11 @@ impl<'a, T: OpEnum> RegisteredEnum<'a, T> { } delegate! { to self.op_enum { + /// Name of the operation - derived from strum serialization. pub fn name(&self) -> &str; + /// Any type args which define this operation. Default is no type arguments. pub fn type_args(&self) -> Vec; + /// Description of the operation. pub fn description(&self) -> T::Description; } } From b1723eede1e6e8d98456eb8a9e5b4739d992db58 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 14:27:27 +0000 Subject: [PATCH 06/14] localise trait requirements --- src/extension/op_def.rs | 2 +- src/extension/simple_op.rs | 50 +++++++++++++++++++++---------------- src/std_extensions/logic.rs | 6 ++++- 3 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 335ef14be..4b8532942 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -438,7 +438,7 @@ impl Extension { pub fn add_op_enum( &mut self, - op: &impl super::simple_op::OpEnum, + op: &(impl super::simple_op::OpEnum + super::simple_op::OpEnumName), ) -> Result<&mut OpDef, ExtensionBuildError> { let def = self.add_op( op.name().into(), diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index d90244e72..157581ead 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -1,5 +1,4 @@ //! A trait that enum for op definitions that gathers up some shared functionality. -use std::str::FromStr; use strum::IntoEnumIterator; @@ -32,22 +31,22 @@ pub enum OpLoadError { LoadError(T), } -trait IntoStaticSt { - fn to_static_str(&self) -> &str; +pub trait OpEnumName { + fn name(&self) -> &str; } -impl IntoStaticSt for T +impl OpEnumName for T where for<'a> &'a T: Into<&'static str>, { - fn to_static_str(&self) -> &str { + fn name(&self) -> &str { self.into() } } /// A trait that operation sets defined by simple (C-style) enums can implement /// to simplify interactions with the extension. /// Relies on `strum_macros::{EnumIter, EnumString, IntoStaticStr}` -pub trait OpEnum: FromStr + IntoEnumIterator + IntoStaticSt { +pub trait OpEnum: OpEnumName { // TODO can be removed after rust 1.75 /// Error thrown when loading from string fails. type LoadError: std::error::Error; @@ -68,23 +67,17 @@ pub trait OpEnum: FromStr + IntoEnumIterator + IntoStaticSt { /// Edit the opdef before finalising. By default does nothing. fn post_opdef(&self, _def: &mut OpDef) {} - /// Name of the operation - derived from strum serialization. - fn name(&self) -> &str { - self.to_static_str() - } - /// Try to load one of the operations of this set from an [OpDef]. - fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result; - - /// Iterator over all operations in the set. Non-trivial variants will have - /// default values used for the members. - fn all_variants() -> ::Iterator { - ::iter() - } + fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result + where + Self: Sized; /// Try to instantiate a variant from an [OpType]. Default behaviour assumes /// an [ExtensionOp] and loads from the name. - fn from_optype(op: &OpType) -> Option { + fn from_optype(op: &OpType) -> Option + where + Self: Sized, + { let ext: &ExtensionOp = op.as_leaf_op()?.as_extension_op()?; Self::from_op_def(ext.def(), ext.args()).ok() } @@ -93,7 +86,10 @@ pub trait OpEnum: FromStr + IntoEnumIterator + IntoStaticSt { self, extension_id: ExtensionId, registry: &'r ExtensionRegistry, - ) -> RegisteredEnum<'r, Self> { + ) -> RegisteredEnum<'r, Self> + where + Self: Sized, + { RegisteredEnum { extension_id, registry, @@ -101,8 +97,20 @@ pub trait OpEnum: FromStr + IntoEnumIterator + IntoStaticSt { } } + /// Iterator over all operations in the set. Non-trivial variants will have + /// default values used for the members. + fn all_variants() -> ::Iterator + where + Self: IntoEnumIterator, + { + ::iter() + } + /// load all variants of a [OpEnum] in to an extension as op defs. - fn load_all_ops(extension: &mut Extension) -> Result<(), ExtensionBuildError> { + fn load_all_ops(extension: &mut Extension) -> Result<(), ExtensionBuildError> + where + Self: IntoEnumIterator, + { for op in Self::all_variants() { extension.add_op_enum(&op)?; } diff --git a/src/std_extensions/logic.rs b/src/std_extensions/logic.rs index b9c923934..3567c3b80 100644 --- a/src/std_extensions/logic.rs +++ b/src/std_extensions/logic.rs @@ -129,7 +129,11 @@ lazy_static! { pub(crate) mod test { use super::{extension, LogicOp, EXTENSION, EXTENSION_ID, FALSE_NAME, TRUE_NAME}; use crate::{ - extension::{prelude::BOOL_T, simple_op::OpEnum, ExtensionRegistry}, + extension::{ + prelude::BOOL_T, + simple_op::{OpEnum, OpEnumName}, + ExtensionRegistry, + }, ops::OpType, types::TypeArg, Extension, From b1fde5ccf61d43d1d7b259ebd745a225f0d3bc83 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 14:57:49 +0000 Subject: [PATCH 07/14] simplify loading from string --- src/extension/simple_op.rs | 26 +++++++++++++++++--------- src/std_extensions/logic.rs | 21 +++++---------------- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index 157581ead..d4adb02c3 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -26,9 +26,12 @@ pub struct WrongExtension { #[derive(Debug, Error, PartialEq)] #[error("{0}")] #[allow(missing_docs)] -pub enum OpLoadError { +pub enum OpLoadError { WrongExtension(#[from] WrongExtension), - LoadError(T), + #[error("Op with name {0} is not a member of this enum.")] + NotEnumMember(String), + #[error("Type args invalid: {0}.")] + InvalidArgs(#[from] SignatureError), } pub trait OpEnumName { @@ -47,9 +50,6 @@ where /// to simplify interactions with the extension. /// Relies on `strum_macros::{EnumIter, EnumString, IntoStaticStr}` pub trait OpEnum: OpEnumName { - // TODO can be removed after rust 1.75 - /// Error thrown when loading from string fails. - type LoadError: std::error::Error; /// Description type. type Description: ToString; @@ -68,7 +68,7 @@ pub trait OpEnum: OpEnumName { fn post_opdef(&self, _def: &mut OpDef) {} /// Try to load one of the operations of this set from an [OpDef]. - fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result + fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result where Self: Sized; @@ -118,6 +118,16 @@ pub trait OpEnum: OpEnumName { } } +/// Load an [OpEnum] from its name. Works best for C-style enums where each +/// variant corresponds to an [OpDef] and an [OpType], i,e, there are no type parameters. +/// See [strum_macros::EnumString]. +pub fn try_from_name(name: &str) -> Result +where + T: std::str::FromStr + OpEnum, +{ + T::from_str(name).map_err(|_| OpLoadError::NotEnumMember(name.to_string())) +} + /// Wrap an [OpEnum] with an extension registry to allow type computation. /// Generate from [OpEnum::to_registered] pub struct RegisteredEnum<'r, T> { @@ -184,8 +194,6 @@ mod test { #[error("Dummy")] struct DummyError; impl OpEnum for DummyEnum { - type LoadError = DummyError; - type Description = &'static str; fn def_signature(&self) -> SignatureFunc { @@ -196,7 +204,7 @@ mod test { "dummy" } - fn from_op_def(_op_def: &OpDef, _args: &[TypeArg]) -> Result { + fn from_op_def(_op_def: &OpDef, _args: &[TypeArg]) -> Result { Ok(Self::Dumb) } } diff --git a/src/std_extensions/logic.rs b/src/std_extensions/logic.rs index 3567c3b80..622dd76de 100644 --- a/src/std_extensions/logic.rs +++ b/src/std_extensions/logic.rs @@ -4,8 +4,9 @@ use strum_macros::{EnumIter, EnumString, IntoStaticStr}; use crate::{ extension::{ - prelude::BOOL_T, simple_op::OpEnum, ExtensionId, OpDef, SignatureError, SignatureFromArgs, - SignatureFunc, + prelude::BOOL_T, + simple_op::{try_from_name, OpEnum, OpLoadError}, + ExtensionId, OpDef, SignatureError, SignatureFromArgs, SignatureFunc, }, ops, type_row, types::{ @@ -15,8 +16,6 @@ use crate::{ Extension, }; use lazy_static::lazy_static; -use std::str::FromStr; -use thiserror::Error; /// Name of extension false value. pub const FALSE_NAME: &str = "FALSE"; /// Name of extension true value. @@ -31,21 +30,11 @@ pub enum LogicOp { Not, } -/// Error in trying to load logic operation. -#[derive(Debug, Error)] -pub enum LogicOpLoadError { - #[error("Not a logic extension operation.")] - NotLogicOp, - #[error("Type args invalid: {0}.")] - InvalidArgs(#[from] SignatureError), -} - impl OpEnum for LogicOp { - type LoadError = LogicOpLoadError; type Description = &'static str; - fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result { - let mut out = Self::from_str(op_def.name()).map_err(|_| LogicOpLoadError::NotLogicOp)?; + fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result { + let mut out: LogicOp = try_from_name(op_def.name())?; match &mut out { LogicOp::And(i) | LogicOp::Or(i) => { let [TypeArg::BoundedNat { n }] = *args else { From a35c96a921bbdd10b8840975a9e999f781906740 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 14:58:17 +0000 Subject: [PATCH 08/14] use OpEnum instead of OpEnumName --- src/extension/op_def.rs | 9 +++------ src/extension/simple_op.rs | 24 +++++++++++------------- src/std_extensions/logic.rs | 10 +++------- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 4b8532942..44bcbd2c9 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -10,6 +10,7 @@ use super::{ Extension, ExtensionBuildError, ExtensionId, ExtensionRegistry, ExtensionSet, SignatureError, }; +use crate::ops::OpName; use crate::types::type_param::{check_type_args, TypeArg, TypeParam}; use crate::types::{FunctionType, PolyFuncType}; use crate::Hugr; @@ -438,13 +439,9 @@ impl Extension { pub fn add_op_enum( &mut self, - op: &(impl super::simple_op::OpEnum + super::simple_op::OpEnumName), + op: &(impl super::simple_op::OpEnum + OpName), ) -> Result<&mut OpDef, ExtensionBuildError> { - let def = self.add_op( - op.name().into(), - op.description().to_string(), - op.def_signature(), - )?; + let def = self.add_op(op.name(), op.description().to_string(), op.def_signature())?; op.post_opdef(def); diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index d4adb02c3..b7aa89fa8 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -1,9 +1,10 @@ //! A trait that enum for op definitions that gathers up some shared functionality. +use smol_str::SmolStr; use strum::IntoEnumIterator; use crate::{ - ops::{custom::ExtensionOp, LeafOp, OpType}, + ops::{custom::ExtensionOp, LeafOp, OpName, OpType}, types::{FunctionType, TypeArg}, Extension, }; @@ -34,22 +35,19 @@ pub enum OpLoadError { InvalidArgs(#[from] SignatureError), } -pub trait OpEnumName { - fn name(&self) -> &str; -} - -impl OpEnumName for T +impl OpName for T where for<'a> &'a T: Into<&'static str>, { - fn name(&self) -> &str { - self.into() + fn name(&self) -> SmolStr { + let s = self.into(); + s.into() } } /// A trait that operation sets defined by simple (C-style) enums can implement /// to simplify interactions with the extension. /// Relies on `strum_macros::{EnumIter, EnumString, IntoStaticStr}` -pub trait OpEnum: OpEnumName { +pub trait OpEnum: OpName { /// Description type. type Description: ToString; @@ -145,7 +143,7 @@ impl<'a, T: OpEnum> RegisteredEnum<'a, T> { let leaf: LeafOp = ExtensionOp::new( self.registry .get(&self.extension_id)? - .get_op(self.name())? + .get_op(&self.name())? .clone(), self.type_args(), self.registry, @@ -162,7 +160,7 @@ impl<'a, T: OpEnum> RegisteredEnum<'a, T> { self.registry .get(&self.extension_id) .expect("should return 'Extension not in registry' error here.") - .get_op(self.name()) + .get_op(&self.name()) .expect("should return 'Op not in extension' error here."), &self.type_args(), self.registry, @@ -171,7 +169,7 @@ impl<'a, T: OpEnum> RegisteredEnum<'a, T> { delegate! { to self.op_enum { /// Name of the operation - derived from strum serialization. - pub fn name(&self) -> &str; + pub fn name(&self) -> SmolStr; /// Any type args which define this operation. Default is no type arguments. pub fn type_args(&self) -> Vec; /// Description of the operation. @@ -219,7 +217,7 @@ mod test { e.add_op_enum(&o).unwrap(); assert_eq!( - DummyEnum::from_op_def(e.get_op(o.name()).unwrap(), &[]).unwrap(), + DummyEnum::from_op_def(e.get_op(&o.name()).unwrap(), &[]).unwrap(), o ); diff --git a/src/std_extensions/logic.rs b/src/std_extensions/logic.rs index 622dd76de..368060ca4 100644 --- a/src/std_extensions/logic.rs +++ b/src/std_extensions/logic.rs @@ -118,12 +118,8 @@ lazy_static! { pub(crate) mod test { use super::{extension, LogicOp, EXTENSION, EXTENSION_ID, FALSE_NAME, TRUE_NAME}; use crate::{ - extension::{ - prelude::BOOL_T, - simple_op::{OpEnum, OpEnumName}, - ExtensionRegistry, - }, - ops::OpType, + extension::{prelude::BOOL_T, simple_op::OpEnum, ExtensionRegistry}, + ops::{OpName, OpType}, types::TypeArg, Extension, }; @@ -141,7 +137,7 @@ pub(crate) mod test { for op in LogicOp::all_variants() { assert_eq!( LogicOp::from_op_def( - r.get_op(op.name()).unwrap(), + r.get_op(&op.name()).unwrap(), // `all_variants` will set default type arg values. &[TypeArg::BoundedNat { n: 0 }] ) From 027f54c8cd18757d92accff97c45a6bbbd815782 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 15:19:33 +0000 Subject: [PATCH 09/14] simplifications and cleanups --- src/extension/op_def.rs | 2 +- src/extension/simple_op.rs | 74 +++++++++++++++++-------------------- src/std_extensions/logic.rs | 5 +-- 3 files changed, 36 insertions(+), 45 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 44bcbd2c9..7e95300ab 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -441,7 +441,7 @@ impl Extension { &mut self, op: &(impl super::simple_op::OpEnum + OpName), ) -> Result<&mut OpDef, ExtensionBuildError> { - let def = self.add_op(op.name(), op.description().to_string(), op.def_signature())?; + let def = self.add_op(op.name(), op.description(), op.def_signature())?; op.post_opdef(def); diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index b7aa89fa8..474cf2a50 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -15,20 +15,12 @@ use super::{ }; use delegate::delegate; use thiserror::Error; -/// Error when definition extension does not match that of the [OpEnum] -#[derive(Debug, Error, PartialEq)] -#[error("Expected extension ID {expected} but found {provided}.")] -pub struct WrongExtension { - expected: ExtensionId, - provided: ExtensionId, -} /// Error loading [OpEnum] #[derive(Debug, Error, PartialEq)] #[error("{0}")] #[allow(missing_docs)] pub enum OpLoadError { - WrongExtension(#[from] WrongExtension), #[error("Op with name {0} is not a member of this enum.")] NotEnumMember(String), #[error("Type args invalid: {0}.")] @@ -48,14 +40,18 @@ where /// to simplify interactions with the extension. /// Relies on `strum_macros::{EnumIter, EnumString, IntoStaticStr}` pub trait OpEnum: OpName { - /// Description type. - type Description: ToString; + /// Try to load one of the operations of this set from an [OpDef]. + fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result + where + Self: Sized; /// Return the signature (polymorphic function type) of the operation. fn def_signature(&self) -> SignatureFunc; - /// Description of the operation. - fn description(&self) -> Self::Description; + /// Description of the operation. By default, the same as `self.name()`. + fn description(&self) -> String { + self.name().to_string() + } /// Any type args which define this operation. Default is no type arguments. fn type_args(&self) -> Vec { @@ -65,11 +61,6 @@ pub trait OpEnum: OpName { /// Edit the opdef before finalising. By default does nothing. fn post_opdef(&self, _def: &mut OpDef) {} - /// Try to load one of the operations of this set from an [OpDef]. - fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result - where - Self: Sized; - /// Try to instantiate a variant from an [OpType]. Default behaviour assumes /// an [ExtensionOp] and loads from the name. fn from_optype(op: &OpType) -> Option @@ -80,11 +71,11 @@ pub trait OpEnum: OpName { Self::from_op_def(ext.def(), ext.args()).ok() } - fn to_registered<'r>( + fn to_registered( self, extension_id: ExtensionId, - registry: &'r ExtensionRegistry, - ) -> RegisteredEnum<'r, Self> + registry: &ExtensionRegistry, + ) -> RegisteredEnum<'_, Self> where Self: Sized, { @@ -137,7 +128,14 @@ pub struct RegisteredEnum<'r, T> { op_enum: T, } -impl<'a, T: OpEnum> RegisteredEnum<'a, T> { +impl RegisteredEnum<'_, T> { + /// Extract the inner wrapped value + pub fn to_inner(self) -> T { + self.op_enum + } +} + +impl RegisteredEnum<'_, T> { /// Generate an [OpType]. pub fn to_optype(&self) -> Option { let leaf: LeafOp = ExtensionOp::new( @@ -166,6 +164,7 @@ impl<'a, T: OpEnum> RegisteredEnum<'a, T> { self.registry, ) } + delegate! { to self.op_enum { /// Name of the operation - derived from strum serialization. @@ -173,7 +172,7 @@ impl<'a, T: OpEnum> RegisteredEnum<'a, T> { /// Any type args which define this operation. Default is no type arguments. pub fn type_args(&self) -> Vec; /// Description of the operation. - pub fn description(&self) -> T::Description; + pub fn description(&self) -> String; } } } @@ -188,20 +187,12 @@ mod test { enum DummyEnum { Dumb, } - #[derive(Debug, thiserror::Error, PartialEq)] - #[error("Dummy")] - struct DummyError; - impl OpEnum for DummyEnum { - type Description = &'static str; + impl OpEnum for DummyEnum { fn def_signature(&self) -> SignatureFunc { FunctionType::new_endo(type_row![]).into() } - fn description(&self) -> Self::Description { - "dummy" - } - fn from_op_def(_op_def: &OpDef, _args: &[TypeArg]) -> Result { Ok(Self::Dumb) } @@ -221,18 +212,19 @@ mod test { o ); + let registry = ExtensionRegistry::try_new([e.to_owned()]).unwrap(); + let registered = o.clone().to_registered(ext_name, ®istry); assert_eq!( - DummyEnum::from_optype( - &o.clone() - .to_registered( - ext_name, - &ExtensionRegistry::try_new([e.to_owned()]).unwrap() - ) - .to_optype() - .unwrap() - ) - .unwrap(), + DummyEnum::from_optype(®istered.to_optype().unwrap()).unwrap(), o ); + assert_eq!( + registered.function_type().unwrap(), + FunctionType::new_endo(type_row![]) + ); + + assert_eq!(registered.description(), "Dumb"); + + assert_eq!(registered.to_inner(), o); } } diff --git a/src/std_extensions/logic.rs b/src/std_extensions/logic.rs index 368060ca4..1d9d78630 100644 --- a/src/std_extensions/logic.rs +++ b/src/std_extensions/logic.rs @@ -31,8 +31,6 @@ pub enum LogicOp { } impl OpEnum for LogicOp { - type Description = &'static str; - fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result { let mut out: LogicOp = try_from_name(op_def.name())?; match &mut out { @@ -55,12 +53,13 @@ impl OpEnum for LogicOp { } } - fn description(&self) -> &'static str { + fn description(&self) -> String { match self { LogicOp::And(_) => "logical 'and'", LogicOp::Or(_) => "logical 'or'", LogicOp::Not => "logical 'not'", } + .to_string() } fn type_args(&self) -> Vec { match self { From acb131efa0f75c7ea0cdfcf469585a5e578b4dd1 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 15:25:23 +0000 Subject: [PATCH 10/14] add missing docstrings --- src/extension/op_def.rs | 17 ++++++++++++++++- src/extension/simple_op.rs | 2 ++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 7e95300ab..c204e8a22 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use smol_str::SmolStr; +use super::simple_op::OpEnum; use super::{ Extension, ExtensionBuildError, ExtensionId, ExtensionRegistry, ExtensionSet, SignatureError, }; @@ -212,6 +213,18 @@ impl SignatureFunc { SignatureFunc::CustomFunc(func) => func.static_params(), } } + + /// Compute the concrete signature ([FunctionType]). + /// + /// # Panics + /// + /// Panics if is [SignatureFunc::CustomFunc] and there are not enough type + /// arguments provided to match the number of static parameters. + /// + /// # Errors + /// + /// This function will return an error if the type arguments are invalid or + /// there is some error in type computation. pub fn compute_signature( &self, def: &OpDef, @@ -437,9 +450,11 @@ impl Extension { } } + /// Add an operation implemented as an [OpEnum], which can provide the data + /// required to define an [OpDef]. pub fn add_op_enum( &mut self, - op: &(impl super::simple_op::OpEnum + OpName), + op: &(impl OpEnum + OpName), ) -> Result<&mut OpDef, ExtensionBuildError> { let def = self.add_op(op.name(), op.description(), op.def_signature())?; diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index 474cf2a50..df5e77323 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -71,6 +71,8 @@ pub trait OpEnum: OpName { Self::from_op_def(ext.def(), ext.args()).ok() } + /// Given the ID of the extension this operation is defined in, and a + /// registry containing that extension, return a [RegisteredEnum]. fn to_registered( self, extension_id: ExtensionId, From 406eccd3d9042cd03409ad54049f4259aa0ce3c0 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 17:19:01 +0000 Subject: [PATCH 11/14] split def-like trait and optype-like trait --- src/extension/op_def.rs | 15 ---- src/extension/simple_op.rs | 150 ++++++++++++++++++------------------ src/ops/custom.rs | 7 ++ src/std_extensions/logic.rs | 143 +++++++++++++++++++++------------- 4 files changed, 169 insertions(+), 146 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index c204e8a22..5f495ad15 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -6,12 +6,10 @@ use std::sync::Arc; use smol_str::SmolStr; -use super::simple_op::OpEnum; use super::{ Extension, ExtensionBuildError, ExtensionId, ExtensionRegistry, ExtensionSet, SignatureError, }; -use crate::ops::OpName; use crate::types::type_param::{check_type_args, TypeArg, TypeParam}; use crate::types::{FunctionType, PolyFuncType}; use crate::Hugr; @@ -449,19 +447,6 @@ impl Extension { Entry::Vacant(ve) => Ok(Arc::get_mut(ve.insert(Arc::new(op))).unwrap()), } } - - /// Add an operation implemented as an [OpEnum], which can provide the data - /// required to define an [OpDef]. - pub fn add_op_enum( - &mut self, - op: &(impl OpEnum + OpName), - ) -> Result<&mut OpDef, ExtensionBuildError> { - let def = self.add_op(op.name(), op.description(), op.def_signature())?; - - op.post_opdef(def); - - Ok(def) - } } #[cfg(test)] diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index df5e77323..e0fb092e1 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -4,8 +4,8 @@ use smol_str::SmolStr; use strum::IntoEnumIterator; use crate::{ - ops::{custom::ExtensionOp, LeafOp, OpName, OpType}, - types::{FunctionType, TypeArg}, + ops::{custom::ExtensionOp, OpName, OpType}, + types::TypeArg, Extension, }; @@ -16,13 +16,13 @@ use super::{ use delegate::delegate; use thiserror::Error; -/// Error loading [OpEnum] +/// Error loading operation. #[derive(Debug, Error, PartialEq)] #[error("{0}")] #[allow(missing_docs)] pub enum OpLoadError { - #[error("Op with name {0} is not a member of this enum.")] - NotEnumMember(String), + #[error("Op with name {0} is not a member of this set.")] + NotMember(String), #[error("Type args invalid: {0}.")] InvalidArgs(#[from] SignatureError), } @@ -36,31 +36,55 @@ where s.into() } } -/// A trait that operation sets defined by simple (C-style) enums can implement -/// to simplify interactions with the extension. -/// Relies on `strum_macros::{EnumIter, EnumString, IntoStaticStr}` -pub trait OpEnum: OpName { + +/// Traits implemented by types which can add themselves to [`Extension`]s as +/// [`OpDef`]s or load themselves from an [`OpDef`] +pub trait MakeOpDef: OpName { /// Try to load one of the operations of this set from an [OpDef]. - fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result + fn from_def(op_def: &OpDef) -> Result where Self: Sized; /// Return the signature (polymorphic function type) of the operation. - fn def_signature(&self) -> SignatureFunc; + fn signature(&self) -> SignatureFunc; /// Description of the operation. By default, the same as `self.name()`. fn description(&self) -> String { self.name().to_string() } - /// Any type args which define this operation. Default is no type arguments. - fn type_args(&self) -> Vec { - vec![] - } - /// Edit the opdef before finalising. By default does nothing. fn post_opdef(&self, _def: &mut OpDef) {} + /// Add an operation implemented as an [MakeOpDef], which can provide the data + /// required to define an [OpDef], to an extension. + fn add_to_extension(&self, extension: &mut Extension) -> Result<(), ExtensionBuildError> { + let def = extension.add_op(self.name(), self.description(), self.signature())?; + + self.post_opdef(def); + + Ok(()) + } + + /// load all variants of a [OpEnum] in to an extension as op defs. + fn load_all_ops(extension: &mut Extension) -> Result<(), ExtensionBuildError> + where + Self: IntoEnumIterator, + { + for op in Self::iter() { + op.add_to_extension(extension)?; + } + Ok(()) + } +} + +/// Traits implemented by types which can be loaded from [`ExtensionOp`]s, +/// i.e. concrete instances of [`OpDef`]s, with defined type arguments. +pub trait MakeExtensionOp: OpName { + /// Try to load one of the operations of this set from an [OpDef]. + fn from_extension_op(ext_op: &ExtensionOp) -> Result + where + Self: Sized; /// Try to instantiate a variant from an [OpType]. Default behaviour assumes /// an [ExtensionOp] and loads from the name. fn from_optype(op: &OpType) -> Option @@ -68,44 +92,43 @@ pub trait OpEnum: OpName { Self: Sized, { let ext: &ExtensionOp = op.as_leaf_op()?.as_extension_op()?; - Self::from_op_def(ext.def(), ext.args()).ok() + Self::from_extension_op(ext).ok() } + /// Any type args which define this operation. + fn type_args(&self) -> Vec; + /// Given the ID of the extension this operation is defined in, and a /// registry containing that extension, return a [RegisteredEnum]. fn to_registered( self, extension_id: ExtensionId, registry: &ExtensionRegistry, - ) -> RegisteredEnum<'_, Self> + ) -> RegisteredOp<'_, Self> where Self: Sized, { - RegisteredEnum { + RegisteredOp { extension_id, registry, - op_enum: self, + op: self, } } +} - /// Iterator over all operations in the set. Non-trivial variants will have - /// default values used for the members. - fn all_variants() -> ::Iterator +/// Blanket implementation for non-polymorphic operations - no type parameters. +impl MakeExtensionOp for T { + #[inline] + fn from_extension_op(ext_op: &ExtensionOp) -> Result where - Self: IntoEnumIterator, + Self: Sized, { - ::iter() + Self::from_def(ext_op.def()) } - /// load all variants of a [OpEnum] in to an extension as op defs. - fn load_all_ops(extension: &mut Extension) -> Result<(), ExtensionBuildError> - where - Self: IntoEnumIterator, - { - for op in Self::all_variants() { - extension.add_op_enum(&op)?; - } - Ok(()) + #[inline] + fn type_args(&self) -> Vec { + vec![] } } @@ -114,33 +137,33 @@ pub trait OpEnum: OpName { /// See [strum_macros::EnumString]. pub fn try_from_name(name: &str) -> Result where - T: std::str::FromStr + OpEnum, + T: std::str::FromStr + MakeOpDef, { - T::from_str(name).map_err(|_| OpLoadError::NotEnumMember(name.to_string())) + T::from_str(name).map_err(|_| OpLoadError::NotMember(name.to_string())) } /// Wrap an [OpEnum] with an extension registry to allow type computation. /// Generate from [OpEnum::to_registered] -pub struct RegisteredEnum<'r, T> { +pub struct RegisteredOp<'r, T> { /// The name of the extension these ops belong to. extension_id: ExtensionId, /// A registry of all extensions, used for type computation. registry: &'r ExtensionRegistry, /// The inner [OpEnum] - op_enum: T, + op: T, } -impl RegisteredEnum<'_, T> { +impl RegisteredOp<'_, T> { /// Extract the inner wrapped value pub fn to_inner(self) -> T { - self.op_enum + self.op } } -impl RegisteredEnum<'_, T> { +impl RegisteredOp<'_, T> { /// Generate an [OpType]. - pub fn to_optype(&self) -> Option { - let leaf: LeafOp = ExtensionOp::new( + pub fn to_extension_op(&self) -> Option { + ExtensionOp::new( self.registry .get(&self.extension_id)? .get_op(&self.name())? @@ -148,33 +171,15 @@ impl RegisteredEnum<'_, T> { self.type_args(), self.registry, ) - .ok()? - .into(); - - Some(leaf.into()) - } - - /// Compute the [FunctionType] for this operation, instantiating with type arguments. - pub fn function_type(&self) -> Result { - self.op_enum.def_signature().compute_signature( - self.registry - .get(&self.extension_id) - .expect("should return 'Extension not in registry' error here.") - .get_op(&self.name()) - .expect("should return 'Op not in extension' error here."), - &self.type_args(), - self.registry, - ) + .ok() } delegate! { - to self.op_enum { + to self.op { /// Name of the operation - derived from strum serialization. pub fn name(&self) -> SmolStr; /// Any type args which define this operation. Default is no type arguments. pub fn type_args(&self) -> Vec; - /// Description of the operation. - pub fn description(&self) -> String; } } } @@ -190,12 +195,12 @@ mod test { Dumb, } - impl OpEnum for DummyEnum { - fn def_signature(&self) -> SignatureFunc { + impl MakeOpDef for DummyEnum { + fn signature(&self) -> SignatureFunc { FunctionType::new_endo(type_row![]).into() } - fn from_op_def(_op_def: &OpDef, _args: &[TypeArg]) -> Result { + fn from_def(_op_def: &OpDef) -> Result { Ok(Self::Dumb) } } @@ -207,25 +212,18 @@ mod test { let ext_name = ExtensionId::new("dummy").unwrap(); let mut e = Extension::new(ext_name.clone()); - e.add_op_enum(&o).unwrap(); - + o.add_to_extension(&mut e).unwrap(); assert_eq!( - DummyEnum::from_op_def(e.get_op(&o.name()).unwrap(), &[]).unwrap(), + DummyEnum::from_def(e.get_op(&o.name()).unwrap()).unwrap(), o ); let registry = ExtensionRegistry::try_new([e.to_owned()]).unwrap(); let registered = o.clone().to_registered(ext_name, ®istry); assert_eq!( - DummyEnum::from_optype(®istered.to_optype().unwrap()).unwrap(), + DummyEnum::from_optype(®istered.to_extension_op().unwrap().into()).unwrap(), o ); - assert_eq!( - registered.function_type().unwrap(), - FunctionType::new_endo(type_row![]) - ); - - assert_eq!(registered.description(), "Dumb"); assert_eq!(registered.to_inner(), o); } diff --git a/src/ops/custom.rs b/src/ops/custom.rs index 5f2af204f..f5c013d60 100644 --- a/src/ops/custom.rs +++ b/src/ops/custom.rs @@ -157,6 +157,13 @@ impl From for LeafOp { } } +impl From for OpType { + fn from(value: ExtensionOp) -> Self { + let leaf: LeafOp = value.into(); + leaf.into() + } +} + impl PartialEq for ExtensionOp { fn eq(&self, other: &Self) -> bool { Arc::::ptr_eq(&self.def, &other.def) && self.args == other.args diff --git a/src/std_extensions/logic.rs b/src/std_extensions/logic.rs index 1d9d78630..359a74cc5 100644 --- a/src/std_extensions/logic.rs +++ b/src/std_extensions/logic.rs @@ -5,10 +5,11 @@ use strum_macros::{EnumIter, EnumString, IntoStaticStr}; use crate::{ extension::{ prelude::BOOL_T, - simple_op::{try_from_name, OpEnum, OpLoadError}, + simple_op::{try_from_name, MakeExtensionOp, MakeOpDef, OpLoadError}, ExtensionId, OpDef, SignatureError, SignatureFromArgs, SignatureFunc, }, - ops, type_row, + ops::{self, custom::ExtensionOp, OpName}, + type_row, types::{ type_param::{TypeArg, TypeParam}, FunctionType, @@ -21,51 +22,80 @@ pub const FALSE_NAME: &str = "FALSE"; /// Name of extension true value. pub const TRUE_NAME: &str = "TRUE"; -/// Logic extension operations. -#[derive(Clone, Debug, Hash, PartialEq, Eq, EnumIter, IntoStaticStr, EnumString)] +/// Logic extension operation definitions. +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, EnumIter, IntoStaticStr, EnumString)] #[allow(missing_docs)] -pub enum LogicOp { - And(u64), - Or(u64), - Not, +pub enum NaryLogic { + And, + Or, } -impl OpEnum for LogicOp { - fn from_op_def(op_def: &OpDef, args: &[TypeArg]) -> Result { - let mut out: LogicOp = try_from_name(op_def.name())?; - match &mut out { - LogicOp::And(i) | LogicOp::Or(i) => { - let [TypeArg::BoundedNat { n }] = *args else { +impl MakeOpDef for NaryLogic { + fn signature(&self) -> SignatureFunc { + logic_op_sig().into() + } + + fn description(&self) -> String { + match self { + NaryLogic::And => "logical 'and'", + NaryLogic::Or => "logical 'or'", + } + .to_string() + } + + fn from_def(op_def: &OpDef) -> Result { + try_from_name(op_def.name()) + } +} + +/// Make a [NaryLogic] operation concrete by setting the type argument. +pub struct ConcreteLogicOp(pub NaryLogic, u64); + +impl OpName for ConcreteLogicOp { + fn name(&self) -> smol_str::SmolStr { + self.0.name() + } +} +impl MakeExtensionOp for ConcreteLogicOp { + fn from_extension_op(ext_op: &ExtensionOp) -> Result { + let def: NaryLogic = NaryLogic::from_def(ext_op.def())?; + Ok(match def { + NaryLogic::And | NaryLogic::Or => { + let [TypeArg::BoundedNat { n }] = *ext_op.args() else { return Err(SignatureError::InvalidTypeArgs.into()); }; - *i = n; + Self(def, n) } - LogicOp::Not => (), - } + }) + } - Ok(out) + fn type_args(&self) -> Vec { + vec![TypeArg::BoundedNat { n: self.1 }] } +} - fn def_signature(&self) -> SignatureFunc { - match self { - LogicOp::Or(_) | LogicOp::And(_) => logic_op_sig().into(), - LogicOp::Not => FunctionType::new_endo(type_row![BOOL_T]).into(), +/// Not operation. +#[derive(Debug, Copy, Clone)] +pub struct NotOp; +impl OpName for NotOp { + fn name(&self) -> smol_str::SmolStr { + "Not".into() + } +} +impl MakeOpDef for NotOp { + fn from_def(op_def: &OpDef) -> Result { + if op_def.name() == &NotOp.name() { + Ok(NotOp) + } else { + Err(OpLoadError::NotMember(op_def.name().to_string())) } } - fn description(&self) -> String { - match self { - LogicOp::And(_) => "logical 'and'", - LogicOp::Or(_) => "logical 'or'", - LogicOp::Not => "logical 'not'", - } - .to_string() + fn signature(&self) -> SignatureFunc { + FunctionType::new_endo(type_row![BOOL_T]).into() } - fn type_args(&self) -> Vec { - match self { - LogicOp::And(n) | LogicOp::Or(n) => vec![TypeArg::BoundedNat { n: *n }], - LogicOp::Not => vec![], - } + fn description(&self) -> String { + "logical 'not'".into() } } /// The extension identifier. @@ -97,7 +127,8 @@ fn logic_op_sig() -> impl SignatureFromArgs { /// Extension for basic logical operations. fn extension() -> Extension { let mut extension = Extension::new(EXTENSION_ID); - LogicOp::load_all_ops(&mut extension).unwrap(); + NaryLogic::load_all_ops(&mut extension).unwrap(); + NotOp.add_to_extension(&mut extension).unwrap(); extension .add_value(FALSE_NAME, ops::Const::unit_sum(0, 2)) @@ -115,14 +146,21 @@ lazy_static! { #[cfg(test)] pub(crate) mod test { - use super::{extension, LogicOp, EXTENSION, EXTENSION_ID, FALSE_NAME, TRUE_NAME}; + use super::{ + extension, ConcreteLogicOp, NaryLogic, NotOp, EXTENSION, EXTENSION_ID, FALSE_NAME, + TRUE_NAME, + }; use crate::{ - extension::{prelude::BOOL_T, simple_op::OpEnum, ExtensionRegistry}, - ops::{OpName, OpType}, - types::TypeArg, + extension::{ + prelude::BOOL_T, + simple_op::{MakeExtensionOp, MakeOpDef}, + ExtensionRegistry, + }, + ops::{custom::ExtensionOp, OpName}, Extension, }; use lazy_static::lazy_static; + use strum::IntoEnumIterator; lazy_static! { pub(crate) static ref LOGIC_REG: ExtensionRegistry = ExtensionRegistry::try_new([EXTENSION.to_owned()]).unwrap(); @@ -133,14 +171,9 @@ pub(crate) mod test { assert_eq!(r.name() as &str, "logic"); assert_eq!(r.operations().count(), 3); - for op in LogicOp::all_variants() { + for op in NaryLogic::iter() { assert_eq!( - LogicOp::from_op_def( - r.get_op(&op.name()).unwrap(), - // `all_variants` will set default type arg values. - &[TypeArg::BoundedNat { n: 0 }] - ) - .unwrap(), + NaryLogic::from_def(r.get_op(&op.name()).unwrap(),).unwrap(), op ); } @@ -159,26 +192,26 @@ pub(crate) mod test { } /// Generate a logic extension and "and" operation over [`crate::prelude::BOOL_T`] - pub(crate) fn and_op() -> OpType { - LogicOp::And(2) + pub(crate) fn and_op() -> ExtensionOp { + ConcreteLogicOp(NaryLogic::And, 2) .to_registered(EXTENSION_ID.to_owned(), &LOGIC_REG) - .to_optype() + .to_extension_op() .unwrap() } /// Generate a logic extension and "or" operation over [`crate::prelude::BOOL_T`] - pub(crate) fn or_op() -> OpType { - LogicOp::Or(2) + pub(crate) fn or_op() -> ExtensionOp { + ConcreteLogicOp(NaryLogic::Or, 2) .to_registered(EXTENSION_ID.to_owned(), &LOGIC_REG) - .to_optype() + .to_extension_op() .unwrap() } /// Generate a logic extension and "not" operation over [`crate::prelude::BOOL_T`] - pub(crate) fn not_op() -> OpType { - LogicOp::Not + pub(crate) fn not_op() -> ExtensionOp { + NotOp .to_registered(EXTENSION_ID.to_owned(), &LOGIC_REG) - .to_optype() + .to_extension_op() .unwrap() } } From 2636dc8620a4c4912bf6c17f6c0917a1d6ce3034 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Mon, 27 Nov 2023 17:25:32 +0000 Subject: [PATCH 12/14] address some review comments and fix doclinks --- src/extension/op_def.rs | 2 +- src/extension/simple_op.rs | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/extension/op_def.rs b/src/extension/op_def.rs index 5f495ad15..9f83011ac 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -216,7 +216,7 @@ impl SignatureFunc { /// /// # Panics /// - /// Panics if is [SignatureFunc::CustomFunc] and there are not enough type + /// Panics if `self` is a [SignatureFunc::CustomFunc] and there are not enough type /// arguments provided to match the number of static parameters. /// /// # Errors diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index e0fb092e1..319223f59 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -66,7 +66,8 @@ pub trait MakeOpDef: OpName { Ok(()) } - /// load all variants of a [OpEnum] in to an extension as op defs. + /// Load all variants of an enum of op definitions in to an extension as op defs. + /// See [strum::IntoEnumIterator]. fn load_all_ops(extension: &mut Extension) -> Result<(), ExtensionBuildError> where Self: IntoEnumIterator, @@ -99,7 +100,7 @@ pub trait MakeExtensionOp: OpName { fn type_args(&self) -> Vec; /// Given the ID of the extension this operation is defined in, and a - /// registry containing that extension, return a [RegisteredEnum]. + /// registry containing that extension, return a [RegisteredOp]. fn to_registered( self, extension_id: ExtensionId, @@ -132,8 +133,7 @@ impl MakeExtensionOp for T { } } -/// Load an [OpEnum] from its name. Works best for C-style enums where each -/// variant corresponds to an [OpDef] and an [OpType], i,e, there are no type parameters. +/// Load an [MakeOpDef] from its name. /// See [strum_macros::EnumString]. pub fn try_from_name(name: &str) -> Result where @@ -142,14 +142,15 @@ where T::from_str(name).map_err(|_| OpLoadError::NotMember(name.to_string())) } -/// Wrap an [OpEnum] with an extension registry to allow type computation. -/// Generate from [OpEnum::to_registered] +/// Wrap an [MakeExtensionOp] with an extension registry to allow type computation. +/// Generate from [MakeExtensionOp::to_registered] +#[derive(Clone, Debug)] pub struct RegisteredOp<'r, T> { /// The name of the extension these ops belong to. extension_id: ExtensionId, /// A registry of all extensions, used for type computation. registry: &'r ExtensionRegistry, - /// The inner [OpEnum] + /// The inner [MakeExtensionOp] op: T, } From ec0aa5bae658ea37ffeb8d04c0340562615ef1ed Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Wed, 29 Nov 2023 17:42:32 +0000 Subject: [PATCH 13/14] docstring improvements --- src/extension/simple_op.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/extension/simple_op.rs b/src/extension/simple_op.rs index 319223f59..bd9f280dd 100644 --- a/src/extension/simple_op.rs +++ b/src/extension/simple_op.rs @@ -38,7 +38,9 @@ where } /// Traits implemented by types which can add themselves to [`Extension`]s as -/// [`OpDef`]s or load themselves from an [`OpDef`] +/// [`OpDef`]s or load themselves from an [`OpDef`]. +/// Particularly useful with C-style enums that implement [strum::IntoEnumIterator], +/// as then all definitions can be added to an extension at once. pub trait MakeOpDef: OpName { /// Try to load one of the operations of this set from an [OpDef]. fn from_def(op_def: &OpDef) -> Result From 820fec2e83f3925c213f80745a4d52ac2d19baff Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Wed, 29 Nov 2023 17:43:58 +0000 Subject: [PATCH 14/14] fix issue link --- 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 9f83011ac..5ce8b483f 100644 --- a/src/extension/op_def.rs +++ b/src/extension/op_def.rs @@ -247,8 +247,8 @@ impl SignatureFunc { let res = pf.instantiate(args, exts)?; // TODO bring this assert back once resource inference is done? - // https://github.com/CQCL-DEV/hugr/issues/425 - // assert!(res.contains(self.extension())); + // https://github.com/CQCL/hugr/issues/388 + // debug_assert!(res.extension_reqs.contains(def.extension())); Ok(res) } }