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

Tagged enum support / JSON interoperability #272

Closed
antonok-edm opened this issue Jul 20, 2019 · 7 comments
Closed

Tagged enum support / JSON interoperability #272

antonok-edm opened this issue Jul 20, 2019 · 7 comments

Comments

@antonok-edm
Copy link

I'm currently using serde_json for simple, debuggable manipulation in a web-based frontend, and I'm hoping to reuse the same datatypes for IPC in the context of a higher-performance application connected via a websocket. I'd like to use bincode for that purpose, but I'm running into issues getting the two to play nicely together regarding tagged enums.

For context, here's a simplified example:

use serde::{Deserialize, Serialize};

#[derive(Deserialize, Serialize)]
#[serde(tag = "type")]       // <---
enum Container {
    NoData,
    SomeData(Data),
    NegativeData(Data),
}

#[derive(Deserialize, Serialize)]
#[serde(tag = "variant")]    // <---
enum Data {
    A,
    B,
}

fn main() {
    let container = Container::SomeData(Data::A);

    let v = serde_json::to_string(&container).expect("1");
    println!("{}", v);
    let _: Container = serde_json::from_str(&v).expect("2");

    let encoded: Vec<u8> = bincode::serialize(&container).expect("3");
    let _: Container = bincode::deserialize(&encoded[..]).expect("4");
} 

Using the highlighted serde(tag = ...) attributes above, as suggested by the documentation here, results in easily usable, descriptive JSON like {"type":"SomeData","variant":"A"}. Without the tag attributes, I instead get {"SomeData":"A"}, which is pretty annoying to classify in the Javascript side of things.

bincode, on the other hand, can only deserialize the version without tag attributes. When they're added like in the example above, I get a pretty uninformative DeserializeAnyNotSupported error at expect number 4.

I assume this is unintended behavior? Even if there's a technical reason the deserialization can't work this way, the serialization should probably be prevented in the first place, and a better error message would help.

@jdm
Copy link
Collaborator

jdm commented Jul 21, 2019

I found the error is coming from this code in the deserialize_any implementation. The documentation for the method in Serde does not inspire confidence in the ability to support this in bincode: https://docs.rs/serde/1.0.97/serde/trait.Deserializer.html#tymethod.deserialize_any

@NyxCode
Copy link

NyxCode commented Dec 7, 2019

Is there any update on this? Here is another minimal example which breaks:

#[test]
fn test() {
    use serde::{Serialize, Deserialize};

    #[derive(Serialize, Deserialize, Debug, PartialEq)]
    #[serde(tag = "type", content = "data")]
    enum MyEnum {
        VariantA,
    }

    let serialized = bincode::serialize(&MyEnum::VariantA).unwrap();
    let deserialized = bincode::deserialize::<MyEnum>(&serialized).unwrap();
    assert_eq!(deserialized, MyEnum::VariantA);
}
called `Result::unwrap()` on an `Err` value: Custom("Bincode does not support Deserializer::deserialize_identifier")
thread 'test' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("Bincode does not support Deserializer::deserialize_identifier")', src\libcore\result.rs:1165:5

@da-x
Copy link

da-x commented Jan 26, 2020

@NyxCode - that is an adjacently tagged enum, in the serde parlance.

I have also bumped into this and devised a partial workaround, with the test above extended. Remarks:

  • For bincode's case, we probably don't want the 'tag' string and the 'content' string to be serialized. However, this is what serde seems to instruct bincode to do, because the enum is being regarded as a map. So, the workaround implements a deserialize_identifer that maps to deserialize_str. which mirrors what the bincode serializer does right now. Unfortunately, the strings appear in the encoded data. But at least deserialization works. Probably an architectural change would be needed in serde for this. @dtolnay @erickt ?
  • Looking at the output of serde_derive for this kind of enum, it appeared to me that deserialize_any is called over the enum variant's unit structs and the named-fields enum variant. This prompted me to call deserialize_unit instead of returning an error there. Now tuple variants and unit variants work, but named-fields variants don't :(.

@ZoeyR
Copy link
Collaborator

ZoeyR commented Mar 24, 2020

Unfortunately bincode cannot support deserialize_any since it is not a self-describing format. We have no way of determining how to drive the visitor. We can map deserialize_identifier to deserialize_str, but any further changes would have to be done in serde itself.

@simias
Copy link

simias commented Feb 4, 2021

I know that this is effectively more of a serde issue than a bincode one, but I admit that it's my main gripe with the framework as a whole at the moment. The default externally tagged enums are annoying to work with in JSON, and if you want to use bincode as well then you have no choice.

If only I could opt in the feature only for JSON, that'd effectively solve the issue for me. But as far as I can tell it's not really possible?

@ZoeyR
Copy link
Collaborator

ZoeyR commented Feb 9, 2021

@simias yeah, that's not really possible and is an issue with the serde framework itself.

@stale
Copy link

stale bot commented Jun 13, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants