Repository: sharksforarms/deku
Pull request #544 (2025-04-08 14:42:16)

allow id_pat without storage
Allow the use of `id_pat` without a storage field. In the case where a storage field is specified, we use that. Else, we try the enum discriminant. Finally, we fail if we cannot determine what to use to write the id.

Address https://github.com/sharksforarms/deku/issues/533#issuecomment-2781201234


github-actions[bot]: ## Benchmark for 1aac5f6
  <details>
    <summary>Click to view benchmark</summary>

| Test | Base         | PR               | % |
|------|--------------|------------------|---|
| count_from_u8_specialize | 95.2±0.79ns | **92.0±0.62ns** | **-3.36%** |
| count_no_specialize | 1699.7±25.26ns | 1700.2±33.59ns | +0.03% |
| count_specialize | 95.9±0.75ns | 96.4±1.07ns | +0.52% |
| deku_read_bits | 1223.1±21.10ns | 1229.7±16.73ns | +0.54% |
| deku_read_byte | 4.4±0.10ns | 4.4±0.05ns | 0.00% |
| deku_read_enum | 3.8±0.11ns | 3.8±0.03ns | 0.00% |
| deku_read_vec | 34.9±0.61ns | **30.6±0.33ns** | **-12.32%** |
| deku_write_bits | 260.8±4.14ns | **231.4±7.17ns** | **-11.27%** |
| deku_write_byte | 68.8±0.12ns | **23.7±0.43ns** | **-65.55%** |
| deku_write_enum | 67.2±0.34ns | **22.5±0.36ns** | **-66.52%** |
| deku_write_vec | 523.9±5.32ns | **497.9±7.34ns** | **-4.96%** |
| read_all | 4.3±0.05µs | 4.3±0.08µs | 0.00% |
| read_all_bytes | 5.1±0.03µs | **5.0±0.04µs** | **-1.96%** |

  </details>
  
github-actions[bot]: ## Benchmark for 624eaf2
  <details>
    <summary>Click to view benchmark</summary>

| Test | Base         | PR               | % |
|------|--------------|------------------|---|
| count_from_u8_specialize | 96.9±1.15ns | 96.3±1.39ns | -0.62% |
| count_no_specialize | 1699.4±26.26ns | 1700.5±26.10ns | +0.06% |
| count_specialize | 104.2±1.16ns | **93.5±0.87ns** | **-10.27%** |
| deku_read_bits | 1223.0±24.74ns | 1234.6±18.97ns | +0.95% |
| deku_read_byte | 4.4±0.06ns | 4.4±0.07ns | 0.00% |
| deku_read_enum | 3.8±0.02ns | 3.8±0.05ns | 0.00% |
| deku_read_vec | 30.6±0.65ns | 30.6±0.21ns | 0.00% |
| deku_write_bits | 246.6±5.00ns | **233.6±4.66ns** | **-5.27%** |
| deku_write_byte | 24.0±0.36ns | 24.0±0.31ns | 0.00% |
| deku_write_enum | 22.8±0.36ns | 22.8±0.39ns | 0.00% |
| deku_write_vec | **492.2±5.54ns** | 502.3±13.55ns | **+2.05%** |
| read_all | 4.3±0.07µs | **4.2±0.05µs** | **-2.33%** |
| read_all_bytes | **5.0±0.04µs** | 5.1±0.05µs | **+2.00%** |

  </details>
  
sharksforarms: @wcampbell0x2a done
sharksforarms: x-ref: https://github.com/sharksforarms/deku/pull/540

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9be46f1..a90c971 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -53,7 +53,6 @@ pub struct Data {
 
 ### Changed `id_pat` ([#540](https://github.com/sharksforarms/deku/pull/540))
 The `id_pat` attribute has been restored to the behavior of `0.16.0`, removing the seek and re-read.
-Added the requirement of the type used to read the id needs to be the same as the storage unit and force no attributes.
 
 ### Allow token streams for `bytes` attribute ([#489](https://github.com/sharksforarms/deku/pull/489))
 ```rs
diff --git a/deku-derive/src/macros/deku_read.rs b/deku-derive/src/macros/deku_read.rs
index 5ae5b7f..6cfccc7 100644
--- a/deku-derive/src/macros/deku_read.rs
+++ b/deku-derive/src/macros/deku_read.rs
@@ -257,7 +257,8 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
             let ident = &variant.ident;
             let internal_ident = gen_internal_field_ident(&quote!(#ident));
             pre_match_tokens.push(quote! {
-                let #internal_ident = <#id_type>::try_from(Self::#ident as isize)?;
+                // https://doc.rust-lang.org/reference/items/enumerations.html#r-items.enum.discriminant.access-memory
+                let #internal_ident = <#id_type>::try_from(unsafe { *(&Self::#ident as *const Self as *const #id_type) })?;
             });
             quote! { _ if __deku_variant_id == #internal_ident }
         } else {
@@ -278,15 +279,6 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
         let variant_read_func = if variant_reader.is_some() {
             quote! { #variant_reader; }
         } else {
-            // VarDefault { id: u8, value: u8 }, is allowed
-            // VarDefault, is not, if we need to store the id
-            if pad_id && variant.id.is_none() && variant.fields.is_empty() {
-                // TODO: This would be nice to point to the field
-                return Err(syn::Error::new(
-                    input.ident.span(),
-                    "DekuRead: id_pat requires storage field",
-                ));
-            }
             let (field_idents, field_reads) =
                 emit_field_reads(input, &variant.fields.as_ref(), &ident, pad_id)?;
 
diff --git a/deku-derive/src/macros/deku_write.rs b/deku-derive/src/macros/deku_write.rs
index d569cf4..7dc6560 100644
--- a/deku-derive/src/macros/deku_write.rs
+++ b/deku-derive/src/macros/deku_write.rs
@@ -263,17 +263,23 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
                         }
                     }
                 }
-            } else if variant.id_pat.is_some() {
+            } else if variant.id_pat.is_some() && !variant.fields.is_empty() {
+                // if the variant has fields, the first must be storing the id
                 quote! {}
             } else if has_discriminant {
                 quote! {
-                    let mut __deku_variant_id: #id_type = Self::#variant_ident as #id_type;
+                    // https://doc.rust-lang.org/reference/items/enumerations.html#r-items.enum.discriminant.access-memory
+                    let mut __deku_variant_id: #id_type = unsafe { *(&Self::#variant_ident as *const Self as *const #id_type) };
                     __deku_variant_id.to_writer(__deku_writer, (#id_args))?;
                 }
             } else {
                 return Err(syn::Error::new(
                     variant.ident.span(),
-                    "DekuWrite: `id` must be specified on non-unit variants",
+                    if variant.id_pat.is_some() && !has_discriminant {
+                        "DekuWrite: cannot determine write `id`. must provide storage for the id or discriminant"
+                    } else {
+                        "DekuWrite: `id` must be specified on non-unit variants"
+                    },
                 ));
             }
         } else {
@@ -431,15 +437,6 @@ fn emit_field_writes(
     object_prefix: Option<TokenStream>,
     ident: &TokenStream,
 ) -> Result<Vec<TokenStream>, syn::Error> {
-    // VarDefault { id: u8, value: u8 }, is allowed
-    // VarDefault, is not, if we need to store the id
-    if is_id_pat && input.id.is_none() && fields.is_empty() {
-        // TODO: This would be nice to point to the field
-        return Err(syn::Error::new(
-            input.ident.span(),
-            "DekuRead: id_pat requires storage field",
-        ));
-    }
     let mut is_id_pat = is_id_pat;
     fields
         .iter()
diff --git a/src/attributes.rs b/src/attributes.rs
index e5a23eb..b235c6e 100644
--- a/src/attributes.rs
+++ b/src/attributes.rs
@@ -1596,7 +1596,10 @@ assert_eq!(
 
 Specify the identifier in the form of a match pattern for the enum variant.
 
-The first field of the variant is used for storage, and must be the same type as `id_type` and no attributes.
+The first field of the variant may be used for storage, and must be the same type as `id_type` and no attributes.
+
+If no storage for the id is provided, the enum discriminent (if provided) will be used to write as the id for that variant
+
 The writing of the field will use the same options as the reading.
 
 Example:
diff --git a/src/error.rs b/src/error.rs
index d7739b8..d41571c 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -29,7 +29,7 @@ impl NeedSize {
     /// Number of bytes needed
     #[inline]
     pub fn byte_size(&self) -> usize {
-        (self.bits + 7) / 8
+        self.bits.div_ceil(8)
     }
 }
 
diff --git a/src/impls/hashset.rs b/src/impls/hashset.rs
index 6488760..df5b721 100644
--- a/src/impls/hashset.rs
+++ b/src/impls/hashset.rs
@@ -11,9 +11,9 @@ use crate::{DekuError, DekuReader, DekuWriter};
 /// * `capacity` - an optional capacity to pre-allocate the hashset with
 /// * `ctx` - The context required by `T`. It will be passed to every `T` when constructing.
 /// * `predicate` - the predicate that decides when to stop reading `T`s
-///    The predicate takes two parameters: the number of bits that have been read so far,
-///    and a borrow of the latest value to have been read. It should return `true` if reading
-///    should now stop, and `false` otherwise
+///   The predicate takes two parameters: the number of bits that have been read so far,
+///   and a borrow of the latest value to have been read. It should return `true` if reading
+///   should now stop, and `false` otherwise
 #[allow(clippy::type_complexity)]
 fn from_reader_with_ctx_hashset_with_predicate<'a, T, S, Ctx, Predicate, R: Read + Seek>(
     reader: &mut crate::reader::Reader<R>,
diff --git a/src/impls/primitive.rs b/src/impls/primitive.rs
index 00d32ab..439ada1 100644
--- a/src/impls/primitive.rs
+++ b/src/impls/primitive.rs
@@ -173,7 +173,7 @@ macro_rules! ImplDekuReadBits {
                 // PANIC: We already check that input.len() < bit_size above, so no panic will happen
                 let bit_slice = &input;
 
-                let pad = 8 * ((bit_slice.len() + 7) / 8) - bit_slice.len();
+                let pad = 8 * bit_slice.len().div_ceil(8) - bit_slice.len();
 
                 // if everything is aligned, just read the value
                 if pad == 0 && bit_slice.len() == MAX_TYPE_BITS {
@@ -291,7 +291,7 @@ macro_rules! ImplDekuReadBits {
 
                 let bit_slice = &input[..bit_size];
 
-                let pad = 8 * ((bit_slice.len() + 7) / 8) - bit_slice.len();
+                let pad = 8 * bit_slice.len().div_ceil(8) - bit_slice.len();
 
                 // if everything is aligned, just read the value
                 if pad == 0 && bit_slice.len() == MAX_TYPE_BITS {
@@ -1308,7 +1308,7 @@ mod tests {
     #[rstest(input, endian, byte_size, expected,
         case::normal_le(0xDDCC_BBAA, Endian::Little, None, vec![0xAA, 0xBB, 0xCC, 0xDD]),
         case::normal_be(0xDDCC_BBAA, Endian::Big, None, vec![0xDD, 0xCC, 0xBB, 0xAA]),
-        case::byte_size_be_smaller(0x00ffABAA, Endian::Big, Some(2), vec![0xab, 0xaa]),
+        case::byte_size_be_smaller(0x00FFABAA, Endian::Big, Some(2), vec![0xab, 0xaa]),
         #[should_panic(expected = "InvalidParam(\"byte size 10 is larger then input 4\")")]
         case::byte_size_le_bigger(0x03AB, Endian::Little, Some(10), vec![0xAB, 0b11_000000]),
     )]
diff --git a/src/lib.rs b/src/lib.rs
index 07fa62d..312438f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -336,7 +336,7 @@ environment, you will see logging messages as Deku does its deserialising.
 
 - With the use of the `no-assert-string` feature, you can remove the strings Deku adds to assertion errors.
 - `DekuError` whenever possible will use a `'static str`, to make the errors compile away when following a
-   guide such as [min-sized-rust](https://github.com/johnthagen/min-sized-rust).
+  guide such as [min-sized-rust](https://github.com/johnthagen/min-sized-rust).
 
 # Performance: Compile without `bitvec`
 The feature `bits` enables the `bitvec` crate to use when reading and writing, which is enabled by default.


Test patch:
diff --git a/tests/test_common/mod.rs b/tests/test_common/mod.rs
index 5d9eef4..91a44fa 100644
--- a/tests/test_common/mod.rs
+++ b/tests/test_common/mod.rs
@@ -1,3 +1,5 @@
+#![allow(missing_docs)]
+
 pub trait FromBeBytes: Sized {
     type Bytes;
     fn from_be_bytes(_: Self::Bytes) -> Self;
diff --git a/tests/test_compile/cases/enum_validation.rs b/tests/test_compile/cases/enum_validation.rs
index 9b27603..63aa645 100644
--- a/tests/test_compile/cases/enum_validation.rs
+++ b/tests/test_compile/cases/enum_validation.rs
@@ -124,7 +124,7 @@ pub enum Test17 {
     B,
 }
 
-// Test id_pat id storage must exist (write)
+// Test cannot determine id write
 #[derive(PartialEq, Debug, DekuWrite)]
 #[deku(id_type = "u8")]
 pub enum Test18 {
@@ -132,4 +132,13 @@ pub enum Test18 {
     B,
 }
 
+// Test #[repr(inttype)]
+#[derive(PartialEq, Debug, DekuWrite)]
+#[deku(id_type = "u8")]
+pub enum Test19 {
+    A = 0,
+    #[deku(id_pat = "_")]
+    B(u8),
+}
+
 fn main() {}
diff --git a/tests/test_compile/cases/enum_validation.stderr b/tests/test_compile/cases/enum_validation.stderr
index cd3da52..226398a 100644
--- a/tests/test_compile/cases/enum_validation.stderr
+++ b/tests/test_compile/cases/enum_validation.stderr
@@ -82,17 +82,17 @@ error: DekuRead: id_pat id storage cannot have attributes
 98 | pub enum Test14 {
    |          ^^^^^^
 
-error: DekuRead: id_pat requires storage field
-   --> tests/test_compile/cases/enum_validation.rs:122:10
+error: DekuWrite: cannot determine write `id`. must provide storage for the id or discriminant
+   --> tests/test_compile/cases/enum_validation.rs:132:5
     |
-122 | pub enum Test17 {
-    |          ^^^^^^
+132 |     B,
+    |     ^
 
-error: DekuRead: id_pat requires storage field
-   --> tests/test_compile/cases/enum_validation.rs:130:10
+error[E0732]: `#[repr(inttype)]` must be specified
+   --> tests/test_compile/cases/enum_validation.rs:138:1
     |
-130 | pub enum Test18 {
-    |          ^^^^^^
+138 | pub enum Test19 {
+    | ^^^^^^^^^^^^^^^
 
 error[E0308]: `?` operator has incompatible types
    --> tests/test_compile/cases/enum_validation.rs:104:28
diff --git a/tests/test_enum.rs b/tests/test_enum.rs
index cfb0110..c3f38e4 100644
--- a/tests/test_enum.rs
+++ b/tests/test_enum.rs
@@ -122,6 +122,96 @@ fn test_enum_array_type() {
     assert_eq!(input.to_vec(), ret_write);
 }
 
+#[cfg(feature = "bits")]
+#[test]
+fn test_enum_id_pat_with_discriminant() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct DekuTest {
+        inner: TestEnum,
+        #[deku(bits = 5)]
+        rest: u8,
+    }
+
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    #[deku(id_type = "u8", bits = "3")]
+    #[repr(u8)]
+    enum TestEnum {
+        VarA = 0,
+        VarB,
+        #[deku(id_pat = "_")]
+        VarC,
+    }
+
+    let input = &[0b001_00101];
+    let ret_read = DekuTest::try_from(input.as_slice()).unwrap();
+    assert_eq!(
+        DekuTest {
+            inner: TestEnum::VarB,
+            rest: 0b00101
+        },
+        ret_read
+    );
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!(input.to_vec(), ret_write);
+
+    let input = &[0b101_00101];
+    let ret_read = DekuTest::try_from(input.as_slice()).unwrap();
+    assert_eq!(
+        DekuTest {
+            inner: TestEnum::VarC,
+            rest: 0b00101
+        },
+        ret_read
+    );
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!(vec![0b010_00101], ret_write);
+}
+
+#[cfg(feature = "bits")]
+#[test]
+fn test_enum_id_pat_with_discriminant_and_storage() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct DekuTest {
+        inner: TestEnumStorage,
+        #[deku(bits = 5)]
+        rest: u8,
+    }
+
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    #[deku(id_type = "u8", bits = "3")]
+    #[repr(u8)]
+    enum TestEnumStorage {
+        VarA = 0,
+        VarB,
+        #[deku(id_pat = "_")]
+        VarC(u8),
+    }
+
+    let input = &[0b001_00101];
+    let ret_read = DekuTest::try_from(input.as_slice()).unwrap();
+    assert_eq!(
+        DekuTest {
+            inner: TestEnumStorage::VarB,
+            rest: 0b00101
+        },
+        ret_read
+    );
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!(input.to_vec(), ret_write);
+
+    let input = &[0b101_00101];
+    let ret_read = DekuTest::try_from(input.as_slice()).unwrap();
+    assert_eq!(
+        DekuTest {
+            inner: TestEnumStorage::VarC(0b101),
+            rest: 0b00101
+        },
+        ret_read
+    );
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!(input.to_vec(), ret_write);
+}
+
 #[test]
 fn test_id_pat_with_id() {
     #[derive(PartialEq, Debug, DekuRead, DekuWrite)]