Skip to content

Commit

Permalink
refactor!: use JSON rather than YAML in opaque fields.
Browse files Browse the repository at this point in the history
Closes #1048

BREAKING_CHANGE: use `serde_json::Value` rather than `serde_yaml::Value` in `CustomSerialized`, `CustomTypeArg`, `OpDef`, `OperationDeclaration`
  • Loading branch information
ss2165 committed Jul 24, 2024
1 parent 32fb093 commit 74d4531
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 58 deletions.
2 changes: 1 addition & 1 deletion hugr-core/src/extension/declarative/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub(super) struct OperationDeclaration {
/// This data is kept in the Hugr, and may be accessed by the relevant runtime.
#[serde(default)]
#[serde(skip_serializing_if = "crate::utils::is_default")]
misc: HashMap<String, serde_yaml::Value>,
misc: HashMap<String, serde_json::Value>,
/// A pre-compiled lowering routine.
///
/// This is not yet supported, and will raise an error if present.
Expand Down
12 changes: 6 additions & 6 deletions hugr-core/src/extension/op_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub trait CustomLowerFunc: Send + Sync {
&self,
name: &OpNameRef,
arg_values: &[TypeArg],
misc: &HashMap<String, serde_yaml::Value>,
misc: &HashMap<String, serde_json::Value>,
available_extensions: &ExtensionSet,
) -> Option<Hugr>;
}
Expand Down Expand Up @@ -319,7 +319,7 @@ pub struct OpDef {
description: String,
/// Miscellaneous data associated with the operation.
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
misc: HashMap<String, serde_yaml::Value>,
misc: HashMap<String, serde_json::Value>,

#[serde(flatten)]
signature_func: SignatureFunc,
Expand Down Expand Up @@ -434,8 +434,8 @@ impl OpDef {
pub fn add_misc(
&mut self,
k: impl ToString,
v: serde_yaml::Value,
) -> Option<serde_yaml::Value> {
v: serde_json::Value,
) -> Option<serde_json::Value> {
self.misc.insert(k.to_string(), v)
}

Expand Down Expand Up @@ -825,9 +825,9 @@ pub(super) mod test {
type Parameters = ();
type Strategy = BoxedStrategy<Self>;
fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
use crate::proptest::{any_serde_yaml_value, any_smolstr, any_string};
use crate::proptest::{any_serde_json_value, any_smolstr, any_string};
use proptest::collection::{hash_map, vec};
let misc = hash_map(any_string(), any_serde_yaml_value(), 0..3);
let misc = hash_map(any_string(), any_serde_json_value(), 0..3);
(
any::<ExtensionId>(),
any_smolstr(),
Expand Down
20 changes: 9 additions & 11 deletions hugr-core/src/ops/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,12 @@ pub enum Value {
/// serialization and deserialization of unknown impls of [CustomConst].
///
/// During serialization we first serialize the internal [`dyn` CustomConst](CustomConst)
/// into a [serde_yaml::Value]. We then create a [CustomSerialized] wrapping
/// into a [serde_json::Value]. We then create a [CustomSerialized] wrapping
/// that value. That [CustomSerialized] is then serialized in place of the
/// [OpaqueValue].
///
/// During deserialization, first we deserialize a [CustomSerialized]. We
/// attempt to deserialize the internal [serde_yaml::Value] using the [`Box<dyn
/// attempt to deserialize the internal [serde_json::Value] using the [`Box<dyn
/// CustomConst>`](CustomConst) impl. This will fail if the appropriate `impl CustomConst`
/// is not linked into the running program, in which case we coerce the
/// [CustomSerialized] into a [`Box<dyn CustomConst>`](CustomConst). The [OpaqueValue] is
Expand Down Expand Up @@ -235,7 +235,7 @@ pub enum Value {
/// assert_eq!(&serde_json::to_value(&ev).unwrap(), &expected_json);
/// assert_eq!(ev, serde_json::from_value(expected_json).unwrap());
///
/// let ev = OpaqueValue::new(CustomSerialized::new(USIZE_T.clone(), serde_yaml::Value::Null, ExtensionSet::default()));
/// let ev = OpaqueValue::new(CustomSerialized::new(USIZE_T.clone(), serde_json::Value::Null, ExtensionSet::default()));
/// let expected_json = json!({
/// "extensions": [],
/// "typ": USIZE_T,
Expand Down Expand Up @@ -544,7 +544,6 @@ mod test {
};
use cool_asserts::assert_matches;
use rstest::{fixture, rstest};
use serde_yaml::Value as YamlValue;

use super::*;

Expand Down Expand Up @@ -682,7 +681,7 @@ mod test {
#[rstest]
#[case(Value::unit(), Type::UNIT, "const:seq:{}")]
#[case(const_usize(), USIZE_T, "const:custom:ConstUsize(")]
#[case(serialized_float(17.4), FLOAT64_TYPE, "const:custom:yaml:Mapping")]
#[case(serialized_float(17.4), FLOAT64_TYPE, "const:custom:json:Object")]
#[case(const_tuple(), Type::new_tuple(type_row![USIZE_T, FLOAT64_TYPE]), "const:seq:{")]
fn const_type(
#[case] const_value: Value,
Expand All @@ -709,24 +708,23 @@ mod test {
}

#[test]
fn test_yaml_const() {
fn test_json_const() {
let ex_id: ExtensionId = "my_extension".try_into().unwrap();
let typ_int = CustomType::new(
"my_type",
vec![TypeArg::BoundedNat { n: 8 }],
ex_id.clone(),
TypeBound::Eq,
);
let yaml_const: Value =
CustomSerialized::new(typ_int.clone(), YamlValue::Number(6.into()), ex_id.clone())
.into();
let json_const: Value =
CustomSerialized::new(typ_int.clone(), 6.into(), ex_id.clone()).into();
let classic_t = Type::new_extension(typ_int.clone());
assert_matches!(classic_t.least_upper_bound(), TypeBound::Eq);
assert_eq!(yaml_const.get_type(), classic_t);
assert_eq!(json_const.get_type(), classic_t);

let typ_qb = CustomType::new("my_type", vec![], ex_id, TypeBound::Eq);
let t = Type::new_extension(typ_qb.clone());
assert_ne!(yaml_const.get_type(), t);
assert_ne!(json_const.get_type(), t);
}

mod proptest {
Expand Down
64 changes: 32 additions & 32 deletions hugr-core/src/ops/constant/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,18 @@ pub fn downcast_equal_consts<T: CustomConst + PartialEq>(
}

/// Serialize any CustomConst using the `impl Serialize for &dyn CustomConst`.
fn serialize_custom_const(cc: &dyn CustomConst) -> Result<serde_yaml::Value, serde_yaml::Error> {
serde_yaml::to_value(cc)
fn serialize_custom_const(cc: &dyn CustomConst) -> Result<serde_json::Value, serde_json::Error> {
serde_json::to_value(cc)
}

/// Deserialize a `Box<&dyn CustomConst>` and attempt to downcast it to `CC`;
/// propagating failure.
fn deserialize_custom_const<CC: CustomConst>(
value: serde_yaml::Value,
) -> Result<CC, serde_yaml::Error> {
value: serde_json::Value,
) -> Result<CC, serde_json::Error> {
match deserialize_dyn_custom_const(value)?.downcast::<CC>() {
Ok(cc) => Ok(*cc),
Err(dyn_cc) => Err(<serde_yaml::Error as serde::de::Error>::custom(format!(
Err(dyn_cc) => Err(<serde_json::Error as serde::de::Error>::custom(format!(
"Failed to deserialize [{}]: {:?}",
std::any::type_name::<CC>(),
dyn_cc
Expand All @@ -124,9 +124,9 @@ fn deserialize_custom_const<CC: CustomConst>(

/// Deserialize a `Box<&dyn CustomConst>`.
fn deserialize_dyn_custom_const(
value: serde_yaml::Value,
) -> Result<Box<dyn CustomConst>, serde_yaml::Error> {
serde_yaml::from_value(value)
value: serde_json::Value,
) -> Result<Box<dyn CustomConst>, serde_json::Error> {
serde_json::from_value(value)
}

impl_downcast!(CustomConst);
Expand All @@ -136,31 +136,31 @@ impl_box_clone!(CustomConst, CustomConstBoxClone);
/// A constant value stored as a serialized blob that can report its own type.
pub struct CustomSerialized {
typ: Type,
value: serde_yaml::Value,
value: serde_json::Value,
extensions: ExtensionSet,
}

#[derive(Debug, Error)]
#[error("Error serializing value into CustomSerialized: err: {err}, value: {payload:?}")]
pub struct SerializeError {
#[source]
err: serde_yaml::Error,
err: serde_json::Error,
payload: Box<dyn CustomConst>,
}

#[derive(Debug, Error)]
#[error("Error deserializing value from CustomSerialized: err: {err}, value: {payload:?}")]
pub struct DeserializeError {
#[source]
err: serde_yaml::Error,
payload: serde_yaml::Value,
err: serde_json::Error,
payload: serde_json::Value,
}

impl CustomSerialized {
/// Creates a new [`CustomSerialized`].
pub fn new(
typ: impl Into<Type>,
value: serde_yaml::Value,
value: serde_json::Value,
exts: impl Into<ExtensionSet>,
) -> Self {
Self {
Expand All @@ -171,7 +171,7 @@ impl CustomSerialized {
}

/// Returns the inner value.
pub fn value(&self) -> &serde_yaml::Value {
pub fn value(&self) -> &serde_json::Value {
&self.value
}

Expand Down Expand Up @@ -238,7 +238,7 @@ impl CustomSerialized {
/// deserialize it. In particular if that inner value were a [Self] whose
/// inner value were a `CC`, then we would still fail.
pub fn try_into_custom_const<CC: CustomConst>(self) -> Result<CC, DeserializeError> {
// ideally we would not have to clone, but serde_yaml does not allow us
// ideally we would not have to clone, but serde_json does not allow us
// to recover the value from the error
deserialize_custom_const(self.value.clone()).map_err(|err| DeserializeError {
err,
Expand All @@ -250,7 +250,7 @@ impl CustomSerialized {
#[typetag::serde]
impl CustomConst for CustomSerialized {
fn name(&self) -> ValueName {
format!("yaml:{:?}", self.value).into()
format!("json:{:?}", self.value).into()
}

fn equal_consts(&self, other: &dyn CustomConst) -> bool {
Expand Down Expand Up @@ -330,13 +330,13 @@ mod test {
struct SerializeCustomConstExample<CC: CustomConst + serde::Serialize + 'static> {
cc: CC,
tag: &'static str,
yaml: serde_yaml::Value,
json: serde_json::Value,
}

impl<CC: CustomConst + serde::Serialize + 'static> SerializeCustomConstExample<CC> {
fn new(cc: CC, tag: &'static str) -> Self {
let yaml = serde_yaml::to_value(&cc).unwrap();
Self { cc, tag, yaml }
let json = serde_json::to_value(&cc).unwrap();
Self { cc, tag, json }
}
}

Expand All @@ -363,21 +363,21 @@ mod test {
>(
#[case] example: SerializeCustomConstExample<CC>,
) {
assert_eq!(example.yaml, serde_yaml::to_value(&example.cc).unwrap()); // sanity check
let expected_yaml: serde_yaml::Value = [
assert_eq!(example.json, serde_json::to_value(&example.cc).unwrap()); // sanity check
let expected_json: serde_json::Value = [
("c".into(), example.tag.into()),
("v".into(), example.yaml.clone()),
("v".into(), example.json.clone()),
]
.into_iter()
.collect::<serde_yaml::Mapping>()
.collect::<serde_json::Map<String, serde_json::Value>>()
.into();

// check serialize_custom_const
assert_eq!(expected_yaml, serialize_custom_const(&example.cc).unwrap());
assert_eq!(expected_json, serialize_custom_const(&example.cc).unwrap());

let expected_custom_serialized = CustomSerialized::new(
example.cc.get_type(),
expected_yaml,
expected_json,
example.cc.extension_reqs(),
);

Expand Down Expand Up @@ -412,12 +412,12 @@ mod test {

// check OpaqueValue serializes/deserializes as a CustomSerialized
let ev: OpaqueValue = example.cc.clone().into();
let ev_val = serde_yaml::to_value(&ev).unwrap();
let ev_val = serde_json::to_value(&ev).unwrap();
assert_eq!(
&ev_val,
&serde_yaml::to_value(&expected_custom_serialized).unwrap()
&serde_json::to_value(&expected_custom_serialized).unwrap()
);
assert_eq!(ev, serde_yaml::from_value(ev_val).unwrap());
assert_eq!(ev, serde_json::from_value(ev_val).unwrap());
}

fn example_custom_serialized() -> (ConstUsize, CustomSerialized) {
Expand Down Expand Up @@ -473,7 +473,7 @@ mod test {
// A serialization round-trip results in an OpaqueValue with the value of inner
assert_eq!(
OpaqueValue::new(inner),
serde_yaml::from_value(serde_yaml::to_value(&ev).unwrap()).unwrap()
serde_json::from_value(serde_json::to_value(&ev).unwrap()).unwrap()
);
}
}
Expand All @@ -485,7 +485,7 @@ mod proptest {
use crate::{
extension::ExtensionSet,
ops::constant::CustomSerialized,
proptest::{any_serde_yaml_value, any_string},
proptest::{any_serde_json_value, any_string},
types::Type,
};

Expand All @@ -502,10 +502,10 @@ mod proptest {
// generate a valid tag(e.g. "ConstInt") then things will
// go wrong: the serde::Deserialize impl for that type will
// interpret "v" and fail.
let value = (any_serde_yaml_value(), any_string()).prop_map(|(content, tag)| {
let value = (any_serde_json_value(), any_string()).prop_map(|(content, tag)| {
[("c".into(), tag.into()), ("v".into(), content)]
.into_iter()
.collect::<serde_yaml::Mapping>()
.collect::<serde_json::Map<String, _>>()
.into()
});
(typ, value, extensions)
Expand Down
18 changes: 10 additions & 8 deletions hugr-core/src/proptest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ lazy_static! {
].sboxed()
};

/// A strategy for an arbitrary non-recursive [serde_yaml::Value].
/// A strategy for an arbitrary non-recursive [serde_json::Value].
/// In particular, no `Mapping`, `Sequence`, or `Tagged`.
///
/// This is used as the base strategy for the general
/// [recursive](Strategy::prop_recursive) strategy.
static ref ANY_SERDE_YAML_VALUE_LEAF: SBoxedStrategy<serde_yaml::Value> = {
use serde_yaml::value::Value;
static ref ANY_SERDE_JSON_VALUE_LEAF: SBoxedStrategy<serde_json::Value> = {
use serde_json::value::Value;
prop_oneof![
Just(Value::Null),
any::<bool>().prop_map_into(),
Expand Down Expand Up @@ -155,9 +155,9 @@ pub fn any_smolstr() -> SBoxedStrategy<SmolStr> {
ANY_STRING.clone().prop_map_into().sboxed()
}

pub fn any_serde_yaml_value() -> impl Strategy<Value = serde_yaml::Value> {
// use serde_yaml::value::{Tag, TaggedValue, Value};
ANY_SERDE_YAML_VALUE_LEAF
pub fn any_serde_json_value() -> impl Strategy<Value = serde_json::Value> {
// use serde_json::value::{Tag, TaggedValue, Value};
ANY_SERDE_JSON_VALUE_LEAF
.clone()
.prop_recursive(
3, // No more than 3 branch levels deep
Expand All @@ -168,8 +168,10 @@ pub fn any_serde_yaml_value() -> impl Strategy<Value = serde_yaml::Value> {
// TODO TaggedValue doesn't roundtrip through JSON
// (any_nonempty_string().prop_map(Tag::new), element.clone()).prop_map(|(tag, value)| Value::Tagged(Box::new(TaggedValue { tag, value }))),
proptest::collection::vec(element.clone(), 0..3).prop_map_into(),
vec((any_string().prop_map_into(), element.clone()), 0..3)
.prop_map(|x| x.into_iter().collect::<serde_yaml::Mapping>().into())
vec((any_string().prop_map_into(), element.clone()), 0..3).prop_map(|x| x
.into_iter()
.collect::<serde_json::Map<String, serde_json::Value>>()
.into())
]
},
)
Expand Down

0 comments on commit 74d4531

Please sign in to comment.