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

Commit

Permalink
Fix encoding of packed variants in new enum format
Browse files Browse the repository at this point in the history
Fixes #187

Legacy enum format supported packed encoding pretty well but with the
switch to new enum format there was a regression (#173).
This commit fixes the new enum format in packed encoding.

For example consider the following structure:
```rust
enum Enum {
    Unit,
    NewType(i32),
    Tuple(i32, i32),
    Struct { x: i32, y: i32 },
}
```

Legacy enum packed encodings are:
```
Empty: <variant number>
NewType: [<variant number>, value]
Tuple: [<variant number>, values..]
Struct: [<variant number>, {<struct>}]
```

Non-legacy enum packed encodings before this commit:
```
Empty: <variant number>
NewType: {"<variant>": value}
Tuple: {"<variant>": [values..]}
Struct: {<variant number>: {<struct>}}
```
Notice how NewType and Tuple store the name instead of variant number.

Fixed after this commit:
```
Empty: <variant number>
NewType: {<variant number>: value}
Tuple: {<variant number>: [values..]}
Struct: {<variant number>: {<struct>}}
```

After this commit the packed encoding can be briefly described as:
All struct fields and enum variants are encoded as their field number
rather than name.
This applies to all types of members (unit, newtype, tuple and struct).
  • Loading branch information
matix2267 committed Jun 14, 2021
1 parent a218403 commit 521335b
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 32 deletions.
29 changes: 0 additions & 29 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,35 +158,6 @@
//! assert_eq!(cbor.len(), 1);
//!
//! let cbor = to_vec_packed(&SecondVariant(0)).unwrap();
//! assert_eq!(cbor.len(), 16); // Includes 13 bytes of "SecondVariant"
//! ```
//!
//! Serialize using minimal encoding
//!
//! ```rust
//! use serde_derive::{Deserialize, Serialize};
//! use serde_cbor::{Result, Serializer, ser::{self, IoWrite}};
//! use WithTwoVariants::*;
//!
//! fn to_vec_minimal<T>(value: &T) -> Result<Vec<u8>>
//! where
//! T: serde::Serialize,
//! {
//! let mut vec = Vec::new();
//! value.serialize(&mut Serializer::new(&mut IoWrite::new(&mut vec)).packed_format().legacy_enums())?;
//! Ok(vec)
//! }
//!
//! #[derive(Debug, Serialize, Deserialize)]
//! enum WithTwoVariants {
//! FirstVariant,
//! SecondVariant(u8),
//! }
//!
//! let cbor = to_vec_minimal(&FirstVariant).unwrap();
//! assert_eq!(cbor.len(), 1);
//!
//! let cbor = to_vec_minimal(&SecondVariant(0)).unwrap();
//! assert_eq!(cbor.len(), 3);
//! ```
//!
Expand Down
12 changes: 10 additions & 2 deletions src/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,11 @@ where
{
if self.enum_as_map {
self.write_u64(5, 1u64)?;
variant.serialize(&mut *self)?;
if self.packed {
variant_index.serialize(&mut *self)?;
} else {
variant.serialize(&mut *self)?;
}
} else {
self.writer.write_all(&[4 << 5 | 2]).map_err(|e| e.into())?;
self.serialize_unit_variant(name, variant_index, variant)?;
Expand Down Expand Up @@ -464,7 +468,11 @@ where
) -> Result<&'a mut Serializer<W>> {
if self.enum_as_map {
self.write_u64(5, 1u64)?;
variant.serialize(&mut *self)?;
if self.packed {
variant_index.serialize(&mut *self)?;
} else {
variant.serialize(&mut *self)?;
}
self.serialize_tuple(len)
} else {
self.write_u64(4, (len + 1) as u64)?;
Expand Down
47 changes: 46 additions & 1 deletion tests/enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn test_simple_data_enum_roundtrip() {
mod std_tests {
use std::collections::BTreeMap;

use serde_cbor::ser::{IoWrite, Serializer};
use serde_cbor::ser::{to_vec_packed, IoWrite, Serializer};
use serde_cbor::value::Value;
use serde_cbor::{from_slice, to_vec};

Expand Down Expand Up @@ -233,4 +233,49 @@ mod std_tests {
let point_map_ds = from_slice(&point_map_s).unwrap();
assert_eq!(Bar::Point { x: 5, y: -5 }, point_map_ds);
}

#[test]
fn test_packed_variants() {
// unit variants serialize as just the <variant number>
let empty_s = to_vec_packed(&Bar::Empty).unwrap();
let empty_num_s = to_vec_packed(&0).unwrap();
assert_eq!(empty_s, empty_num_s);

// newtype variants serialize like {<variant number>: value}
let number_s = to_vec_packed(&Bar::Number(42)).unwrap();
let mut number_map = BTreeMap::new();
number_map.insert(1, 42);
let number_map_s = to_vec_packed(&number_map).unwrap();
assert_eq!(number_s, number_map_s);

// multi-element tuple variants serialize like {<variant number>: [values..]}
let flag_s = to_vec_packed(&Bar::Flag("foo".to_string(), true)).unwrap();
let mut flag_map = BTreeMap::new();
flag_map.insert(2, vec![Value::Text("foo".to_string()), Value::Bool(true)]);
let flag_map_s = to_vec_packed(&flag_map).unwrap();
assert_eq!(flag_s, flag_map_s);

// struct-variants serialize like {<variant number>, {struct..}}
let point_s = to_vec_packed(&Bar::Point { x: 5, y: -5 }).unwrap();
let mut struct_map = BTreeMap::new();
struct_map.insert(Value::Integer(0), Value::Integer(5));
struct_map.insert(Value::Integer(1), Value::Integer(-5));
let mut point_map = BTreeMap::new();
point_map.insert(3, Value::Map(struct_map));
let point_map_s = to_vec_packed(&point_map).unwrap();
assert_eq!(point_s, point_map_s);

// deserialization of all encodings should just work
let empty_ds = from_slice(&empty_s).unwrap();
assert_eq!(Bar::Empty, empty_ds);

let number_ds = from_slice(&number_s).unwrap();
assert_eq!(Bar::Number(42), number_ds);

let flag_ds = from_slice(&flag_s).unwrap();
assert_eq!(Bar::Flag("foo".to_string(), true), flag_ds);

let point_ds = from_slice(&point_s).unwrap();
assert_eq!(Bar::Point { x: 5, y: -5 }, point_ds);
}
}

0 comments on commit 521335b

Please sign in to comment.