serde-rs / serde

Serialization framework for Rust
https://serde.rs/
Apache License 2.0
9.16k stars 775 forks source link

Question about serializing my own struct #177

Closed rigdent closed 8 years ago

rigdent commented 9 years ago

Hello,

First thanks for the great crate! I was using rustc_serialize before, but recently found about this create which is supposed to be faster and better supported.

I do have a few questions about serializing my own structs that I was hoping someone could answer. I have a struct where I can't derive serialize for because it has an IpAddr type in it. So I implemented the serialize trait the same way it was done for the point struct in the readme here. My first question is, was that the right way to proceed?

Second, when implementing the MapVisitor trait for the point struct again, what's the point of having a match statement for the states? Why shouldn't I do something like this instead.

impl<'a> serde::ser::MapVisitor for PointMapVisitor<'a> {
    fn visit<S>(&mut self, serializer: &mut S) -> Result<Option<()>, S::Error>
        where S: serde::Serializer
    {
        match self.state {
            0 => {
                self.state += 1;
               try!(serializer.visit_struct_elt("x", &self.value.x));
               Ok(Some(try!(serializer.visit_struct_elt("y", &self.value.y))))
            }
            _ => {
                self.state += 1;
                 Ok(None)
            }
        }
    }
}

The only reason I'm asking the second question is because for longer structs, it takes up a lot of space to do a n length match when serializing a n length struct. Is there a performance reason to split each visit_struct_elt into different visit?

Thanks for taking a look at this and all the awesome work!

rigdent commented 9 years ago

An update. It turns out that implementing the deserialize trait following the example the for Points in the readme doesn't allow you to deserialize using bincode (works fine with serde-json) because the visit function isn't supported in bincode.

Is there another way to use serde and bincode for structs that can't derive the Serialize and Deserialize traits (in my case because IpAddr isn't supported)?

Thanks!

oli-obk commented 9 years ago

The Points example actually doesn't implement the visit function. It implements the visit_map function. Maybe that works for you? if not, please post your de::Visitor and Deserialize implementations for your type.

rigdent commented 9 years ago

It's not that that Points example doesn't implement visit, it's that it uses the visit function. In the implementation of impl serde::Deserialize for PointField near the end, deserializer.visit(PointFieldVisitor) is called. And that visit function isn't supported by bincode.

I can get both the Points example and my struct, implementations based on the Points example, to serialize and deserialize to and from json. It's just that it seems that it currently isn't supported by bincode.

If you instead derive the Serialize and Deserialize the traits, it works with bincode fine. I just can't do that in my case.

Thanks for taking a look!

oli-obk commented 9 years ago

ah, you could just call visit_map right there and you should be fine.

rigdent commented 9 years ago

I just tried that and replacing visit with visit_map, and I still get an error. Here's a copy of the code I'm running. It's just the point example from the readme.

Thanks for the suggestion.

oli-obk commented 9 years ago

bincode actually serializes and deserializes structs as tuples. This is mostly fine. But during deserialization they should be turning the positions back into strings. I'll file a bug-report...

erickt commented 9 years ago

@rigdent: Glad you like it! You got a couple options here. First, you could make a newtype wrapper for IpAddr, as in struct IpAddr(net::IpAddr), and manually implement the trait for that. Then you could use #[derive(Serialize, Deserialize)] for your structure. Another option is to implement the traits for Ipv4Addr and the rest of std::net, which seems pretty reasonable to me. I added #181 to track that.

In regards to your example, #[derive(Deserialize)] actually generates:

struct PointVisitor;

impl serde::de::Visitor for PointVisitor {
    type Value = Point;

    fn visit_seq<V>(&mut self, mut visitor: V) -> Result<Point, V::Error>
        where V: serde::de::SeqVisitor
    {
        let x = match try!(visitor.visit()) {
            Some(value) => value,
            None => { return Err(serde::de::Error::end_of_stream()); }
        };

        let y = match try!(visitor.visit()) {
            Some(value) => value,
            None => { return Err(serde::de::Error::end_of_stream()); }
        };

        try!(visitor.end());

        Ok(Point{ x: x, y: y })
    }

    fn visit_map<V>(&mut self, mut visitor: V) -> Result<Point, V::Error>
        where V: serde::de::MapVisitor
    {
        let mut x = None;
        let mut y = None;

        loop {
            match try!(visitor.visit_key()) {
                Some(PointField::X) => { x = Some(try!(visitor.visit_value())); }
                Some(PointField::Y) => { y = Some(try!(visitor.visit_value())); }
                None => { break; }
            }
        }

        let x = match x {
            Some(x) => x,
            None => try!(visitor.missing_field("x")),
        };

        let y = match y {
            Some(y) => y,
            None => try!(visitor.missing_field("y")),
        };

        try!(visitor.end());

        Ok(Point{ x: x, y: y })
    }
}

It's a bit unfortunate that it has to kind of do the same thing twice. Unfortunately I haven't yet come up with a better approach.

rigdent commented 9 years ago

Thanks for the response! For the code generated by #[derive(Deserialize)], does it not need to generate the implementation of deserialize for PointField like the example? Also, What does the implementation for deserialize look like for Point itself? Thank you!

p.s. is there a way for me to see what #[derive(Deserialize) generates?

erickt commented 8 years ago

@rigdent: I added a Deserialization without Macros section that details what get's generated for #[derive(Deserialize)]. For your own code, if you're using nightly rust, you can do rustc ... -Z unstable-options --pretty expanded, and it'll print out the code with all the macros expanded. If you're using serde_codegen with stable rust, you can simply look in target/debug/build/$crate_$hash/out/$filename, where $crate is the crate you're compiling, $hash is some arbitrary hash, and $filename is whatever you specified in your build.rs.

jwilm commented 8 years ago

I'm dealing with a problem similar to this. Specifically, deserializer.visit(PointFieldVisitor) relies on the visit method. This wasn't an issue in the case of bincode because it serializes structs as sequences. I'm working on a serde wrapper for redis which, like bincode, does not store type data in the serialized format. Thus, I must rely on deserializer type hinting. However, unlike bincode, structs are serialized with keys (think hmgetall if you're familiar with redis). This becomes a problem for the case of the FieldVisitor which simply calls visit.

I could hack something into visit for my deserializer implementation, but that doesn't feel like the right answer long term. It seems that I would need to update the codegen to call something like visit_struct_field which could just map to visit by default, and then implementations like mine could reimplement it to just call visit_str or whatever is appropriate.

Thoughts?