search

sharksforarms / deku

Declarative binary reading and writing: bit-level, symmetric, serialization/deserialization
Apache License 2.0
1.14k stars 55 forks source link

Draft: Support arbitrary enum discriminant #452

Open wcampbell0x2a opened 5 months ago

wcampbell0x2a commented 5 months ago

Closes #306

github-actions[bot] commented 5 months ago

Benchmark for 9d3ece1

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | 1258.8±9.31ns | **1223.8±24.98ns** | **-2.78%** | | deku_read_byte | **3.3±0.09ns** | 3.5±0.06ns | **+6.06%** | | deku_read_enum | **2.5±0.07ns** | 2.7±0.06ns | **+8.00%** | | deku_read_vec | **33.3±0.35ns** | 33.8±0.23ns | **+1.50%** | | deku_write_bits | 167.0±6.57ns | 168.0±3.12ns | +0.60% | | deku_write_byte | 22.9±0.50ns | 22.7±0.37ns | -0.87% | | deku_write_enum | 21.4±0.30ns | **21.1±0.21ns** | **-1.40%** | | deku_write_vec | 402.4±3.79ns | **387.4±4.37ns** | **-3.73%** |
sharksforarms commented 3 months ago

Remove support for defining both id = 1 and having a discriminant

I'm not convinced yet we want to do this as it removes some flexibility (what if I want to read/write some arcane binary format, but use it differently/idiomatically in Rust). One example would be identifiers like 0xAB + 1, 0xAB + 2, ... but in Rust I'd like to represent these as 1, 2, ...

Alternatively, maybe it's best to say "deku will use the attribute id if specified, else it will use the discriminant` wdyt?

wcampbell0x2a commented 3 months ago

Remove support for defining both id = 1 and having a discriminant

I'm not convinced yet we want to do this as it removes some flexibility (what if I want to read/write some arcane binary format, but use it differently/idiomatically in Rust). One example would be identifiers like 0xAB + 1, 0xAB + 2, ... but in Rust I'd like to represent these as 1, 2, ...

Alternatively, maybe it's best to say "deku will use the attribute id if specified, else it will use the discriminant` wdyt?

Agreed, when I have time to return to this I'll implement that.