From ca6b082c0573f9d6f6c81403ac7ea4b5b78260d6 Mon Sep 17 00:00:00 2001 From: Dan Burkert Date: Sat, 27 Sep 2014 14:19:19 -0700 Subject: [PATCH 1/2] libserialize: tuple-arity should be provided to `Decoder::read_tuple` 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] --- src/librbml/lib.rs | 15 +++++++++++---- src/libserialize/json.rs | 34 ++++++++++++++++++++++++++++++---- src/libserialize/serialize.rs | 17 +++++++++++------ 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/librbml/lib.rs b/src/librbml/lib.rs index 95b6d9a17dd18..9bb5777ec9a5e 100644 --- a/src/librbml/lib.rs +++ b/src/librbml/lib.rs @@ -558,9 +558,15 @@ pub mod reader { } fn read_tuple(&mut self, - f: |&mut Decoder<'doc>, uint| -> DecodeResult) -> DecodeResult { + tuple_len: uint, + f: |&mut Decoder<'doc>| -> DecodeResult) -> DecodeResult { debug!("read_tuple()"); - self.read_seq(f) + self.read_seq(|d, len| { + assert!(len == tuple_len, + "expected tuple of length `{}`, found tuple \ + of length `{}`", tuple_len, len); + f(d) + }) } fn read_tuple_arg(&mut self, idx: uint, f: |&mut Decoder<'doc>| -> DecodeResult) @@ -571,10 +577,11 @@ pub mod reader { fn read_tuple_struct(&mut self, name: &str, - f: |&mut Decoder<'doc>, uint| -> DecodeResult) + len: uint, + f: |&mut Decoder<'doc>| -> DecodeResult) -> DecodeResult { debug!("read_tuple_struct(name={})", name); - self.read_tuple(f) + self.read_tuple(len, f) } fn read_tuple_struct_arg(&mut self, diff --git a/src/libserialize/json.rs b/src/libserialize/json.rs index 84b5cdd3cc82b..e5847ce58db12 100644 --- a/src/libserialize/json.rs +++ b/src/libserialize/json.rs @@ -2153,9 +2153,14 @@ impl ::Decoder for Decoder { Ok(value) } - fn read_tuple(&mut self, f: |&mut Decoder, uint| -> DecodeResult) -> DecodeResult { + fn read_tuple(&mut self, tuple_len: uint, f: |&mut Decoder| -> DecodeResult) -> DecodeResult { debug!("read_tuple()"); - self.read_seq(f) + self.read_seq(|d, len| { + assert!(len == tuple_len, + "expected tuple of length `{}`, found tuple \ + of length `{}`", tuple_len, len); + f(d) + }) } fn read_tuple_arg(&mut self, @@ -2167,10 +2172,11 @@ impl ::Decoder for Decoder { fn read_tuple_struct(&mut self, name: &str, - f: |&mut Decoder, uint| -> DecodeResult) + len: uint, + f: |&mut Decoder| -> DecodeResult) -> DecodeResult { debug!("read_tuple_struct(name={})", name); - self.read_tuple(f) + self.read_tuple(len, f) } fn read_tuple_struct_arg(&mut self, @@ -2872,6 +2878,26 @@ 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] + #[should_fail] + fn test_decode_tuple_malformed_length() { + let _ = super::decode::<(uint, uint)>("[1, 2, 3]"); + } + #[test] fn test_read_object() { assert_eq!(from_str("{"), Err(SyntaxError(EOFWhileParsingObject, 1, 2))); diff --git a/src/libserialize/serialize.rs b/src/libserialize/serialize.rs index 1cd1738e36972..b7c37defbfa4e 100644 --- a/src/libserialize/serialize.rs +++ b/src/libserialize/serialize.rs @@ -142,12 +142,13 @@ pub trait Decoder { f: |&mut Self| -> Result) -> Result; - fn read_tuple(&mut self, f: |&mut Self, uint| -> Result) -> Result; + fn read_tuple(&mut self, len: uint, f: |&mut Self| -> Result) -> Result; fn read_tuple_arg(&mut self, a_idx: uint, f: |&mut Self| -> Result) -> Result; fn read_tuple_struct(&mut self, s_name: &str, - f: |&mut Self, uint| -> Result) + len: uint, + f: |&mut Self| -> Result) -> Result; fn read_tuple_struct_arg(&mut self, a_idx: uint, @@ -465,20 +466,24 @@ impl,T:Decodable> Decodable for Option { 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,$($name:Decodable),*> Decodable 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); }) } From 05f6bdaefc7e0b94ea0b1892e6fe5dc79cd2514d Mon Sep 17 00:00:00 2001 From: Dan Burkert Date: Sat, 25 Oct 2014 09:29:41 -0700 Subject: [PATCH 2/2] Tuple deserialization should not fail --- src/librbml/lib.rs | 10 ++++++---- src/libserialize/json.rs | 17 ++++++++++------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/librbml/lib.rs b/src/librbml/lib.rs index 9bb5777ec9a5e..1dfc0d970a91c 100644 --- a/src/librbml/lib.rs +++ b/src/librbml/lib.rs @@ -562,10 +562,12 @@ pub mod reader { f: |&mut Decoder<'doc>| -> DecodeResult) -> DecodeResult { debug!("read_tuple()"); self.read_seq(|d, len| { - assert!(len == tuple_len, - "expected tuple of length `{}`, found tuple \ - of length `{}`", tuple_len, len); - f(d) + if len == tuple_len { + f(d) + } else { + Err(Expected(format!("Expected tuple of length `{}`, \ + found tuple of length `{}`", tuple_len, len))) + } }) } diff --git a/src/libserialize/json.rs b/src/libserialize/json.rs index e5847ce58db12..06f934c075d1a 100644 --- a/src/libserialize/json.rs +++ b/src/libserialize/json.rs @@ -2153,13 +2153,17 @@ impl ::Decoder for Decoder { Ok(value) } - fn read_tuple(&mut self, tuple_len: uint, f: |&mut Decoder| -> DecodeResult) -> DecodeResult { + fn read_tuple(&mut self, + tuple_len: uint, + f: |&mut Decoder| -> DecodeResult) + -> DecodeResult { debug!("read_tuple()"); self.read_seq(|d, len| { - assert!(len == tuple_len, - "expected tuple of length `{}`, found tuple \ - of length `{}`", tuple_len, len); - f(d) + if len == tuple_len { + f(d) + } else { + Err(ExpectedError(format!("Tuple{}", tuple_len), format!("Tuple{}", len))) + } }) } @@ -2893,9 +2897,8 @@ mod tests { } #[test] - #[should_fail] fn test_decode_tuple_malformed_length() { - let _ = super::decode::<(uint, uint)>("[1, 2, 3]"); + assert!(super::decode::<(uint, uint)>("[1, 2, 3]").is_err()); } #[test]