Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: use JSON rather than YAML in opaque fields. #1338

Merged
merged 2 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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`.
ss2165 marked this conversation as resolved.
Show resolved Hide resolved
///
/// 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};
ss2165 marked this conversation as resolved.
Show resolved Hide resolved
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 }))),
ss2165 marked this conversation as resolved.
Show resolved Hide resolved
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
Loading