Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

Value is not Decodable after the move to associated types #44

Open
drbawb opened this issue Jan 7, 2015 · 4 comments
Open

Value is not Decodable after the move to associated types #44

drbawb opened this issue Jan 7, 2015 · 4 comments

Comments

@drbawb
Copy link
Contributor

drbawb commented Jan 7, 2015

I have a branch that has been tracking rustc-serialize: drbawb/rust-msgpack/feature/rustc-serialize

After the latest changes to the serialize crate (see rust-lang/rust/pull/20514) I have been having lots of issues adopting the current architecture of the library to use associated types.

The biggest hurdle is that the signature of Decodable has changed to the following:

pub trait Decodable {
    fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error>;
}

Where D is bound by the trait serialize::Decoder.

This means that inside the decode function we do not have access to the concrete msgpack::Decoder<R> any more.

The previous definition of serialize::Decodable for Value relied on calling the msgpack::Decoder::decode_value() method, which is obviously not accessible through the Decoder trait.

I can't see any way to get access to the concrete decoder.
I even tried std::mem::transmute but because the type parameters for the underlying Reader is also removed from the serialize::Decoder trait: we lack the necessary type information to perform such a cast.


It would appear that the only option left is to re-implement msgpack::Decoder#decode_value() in terms of the generic serialize::Decoder trait.

@3Hren
Copy link

3Hren commented Jan 12, 2015

I also suffer from new Rust serialize interface.

It would appear that the only option left is to re-implement msgpack::Decoder#decode_value() in terms > of the generic serialize::Decoder trait.

But how? By reading a single byte one-by-one via read_u8() and analyzing it with further decoding?
What if you actually need to access Decoder<R: Reader> impl (for example, you might want to cache something)?

@drbawb
Copy link
Contributor Author

drbawb commented Jan 13, 2015

In looking at rustc-serialize::json: they don't even implement Decodable for their enum Json which is the equivalent of our enum Value type.

Instead they have a Builder which takes a Reader and yields a Json enumeration. (This is all hidden behind some helper functions: from_str, from_reader, etc.)

Then their implementation of Decoder just runs through this parsed representation.
So in essence json::Builder does all the work of actually parsing and decoding Json.

impl serialize::Decoder for json::Decoder {} is just there to shove the pre-parsed Json value into the requested Rust type. Since json::Builder doesn't implement the serialize::Decoder interface directly it can do all the caching, etc. that it wants.

The unfortunate thing about this strategy is that it appears to require reading the stream to completion.

@3Hren
Copy link

3Hren commented Jan 13, 2015

The unfortunate thing about this strategy is that it appears to require reading the stream to completion.

This is bad for me, because I use msgpack in conjunction with TcpStream, which yields msgpack object every time it's ready.
Moreover, this is bad even for json, because many applications need to stream json'ed values through sockets.

@drbawb
Copy link
Contributor Author

drbawb commented Jan 13, 2015

It's worth noting the distinction: the json::Builder which can be constructed from a Reader is built on top of json::Parser which is a streaming parser. -- json::Builder calls #read_to_end() on the Reader so constructing it from a TcpStream would not be a good idea.

In practice I can't say I've ever done that. I usually pull TcpStream#read() into a buffer and then construct the entire Json value from that buffer. In my recent applications I'm using websockets; but any protocol w/ basic framing can be used to guarantee you've received the entire message before trying to deserialize it.

If you really need to do streaming (e.g: you're receiving fragments of a message) then you would have to use serialize::json::Parser directly. Again that's not a common use-case for me; especially considering std::io is synchronous.

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

No branches or pull requests

2 participants