Skip to content
This repository was archived by the owner on Aug 15, 2021. It is now read-only.

feat: implement simple tagging#129

Closed
dignifiedquire wants to merge 2 commits intopyfisch:masterfrom
dignifiedquire:tags
Closed

feat: implement simple tagging#129
dignifiedquire wants to merge 2 commits intopyfisch:masterfrom
dignifiedquire:tags

Conversation

@dignifiedquire
Copy link

@dignifiedquire dignifiedquire commented Jun 19, 2019

Mostly putting this here for visibility, I can very much understand if this never gets merged but I really need a solution to tags.

  • for now behind a feature flags tags
  • it will generate artifacts when used with other serializers
  • conceptually implemented in the same way as rust-cbor does it
  • will be a breaking change as
    • tags now get emitted as u64 values
    • the self describing tag gets emitted as well, but this could be changed
  • I have not added Tagged to the Value type yet, unclear if this needs to happen or not

Example Code

#[derive(Debug, PartialEq)]
struct MyTaggedValue(Vec<u8>);

impl MyTaggedValue {
    fn tag() -> u64 {
        9
    }
}

impl Serialize for MyTaggedValue {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        serde_cbor::EncodeCborTag::new(Self::tag(), &self.0).serialize(serializer)
    }
}

impl<'de> Deserialize<'de> for MyTaggedValue {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        let wrapper = serde_cbor::EncodeCborTag::deserialize(deserializer)?;
        if wrapper.tag() != Self::tag() {
            return Err(serde::de::Error::custom(format!(
                "Invalid tag: {}, expected {}",
                wrapper.tag(),
                Self::tag()
            )));
        }
        Ok(MyTaggedValue(wrapper.value()))
    }
}

// Roundtrip with custom type

let value = MyTaggedValue(vec![1, 2, 3]);

// C9       # tag(9)
//    83    # array(3)
//       01 # unsigned(1)
//       02 # unsigned(2)
//       03 # unsigned(3)

let bytes = b"\xC9\x83\x01\x02\x03";
assert_eq!(&to_vec(&value).unwrap()[..], bytes);
let res: MyTaggedValue = serde_cbor::de::from_slice_with_scratch(bytes, &mut []).unwrap();

assert_eq!(res, value);

Ref #56

@pyfisch
Copy link
Owner

pyfisch commented Sep 2, 2019

Mostly putting this here for visibility, I can very much understand if this never gets merged but I really need a solution to tags.

I really hope this helps anyone who needs tags. I will think about adopting a similar version in the future.

@vmx
Copy link

vmx commented Sep 10, 2019

@pyfisch This looks like a solid basis for tags. What would be needed to get this over the finish line in order to get tag support merged?

@vmx
Copy link

vmx commented Sep 10, 2019

The force-push was just a rebase on top of current master.

@pyfisch
Copy link
Owner

pyfisch commented Sep 10, 2019

@pyfisch This looks like a solid basis for tags. What would be needed to get this over the finish line in order to get tag support merged?

The virtue of serde is that if you annotate a type with #[derive(Serialize, Deserialize)] it can be stored in many different data formats without the type being aware of the data format and vice versa.
Tags already break this concept somewhat because you need to add them to types and they vary based on the data format. This would be still ok as users can opt-in into using them.
This change works around the serde data model to express tags and in the process breaks other data formats using types which include CBOR tags or prevents the deserialization of tagged values if the value does not expect a tag.

To get tag support merged the serde data model needs to be changed (or as I am told specialization needs to land in rust #31844). Then support for CBOR tags can be implemented.

The force-push was just a rebase on top of current master.

How does this work with a branch of another user?

@crossi704
Copy link

A proposal:
dignifiedquire#1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants