From ec1ea24daf69baa6410a88bb0284bb3bf31695fa Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 2 Aug 2023 11:41:24 +0100 Subject: [PATCH 1/6] CustomCheckFail is now an enum with Message or TypeMismatch. (Remove new.) --- src/extensions/rotation.rs | 2 +- src/values.rs | 19 +++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/extensions/rotation.rs b/src/extensions/rotation.rs index 8d18e2840..520d38117 100644 --- a/src/extensions/rotation.rs +++ b/src/extensions/rotation.rs @@ -119,7 +119,7 @@ impl CustomConst for Constant { if &self_typ.custom_type() == typ { Ok(()) } else { - Err(CustomCheckFail::new( + Err(CustomCheckFail::Message( "Rotation constant type mismatch.".into(), )) } diff --git a/src/values.rs b/src/values.rs index 6f0c28565..db69a2a4c 100644 --- a/src/values.rs +++ b/src/values.rs @@ -5,7 +5,7 @@ use thiserror::Error; -use crate::types::{ClassicType, Container, HashableType, PrimType}; +use crate::types::{ClassicType, Container, CustomType, HashableType, PrimType}; use crate::{ ops::constant::{ typecheck::{check_int_fits_in_width, ConstIntError}, @@ -213,14 +213,13 @@ pub(crate) fn map_container_type( /// Struct for custom type check fails. #[derive(Clone, Debug, PartialEq, Error)] -#[error("Error when checking custom type.")] -pub struct CustomCheckFail(String); - -impl CustomCheckFail { - /// Creates a new [`CustomCheckFail`]. - pub fn new(message: String) -> Self { - Self(message) - } +pub enum CustomCheckFail { + /// The value had a specific type that was not what was expected + #[error("Expected type: {0} but value was of type: {1}")] + TypeMismatch(CustomType, CustomType), + /// Any other message + #[error("{0}")] + Message(String), } /// Errors that arise from typechecking constants @@ -246,6 +245,6 @@ pub enum ConstTypeError { #[error("Value {1:?} does not match expected type {0}")] ValueCheckFail(ClassicType, ConstValue), /// Error when checking a custom value. - #[error("Custom value type check error: {0:?}")] + #[error("Error when checking custom type: {0:?}")] CustomCheckFail(#[from] CustomCheckFail), } From b21418608872300a14e19aeb4ee4caed59d783ab Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 2 Aug 2023 11:56:17 +0100 Subject: [PATCH 2/6] Add struct CustomSerialized with impl CustomConst --- src/ops/constant.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/ops/constant.rs b/src/ops/constant.rs index b051ac7d2..02947edf9 100644 --- a/src/ops/constant.rs +++ b/src/ops/constant.rs @@ -305,6 +305,27 @@ pub trait CustomConst: impl_downcast!(CustomConst); impl_box_clone!(CustomConst, CustomConstBoxClone); +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +struct CustomSerialized { + typ: CustomType, + value: serde_yaml::Value, +} + +#[typetag::serde] +impl CustomConst for CustomSerialized { + fn name(&self) -> SmolStr { + format!("yaml:{:?}", self.value).into() + } + + fn check_custom_type(&self, typ: &CustomType) -> Result<(), CustomCheckFail> { + if &self.typ == typ { + Ok(()) + } else { + Err(CustomCheckFail::TypeMismatch(typ.clone(), self.typ.clone())) + } + } +} + #[cfg(test)] mod test { use cool_asserts::assert_matches; From 60d60782bde4a8b0393398a0cd68351c9401aaf8 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 2 Aug 2023 11:56:57 +0100 Subject: [PATCH 3/6] Remove ContainerValue::Opaque --- src/ops/constant.rs | 2 -- src/values.rs | 12 +++--------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/ops/constant.rs b/src/ops/constant.rs index 02947edf9..414dd66fd 100644 --- a/src/ops/constant.rs +++ b/src/ops/constant.rs @@ -176,8 +176,6 @@ impl ValueOfType for ConstValue { // A "hashable" value might be an instance of a non-hashable type: // e.g. an empty list is hashable, yet can be checked against a classic element type! if let HashableValue::Container(ctr) = hv { - // Note if ctr is a ContainerValue::Opaque, this means we can check that - // against a Container::Opaque, which is perhaps unnecessary, but harmless. return ctr.map_vals(&ConstValue::Hashable).check_container(cty); } } diff --git a/src/values.rs b/src/values.rs index db69a2a4c..5db2e17df 100644 --- a/src/values.rs +++ b/src/values.rs @@ -94,7 +94,9 @@ impl ValueOfType for HashableValue { /// A value that is a container of other values, e.g. a tuple or sum; /// thus, corresponding to [Container]. Note there is no member /// corresponding to [Container::Alias]; such types must have been -/// resolved to concrete types in order to create instances (values). +/// resolved to concrete types in order to create instances (values), +/// nor to [Container::Opaque], which is left to classes for broader +/// sets of values (see e.g. [ConstValue::Opaque]) #[derive(Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize)] pub enum ContainerValue { /// A [Container::Array] or [Container::Tuple] or [Container::List] @@ -104,11 +106,6 @@ pub enum ContainerValue { /// A [Container::Sum] - for any Sum type where this value meets /// the type of the variant indicated by the tag Sum(usize, Box), // Tag and value - /// A value of a custom type defined by an extension/[Resource]. - /// - /// [Resource]: crate::resource::Resource - // TODO replace this with CustomConst - Opaque(serde_yaml::Value), } impl ContainerValue { @@ -120,7 +117,6 @@ impl ContainerValue { } ContainerValue::Map(_) => "a map".to_string(), ContainerValue::Sum(tag, val) => format!("const:sum:{{tag:{tag}, val:{}}}", val.name()), - ContainerValue::Opaque(c) => format!("const:yaml:{:?}", c), } } pub(crate) fn check_container(&self, ty: &Container) -> Result<(), ConstTypeError> { @@ -160,7 +156,6 @@ impl ContainerValue { (ContainerValue::Sum(tag, value), Container::Sum(variants)) => { value.check_type(variants.get(*tag).ok_or(ConstTypeError::InvalidSumTag)?) } - (ContainerValue::Opaque(_), Container::Opaque(_)) => Ok(()), // TODO (_, Container::Alias(s)) => Err(ConstTypeError::NoAliases(s.to_string())), (_, _) => Err(ValueOfType::container_error(ty.clone(), self.clone())), } @@ -175,7 +170,6 @@ impl ContainerValue { ContainerValue::Sum(tag, value) => { ContainerValue::Sum(*tag, Box::new(f((**value).clone()))) } - ContainerValue::Opaque(v) => ContainerValue::Opaque(v.clone()), } } } From de2295334bed8ee63c46b49584b52388e86be571 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 2 Aug 2023 12:01:13 +0100 Subject: [PATCH 4/6] Comment ConstValue::Opaque: it stays there, so not shared with TypeArg --- src/ops/constant.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ops/constant.rs b/src/ops/constant.rs index 414dd66fd..2ad02d938 100644 --- a/src/ops/constant.rs +++ b/src/ops/constant.rs @@ -139,7 +139,10 @@ pub enum ConstValue { Container(ContainerValue), /// Double precision float F64(f64), - /// An opaque constant value, with cached type. TODO put this into ContainerValue. + /// An opaque constant value, that can check it is of a given [CustomType]. + /// This may include values that are [hashable] + /// + /// [hashable]: crate::types::simple::TypeTag::Hashable // Note: the extra level of tupling is to avoid https://github.com/rust-lang/rust/issues/78808 Opaque((Box,)), } From 0f222e35b3bc682d7faf471c637b1819f61df75b Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 2 Aug 2023 13:00:24 +0100 Subject: [PATCH 5/6] Fix clippy that suddenly starts showing up?! --- src/ops/constant.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ops/constant.rs b/src/ops/constant.rs index 2ad02d938..3fa12b370 100644 --- a/src/ops/constant.rs +++ b/src/ops/constant.rs @@ -400,8 +400,7 @@ mod test { tuple_val2.check_type(&tuple_ty), Err(ConstTypeError::ValueCheckFail(ty, tv2)) => ty == tuple_ty && tv2 == tuple_val2 ); - let tuple_val3 = - ConstValue::sequence(&vec![V_INT, ConstValue::F64(3.3), ConstValue::F64(2.0)]); + let tuple_val3 = ConstValue::sequence(&[V_INT, ConstValue::F64(3.3), ConstValue::F64(2.0)]); assert_eq!( tuple_val3.check_type(&tuple_ty), Err(ConstTypeError::TupleWrongLength) From de423033e061cd5f4b55014c56bc961f94df123d Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 2 Aug 2023 15:13:41 +0100 Subject: [PATCH 6/6] Test --- src/ops/constant.rs | 47 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/src/ops/constant.rs b/src/ops/constant.rs index 3fa12b370..cf0b367c9 100644 --- a/src/ops/constant.rs +++ b/src/ops/constant.rs @@ -330,17 +330,21 @@ impl CustomConst for CustomSerialized { #[cfg(test)] mod test { use cool_asserts::assert_matches; + use serde_yaml::Value; - use super::{typecheck::ConstIntError, Const, ConstValue}; + use super::{typecheck::ConstIntError, Const, ConstValue, CustomSerialized}; use crate::{ - builder::{BuildError, Container, DFGBuilder, Dataflow, DataflowHugr}, + builder::{BuildError, DFGBuilder, Dataflow, DataflowHugr}, classic_row, type_row, - types::{ClassicType, SimpleRow, SimpleType}, - values::{ConstTypeError, HashableValue, ValueOfType}, + types::simple::Container, + types::type_param::TypeArg, + types::{ClassicType, CustomType, HashableType, SimpleRow, SimpleType, TypeTag}, + values::{ConstTypeError, CustomCheckFail, HashableValue, ValueOfType}, }; #[test] fn test_predicate() -> Result<(), BuildError> { + use crate::builder::Container; let pred_rows = vec![ classic_row![ClassicType::i64(), ClassicType::F64], type_row![], @@ -406,4 +410,39 @@ mod test { Err(ConstTypeError::TupleWrongLength) ); } + + #[test] + fn test_yaml_const() { + let typ_int = CustomType::new( + "mytype", + vec![TypeArg::ClassicType(ClassicType::Hashable( + HashableType::Int(8), + ))], + "myrsrc", + TypeTag::Hashable, + ); + let val = ConstValue::Opaque((Box::new(CustomSerialized { + typ: typ_int.clone(), + value: Value::Number(6.into()), + }),)); + let SimpleType::Classic(classic_t) = typ_int.clone().into() + else {panic!("Hashable CustomType returned as non-Classic");}; + assert_matches!(classic_t, ClassicType::Hashable(_)); + val.check_type(&classic_t).unwrap(); + + // This misrepresents the CustomType, so doesn't really "have to work". + // But just as documentation of current behaviour: + val.check_type(&ClassicType::Container(Container::Opaque(typ_int.clone()))) + .unwrap(); + + let typ_float = CustomType::new( + "mytype", + vec![TypeArg::ClassicType(ClassicType::F64)], + "myrsrc", + TypeTag::Hashable, + ); + let t: SimpleType = typ_float.clone().into(); + assert_matches!(val.check_type(&t.try_into().unwrap()), + Err(ConstTypeError::CustomCheckFail(CustomCheckFail::TypeMismatch(a, b))) => a == typ_int && b == typ_float); + } }