Skip to content

Commit

Permalink
auto merge of rust-lang#17595 : danburkert/rust/tuple-index-deseriali…
Browse files Browse the repository at this point in the history
…zation, r=alexcrichton

Currently `Decoder` implementations are not provided the tuple arity as
a parameter to `read_tuple`. This forces all encoder/decoder combos to
serialize the arity along with the elements. Tuple-arity is always known
statically at the decode site, because it is part of the type of the
tuple, so it could instead be provided as an argument to `read_tuple`,
as it is to `read_struct`.

The upside to this is that serialized tuples could become smaller in
encoder/decoder implementations which choose not to serialize type
(arity) information. For example, @TyOverby's
[binary-encode](https://github.com/TyOverby/binary-encode) format is
currently forced to serialize the tuple-arity along with every tuple,
despite the information being statically known at the decode site.

A downside to this change is that the tuple-arity of serialized tuples
can no longer be automatically checked during deserialization. However,
for formats which do serialize the tuple-arity, either explicitly (rbml)
or implicitly (json), this check can be added to the `read_tuple` method.

The signature of `Deserialize::read_tuple` and
`Deserialize::read_tuple_struct` are changed, and thus binary
backwards-compatibility is broken. This change does *not* force
serialization formats to change, and thus does not break decoding values
serialized prior to this change.

[breaking-change]
  • Loading branch information
bors committed Nov 1, 2014
2 parents 39f90ae + 05f6bda commit 3327ecc
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 14 deletions.
17 changes: 13 additions & 4 deletions src/librbml/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,17 @@ pub mod reader {
}

fn read_tuple<T>(&mut self,
f: |&mut Decoder<'doc>, uint| -> DecodeResult<T>) -> DecodeResult<T> {
tuple_len: uint,
f: |&mut Decoder<'doc>| -> DecodeResult<T>) -> DecodeResult<T> {
debug!("read_tuple()");
self.read_seq(f)
self.read_seq(|d, len| {
if len == tuple_len {
f(d)
} else {
Err(Expected(format!("Expected tuple of length `{}`, \
found tuple of length `{}`", tuple_len, len)))
}
})
}

fn read_tuple_arg<T>(&mut self, idx: uint, f: |&mut Decoder<'doc>| -> DecodeResult<T>)
Expand All @@ -571,10 +579,11 @@ pub mod reader {

fn read_tuple_struct<T>(&mut self,
name: &str,
f: |&mut Decoder<'doc>, uint| -> DecodeResult<T>)
len: uint,
f: |&mut Decoder<'doc>| -> DecodeResult<T>)
-> DecodeResult<T> {
debug!("read_tuple_struct(name={})", name);
self.read_tuple(f)
self.read_tuple(len, f)
}

fn read_tuple_struct_arg<T>(&mut self,
Expand Down
37 changes: 33 additions & 4 deletions src/libserialize/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2153,9 +2153,18 @@ impl ::Decoder<DecoderError> for Decoder {
Ok(value)
}

fn read_tuple<T>(&mut self, f: |&mut Decoder, uint| -> DecodeResult<T>) -> DecodeResult<T> {
fn read_tuple<T>(&mut self,
tuple_len: uint,
f: |&mut Decoder| -> DecodeResult<T>)
-> DecodeResult<T> {
debug!("read_tuple()");
self.read_seq(f)
self.read_seq(|d, len| {
if len == tuple_len {
f(d)
} else {
Err(ExpectedError(format!("Tuple{}", tuple_len), format!("Tuple{}", len)))
}
})
}

fn read_tuple_arg<T>(&mut self,
Expand All @@ -2167,10 +2176,11 @@ impl ::Decoder<DecoderError> for Decoder {

fn read_tuple_struct<T>(&mut self,
name: &str,
f: |&mut Decoder, uint| -> DecodeResult<T>)
len: uint,
f: |&mut Decoder| -> DecodeResult<T>)
-> DecodeResult<T> {
debug!("read_tuple_struct(name={})", name);
self.read_tuple(f)
self.read_tuple(len, f)
}

fn read_tuple_struct_arg<T>(&mut self,
Expand Down Expand Up @@ -2872,6 +2882,25 @@ mod tests {
assert_eq!(v, vec![vec![3], vec![1, 2]]);
}

#[test]
fn test_decode_tuple() {
let t: (uint, uint, uint) = super::decode("[1, 2, 3]").unwrap();
assert_eq!(t, (1u, 2, 3))

let t: (uint, string::String) = super::decode("[1, \"two\"]").unwrap();
assert_eq!(t, (1u, "two".to_string()));
}

#[test]
fn test_decode_tuple_malformed_types() {
assert!(super::decode::<(uint, string::String)>("[1, 2]").is_err());
}

#[test]
fn test_decode_tuple_malformed_length() {
assert!(super::decode::<(uint, uint)>("[1, 2, 3]").is_err());
}

#[test]
fn test_read_object() {
assert_eq!(from_str("{"), Err(SyntaxError(EOFWhileParsingObject, 1, 2)));
Expand Down
17 changes: 11 additions & 6 deletions src/libserialize/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,13 @@ pub trait Decoder<E> {
f: |&mut Self| -> Result<T, E>)
-> Result<T, E>;

fn read_tuple<T>(&mut self, f: |&mut Self, uint| -> Result<T, E>) -> Result<T, E>;
fn read_tuple<T>(&mut self, len: uint, f: |&mut Self| -> Result<T, E>) -> Result<T, E>;
fn read_tuple_arg<T>(&mut self, a_idx: uint, f: |&mut Self| -> Result<T, E>) -> Result<T, E>;

fn read_tuple_struct<T>(&mut self,
s_name: &str,
f: |&mut Self, uint| -> Result<T, E>)
len: uint,
f: |&mut Self| -> Result<T, E>)
-> Result<T, E>;
fn read_tuple_struct_arg<T>(&mut self,
a_idx: uint,
Expand Down Expand Up @@ -465,20 +466,24 @@ impl<E, D:Decoder<E>,T:Decodable<D, E>> Decodable<D, E> for Option<T> {

macro_rules! peel(($name:ident, $($other:ident,)*) => (tuple!($($other,)*)))

/// Evaluates to the number of identifiers passed to it, for example: `count_idents!(a, b, c) == 3
macro_rules! count_idents {
() => { 0u };
($_i:ident $(, $rest:ident)*) => { 1 + count_idents!($($rest),*) }
}

macro_rules! tuple (
() => ();
( $($name:ident,)+ ) => (
impl<E, D:Decoder<E>,$($name:Decodable<D, E>),*> Decodable<D,E> for ($($name,)*) {
#[allow(non_snake_case)]
fn decode(d: &mut D) -> Result<($($name,)*), E> {
d.read_tuple(|d, amt| {
let len: uint = count_idents!($($name),*);
d.read_tuple(len, |d| {
let mut i = 0;
let ret = ($(try!(d.read_tuple_arg({ i+=1; i-1 }, |d| -> Result<$name,E> {
Decodable::decode(d)
})),)*);
assert!(amt == i,
"expected tuple of length `{}`, found tuple \
of length `{}`", i, amt);
return Ok(ret);
})
}
Expand Down

0 comments on commit 3327ecc

Please sign in to comment.