Skip to content

Commit

Permalink
Use three integer Value variants
Browse files Browse the repository at this point in the history 
 UnsignedInteger(u64): for all non-negative values

 SignedInteger(i64): for negative values that fit in a i64.

 LargeSignedInteger(i128): for negative values that can't be represented
 by i64.

 This should fix pyfisch#140.

Signed-off-by: Mohammad AlSaleh <CE.Mohammad.AlSaleh@gmail.com>
  • Loading branch information
@MoSal
MoSal committed Nov 24, 2019
1 parent 7d1d6d3 commit d76a5a0
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 54 deletions.
2 changes: 1 addition & 1 deletion src/de.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ where
/// ];
/// let mut it = Deserializer::from_slice(&data[..]).into_iter::<Value>();
/// assert_eq!(
/// Value::Integer(1),
/// Value::UnsignedInteger(1),
/// it.next().unwrap().unwrap()
/// );
/// assert_eq!(
Expand Down
16 changes: 13 additions & 3 deletions src/value/de.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,33 @@ impl<'de> de::Deserialize<'de> for Value {
where
E: de::Error,
{
Ok(Value::Integer(v.into()))
Ok(Value::UnsignedInteger(v))
}

#[inline]
fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(Value::Integer(v.into()))
Ok(Value::SignedInteger(v))
}

#[inline]
fn visit_i128<E>(self, v: i128) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(Value::Integer(v))
if v > 2i128.pow(64)-1 || v < -(2i128.pow(64)) {
return Err(E::custom("Integer value must be between -2^64 and 2^64-1"));
}

if v >= 0 {
Ok(Value::UnsignedInteger(v as u64))
} else if v >= 2i128.pow(63) {
Ok(Value::SignedInteger(v as i64))
} else {
Ok(Value::LargeSignedInteger(v))
}
}

#[inline]
Expand Down
57 changes: 41 additions & 16 deletions src/value/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,21 @@ pub enum Value {
Null,
/// Represents a boolean value.
Bool(bool),
/// Integer CBOR numbers.
/// Integer CBOR non-negative numbers.
///
/// The biggest value that can be represented is 2^64 - 1.
/// While the smallest value is -2^64.
/// Values outside this range can't be serialized
UnsignedInteger(u64),
/// Integer CBOR possibly-negative numbers within the i64 range.
///
/// Numbers smaller than -2^63
/// The smallest value that can be represented is -2^64.
SignedInteger(i64),
/// Integer CBOR possibly-negative numbers within the i28 range.
///
/// For numbers smaller than -2^63
///
/// Values smaller than -2^64 can't be serialized
/// and will cause an error.
Integer(i128),
LargeSignedInteger(i128),
/// Represents a floating point value.
Float(f64),
/// Represents a byte string.
Expand Down Expand Up @@ -84,7 +92,16 @@ impl Ord for Value {
return self.major_type().cmp(&other.major_type());
}
match (self, other) {
(Integer(a), Integer(b)) => a.abs().cmp(&b.abs()),
(UnsignedInteger(a), UnsignedInteger(b)) => a.cmp(b),
// Use i128 to avoid possible panic if abs() is called on -2^63
(SignedInteger(a), SignedInteger(b)) => i128::from(*a).abs().cmp(&i128::from(*b).abs()),
(LargeSignedInteger(a), LargeSignedInteger(b)) => a.abs().cmp(&b.abs()),
(UnsignedInteger(a), SignedInteger(b)) => i128::from(*a).abs().cmp(&i128::from(*b).abs()),
(SignedInteger(a), UnsignedInteger(b)) => i128::from(*a).abs().cmp(&i128::from(*b).abs()),
(LargeSignedInteger(a), UnsignedInteger(b)) => a.abs().cmp(&i128::from(*b).abs()),
(UnsignedInteger(a), LargeSignedInteger(b)) => i128::from(*a).abs().cmp(&b.abs()),
(SignedInteger(a), LargeSignedInteger(b)) => i128::from(*a).abs().cmp(&b.abs()),
(LargeSignedInteger(a), SignedInteger(b)) => a.abs().cmp(&i128::from(*b).abs()),
(Bytes(a), Bytes(b)) if a.len() != b.len() => a.len().cmp(&b.len()),
(Text(a), Text(b)) if a.len() != b.len() => a.len().cmp(&b.len()),
(Array(a), Array(b)) if a.len() != b.len() => a.len().cmp(&b.len()),
Expand All @@ -111,15 +128,15 @@ macro_rules! impl_from {
}

impl_from!(Value::Bool, bool);
impl_from!(Value::Integer, i8);
impl_from!(Value::Integer, i16);
impl_from!(Value::Integer, i32);
impl_from!(Value::Integer, i64);
impl_from!(Value::SignedInteger, i8);
impl_from!(Value::SignedInteger, i16);
impl_from!(Value::SignedInteger, i32);
impl_from!(Value::SignedInteger, i64);
// i128 omitted because not all numbers fit in CBOR serialization
impl_from!(Value::Integer, u8);
impl_from!(Value::Integer, u16);
impl_from!(Value::Integer, u32);
impl_from!(Value::Integer, u64);
impl_from!(Value::UnsignedInteger, u8);
impl_from!(Value::UnsignedInteger, u16);
impl_from!(Value::UnsignedInteger, u32);
impl_from!(Value::UnsignedInteger, u64);
// u128 omitted because not all numbers fit in CBOR serialization
impl_from!(Value::Float, f32);
impl_from!(Value::Float, f64);
Expand All @@ -135,13 +152,21 @@ impl Value {
match self {
Null => 7,
Bool(_) => 7,
Integer(v) => {
UnsignedInteger(_) => 0,
SignedInteger(v) => {
if *v >= 0 {
0
} else {
1
}
}
},
LargeSignedInteger(v) => {
if *v >= 0 {
0
} else {
1
}
},
Float(_) => 7,
Bytes(_) => 2,
Text(_) => 3,
Expand Down
20 changes: 16 additions & 4 deletions src/value/ser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ impl serde::Serialize for Value {
S: serde::Serializer,
{
match *self {
Value::Integer(v) => serializer.serialize_i128(v),
Value::UnsignedInteger(v) => serializer.serialize_u64(v),
Value::SignedInteger(v) => serializer.serialize_i64(v),
Value::LargeSignedInteger(v) => serializer.serialize_i128(v),
Value::Bytes(ref v) => serializer.serialize_bytes(&v),
Value::Text(ref v) => serializer.serialize_str(&v),
Value::Array(ref v) => v.serialize(serializer),
Expand Down Expand Up @@ -69,11 +71,21 @@ impl serde::Serializer for Serializer {

#[inline]
fn serialize_i64(self, value: i64) -> Result<Value, Error> {
self.serialize_i128(i128::from(value))
Ok(Value::SignedInteger(value))
}

fn serialize_i128(self, value: i128) -> Result<Value, Error> {
Ok(Value::Integer(value))
if value > 2i128.pow(64)-1 || value < -(2i128.pow(64)) {
return Err(Error::message("The number can't be stored in CBOR"));
}

if value >= 0 {
Ok(Value::UnsignedInteger(value as u64))
} else if value >= 2i128.pow(63) {
Ok(Value::SignedInteger(value as i64))
} else {
Ok(Value::LargeSignedInteger(value))
}
}

#[inline]
Expand All @@ -93,7 +105,7 @@ impl serde::Serializer for Serializer {

#[inline]
fn serialize_u64(self, value: u64) -> Result<Value, Error> {
Ok(Value::Integer(value.into()))
Ok(Value::UnsignedInteger(value))
}

#[inline]
Expand Down
16 changes: 8 additions & 8 deletions tests/canonical.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ mod std_tests {
-4294967296,
]
.iter()
.map(|i| Value::Integer(*i))
.map(|i| Value::SignedInteger(*i))
.collect::<Vec<_>>();

let mut sorted = expected.clone();
Expand Down Expand Up @@ -71,8 +71,8 @@ mod std_tests {
#[test]
fn major_type_canonical_sort_order() {
let expected = vec![
Value::Integer(0),
Value::Integer(-1),
Value::UnsignedInteger(0),
Value::SignedInteger(-1),
Value::Bytes(vec![]),
Value::Text("".to_string()),
Value::Null,
Expand All @@ -88,13 +88,13 @@ mod std_tests {
fn test_rfc_example() {
// See: https://tools.ietf.org/html/draft-ietf-cbor-7049bis-04#section-4.10
let expected = vec![
Value::Integer(10),
Value::Integer(100),
Value::Integer(-1),
Value::UnsignedInteger(10),
Value::UnsignedInteger(100),
Value::SignedInteger(-1),
Value::Text("z".to_owned()),
Value::Text("aa".to_owned()),
Value::Array(vec![Value::Integer(100)]),
Value::Array(vec![Value::Integer(-1)]),
Value::Array(vec![Value::UnsignedInteger(100)]),
Value::Array(vec![Value::SignedInteger(-1)]),
Value::Bool(false),
];
let mut sorted = expected.clone();
Expand Down
38 changes: 19 additions & 19 deletions tests/de.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,19 @@ mod std_tests {
#[test]
fn test_numbers1() {
let value: error::Result<Value> = de::from_slice(&[0x00]);
assert_eq!(value.unwrap(), Value::Integer(0));
assert_eq!(value.unwrap(), Value::UnsignedInteger(0));
}

#[test]
fn test_numbers2() {
let value: error::Result<Value> = de::from_slice(&[0x1a, 0x00, 0xbc, 0x61, 0x4e]);
assert_eq!(value.unwrap(), Value::Integer(12345678));
assert_eq!(value.unwrap(), Value::UnsignedInteger(12345678));
}

#[test]
fn test_numbers3() {
let value: error::Result<Value> = de::from_slice(&[0x39, 0x07, 0xde]);
assert_eq!(value.unwrap(), Value::Integer(-2015));
assert_eq!(value.unwrap(), Value::SignedInteger(-2015));
}

#[test]
Expand All @@ -120,9 +120,9 @@ mod std_tests {
assert_eq!(
value.unwrap(),
Value::Array(vec![
Value::Integer(1),
Value::Integer(2),
Value::Integer(3)
Value::UnsignedInteger(1),
Value::UnsignedInteger(2),
Value::UnsignedInteger(3)
])
);
}
Expand All @@ -133,10 +133,10 @@ mod std_tests {
assert_eq!(
value.unwrap(),
Value::Array(vec![
Value::Integer(1),
Value::UnsignedInteger(1),
Value::Array(vec![
Value::Integer(2),
Value::Array(vec![Value::Integer(3)])
Value::UnsignedInteger(2),
Value::Array(vec![Value::UnsignedInteger(3)])
])
])
);
Expand All @@ -158,10 +158,10 @@ mod std_tests {
fn test_indefinite_object() {
let value: error::Result<Value> = de::from_slice(b"\xbfaa\x01ab\x9f\x02\x03\xff\xff");
let mut object = BTreeMap::new();
object.insert(Value::Text("a".to_owned()), Value::Integer(1));
object.insert(Value::Text("a".to_owned()), Value::UnsignedInteger(1));
object.insert(
Value::Text("b".to_owned()),
Value::Array(vec![Value::Integer(2), Value::Integer(3)]),
Value::Array(vec![Value::UnsignedInteger(2), Value::UnsignedInteger(3)]),
);
assert_eq!(value.unwrap(), Value::Map(object));
}
Expand All @@ -172,9 +172,9 @@ mod std_tests {
assert_eq!(
value.unwrap(),
Value::Array(vec![
Value::Integer(1),
Value::Integer(2),
Value::Integer(3)
Value::UnsignedInteger(1),
Value::UnsignedInteger(2),
Value::UnsignedInteger(3)
])
);
}
Expand Down Expand Up @@ -246,9 +246,9 @@ mod std_tests {
assert_eq!(
value,
vec![
Value::Integer(123456789959),
Value::Integer(-34567897654325468),
Value::Integer(-456787678),
Value::UnsignedInteger(123456789959),
Value::SignedInteger(-34567897654325468),
Value::SignedInteger(-456787678),
Value::Bool(true),
Value::Null,
Value::Null,
Expand Down Expand Up @@ -330,7 +330,7 @@ mod std_tests {
fn stream_deserializer() {
let slice = b"\x01\x66foobar";
let mut it = Deserializer::from_slice(slice).into_iter::<Value>();
assert_eq!(Value::Integer(1), it.next().unwrap().unwrap());
assert_eq!(Value::UnsignedInteger(1), it.next().unwrap().unwrap());
assert_eq!(
Value::Text("foobar".to_string()),
it.next().unwrap().unwrap()
Expand All @@ -342,7 +342,7 @@ mod std_tests {
fn stream_deserializer_eof() {
let slice = b"\x01\x66foob";
let mut it = Deserializer::from_slice(slice).into_iter::<Value>();
assert_eq!(Value::Integer(1), it.next().unwrap().unwrap());
assert_eq!(Value::UnsignedInteger(1), it.next().unwrap().unwrap());
assert!(it.next().unwrap().unwrap_err().is_eof());
}

Expand Down
6 changes: 3 additions & 3 deletions tests/enum.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ mod std_tests {

// tuple-variants serialize like ["<variant>", values..]
let number_s = to_vec_legacy(&Bar::Number(42)).unwrap();
let number_vec = vec![Value::Text("Number".to_string()), Value::Integer(42)];
let number_vec = vec![Value::Text("Number".to_string()), Value::UnsignedInteger(42)];
let number_vec_s = to_vec_legacy(&number_vec).unwrap();
assert_eq!(number_s, number_vec_s);

Expand All @@ -175,8 +175,8 @@ mod std_tests {
// struct-variants serialize like ["<variant>", {struct..}]
let point_s = to_vec_legacy(&Bar::Point { x: 5, y: -5 }).unwrap();
let mut struct_map = BTreeMap::new();
struct_map.insert(Value::Text("x".to_string()), Value::Integer(5));
struct_map.insert(Value::Text("y".to_string()), Value::Integer(-5));
struct_map.insert(Value::Text("x".to_string()), Value::UnsignedInteger(5));
struct_map.insert(Value::Text("y".to_string()), Value::SignedInteger(-5));
let point_vec = vec![
Value::Text("Point".to_string()),
Value::Map(struct_map.clone()),
Expand Down

0 comments on commit d76a5a0

Please sign in to comment.