sharksforarms / deku

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

Fix unknown id Type for EnumExt::deku_id() #398

Closed wcampbell0x2a closed 10 months ago

wcampbell0x2a commented 10 months ago

Remove unwrap causing the following error when the EnumExt tried to infer what type is was to return. An attribute could be added in the future to aid this, but for now we will just not emit the deku_id() for this type of enum. I decided in other causes (which aren't tested), to also remove the error and just not emit the function also.

help: message: called Result::unwrap() on an Err value: Error("expected ,")

Closes #397

github-actions[bot] commented 10 months ago

Benchmark for 65065c4

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | **1111.2±24.21ns** | 1156.7±25.73ns | **+4.09%** | | deku_read_byte | 22.5±0.62ns | 22.6±0.64ns | +0.44% | | deku_read_enum | 9.4±0.10ns | **9.2±0.25ns** | **-2.13%** | | deku_read_vec | 54.2±0.96ns | **52.8±1.41ns** | **-2.58%** | | deku_write_bits | 107.1±2.87ns | 108.1±6.18ns | +0.93% | | deku_write_byte | 121.6±5.47ns | 120.5±4.21ns | -0.90% | | deku_write_enum | 83.1±2.54ns | 81.8±2.75ns | -1.56% | | deku_write_vec | 3.0±0.06µs | 3.0±0.08µs | 0.00% |
sharksforarms commented 10 months ago

Fair enough. From a code-gen perspective, I'm not sure if there's a way of getting the type? probably some hackery via walking the struct/enum, etc.

We currently have type to specify the variant type... wonder if we could extend the use of it to include this case

wcampbell0x2a commented 10 months ago

Fair enough. From a code-gen perspective, I'm not sure if there's a way of getting the type? probably some hackery via walking the struct/enum, etc.

We currently have type to specify the variant type... wonder if we could extend the use of it to include this case

Right now we don't allow id and type(to be id_type in the future). I don't know if I have the time/patience right now to figure out syn invocations to parse that xD

sharksforarms commented 10 months ago

Fair enough. From a code-gen perspective, I'm not sure if there's a way of getting the type? probably some hackery via walking the struct/enum, etc. We currently have type to specify the variant type... wonder if we could extend the use of it to include this case

Right now we don't allow id and type(to be id_type in the future). I don't know if I have the time/patience right now to figure out syn invocations to parse that xD

Yeah I agree, I don't favor that approach. You're right, currently we can't use id and type together, what if we could? id - use id to select variant type - use type to read/write variant of type type id + type - use id to select variant with type type

github-actions[bot] commented 10 months ago

Benchmark for dc44332

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | 1199.3±16.88ns | 1187.8±25.00ns | -0.96% | | deku_read_byte | 23.3±0.34ns | 23.3±0.93ns | 0.00% | | deku_read_enum | **9.3±0.08ns** | 9.5±0.22ns | **+2.15%** | | deku_read_vec | **53.5±0.80ns** | 55.0±1.33ns | **+2.80%** | | deku_write_bits | 109.0±1.35ns | 109.7±3.33ns | +0.64% | | deku_write_byte | 123.6±3.85ns | 123.3±3.02ns | -0.24% | | deku_write_enum | 87.6±3.18ns | **83.3±1.78ns** | **-4.91%** | | deku_write_vec | 3.1±0.08µs | **3.0±0.05µs** | **-3.23%** |