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

Packed encoding of enum still includes variant names for non-unit variants #187

Open
joshtriplett opened this issue May 8, 2020 · 1 comment · May be fixed by #215
Open

Packed encoding of enum still includes variant names for non-unit variants #187

joshtriplett opened this issue May 8, 2020 · 1 comment · May be fixed by #215

Comments

@joshtriplett
Copy link

Test case:

use serde::{Deserialize, Serialize};
use std::io::Write;

#[derive(Debug, Deserialize, Serialize)]
enum Thing {
    Thing1(String),
    Thing2(u64),
    Thing3(Box<[Thing]>),
    Thing4,
}

fn main() -> anyhow::Result<()> {
    let thing = Thing::Thing3(vec![Thing::Thing2(42), Thing::Thing4].into_boxed_slice());
    let v = serde_cbor::ser::to_vec_packed(&thing)?;
    std::fs::File::create("cbor")?.write_all(&v)?;
    Ok(())
}

Hex dump of the resulting file:

00000000  a1 66 54 68 69 6e 67 33  82 a1 66 54 68 69 6e 67  |.fThing3..fThing|
00000010  32 18 2a 03                                       |2.*.|

Notice that it includes Thing3 and Thing2, but not Thing4.

From what I can tell, the serialization of unit variants pays attention to self.packed, but the serialization of non-unit variants does not.

matix2267 added a commit to matix2267/cbor that referenced this issue Jun 14, 2021
Fixes pyfisch#187

Legacy enum format supported packed encoding pretty well but with the
switch to new enum format there was a regression (pyfisch#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).
@matix2267 matix2267 linked a pull request Jun 14, 2021 that will close this issue
@matix2267
Copy link

I've created a bugfix for this issue (#215) but I don't have high hopes to get it merged given that this project is effectively unmaintained (see #179).

Feel free to use the bugfix repo directly if it fits your use case.

matix2267 added a commit to matix2267/cbor that referenced this issue May 22, 2022
Fixes pyfisch#187

Legacy enum format supported packed encoding pretty well but with the
switch to new enum format there was a regression (pyfisch#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).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants