Repository: sharksforarms/deku
Pull request #555 (2025-05-16 10:32:51)

fix: fix and test of conditional fields depending on conditional fields
I observed that it was not possible to introduce a skippable temporary field (skip+cond) depending on another temporary skippable field.

The problem was, that for skippable fields, the temporary variable in added within an `if`, but again used later for another field depending on the first skippable field (see unittest).

Solution: Moving the variable outside the `if` solves the problem (in the diff you see a "moved" `if` expression, which makes the variable `fspec2_6` accessible later in the code):
![image](https://github.com/user-attachments/assets/6c9621cf-5f2c-4c57-964c-97c2663838bb)


Please have a look. I gratefully appreciate any feedback to incorporate in this PR.

- cargo test seems to work fine.
- I added a test to illustrate the problem and to prove that it is fixed...


goto40: Note: I also changed one unrelated file (`noseek.rs`) in order to fix some clippy issue. I can undo this, if you prefer to clean this up separately...

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a90c971..bc711d8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## [Unreleased]
 
+## Fixes
+- fix and test of conditional temporary fields depending on conditional temporary fields (did not compile before). See [unittest](./tests/test_attributes/test_temp_value_with_cond.rs).
+
 ## [0.19.0] - 2025-02-07
 
 ## Features
diff --git a/deku-derive/src/macros/deku_write.rs b/deku-derive/src/macros/deku_write.rs
index 8a3ec67..3416537 100644
--- a/deku-derive/src/macros/deku_write.rs
+++ b/deku-derive/src/macros/deku_write.rs
@@ -719,12 +719,8 @@ fn emit_field_write(
         )?;
 
         if f.temp {
-            if let Some(temp_value) = &f.temp_value {
-                let field_type = &f.ty;
-                let internal_field_ident = gen_internal_field_ident(&field_ident);
+            if f.temp_value.is_some() {
                 quote! {
-                    let #internal_field_ident: #field_type = #temp_value;
-                    let #field_ident: &#field_type = &#internal_field_ident;
                     ::#crate_::DekuWriter::to_writer(#object_prefix &#field_ident, __deku_writer, (#write_args))
                 }
             } else {
@@ -768,10 +764,25 @@ fn emit_field_write(
         quote! {}
     };
 
+    let temp_decl = if f.temp {
+        if let Some(temp_value) = &f.temp_value {
+            let field_type = &f.ty;
+            let internal_field_ident = gen_internal_field_ident(&field_ident);
+            Some(quote! {
+                let #internal_field_ident: #field_type = #temp_value;
+                let #field_ident: &#field_type = &#internal_field_ident;
+            })
+        } else {
+            None // Note: temp w/o temp_value with not work
+        }
+    } else {
+        None
+    };
     let field_write_tokens = match (f.skip, &f.cond) {
         (true, Some(field_cond)) => {
             // #[deku(skip, cond = "...")] ==> `skip` if `cond`
             quote! {
+                #temp_decl
                 if (#field_cond) {
                     #skipping_log
                    // skipping, no write
@@ -789,6 +800,7 @@ fn emit_field_write(
         }
         (false, _) => {
             quote! {
+                #temp_decl
                 #field_write_normal
             }
         }
diff --git a/src/noseek.rs b/src/noseek.rs
index 7ead7eb..e892bf0 100644
--- a/src/noseek.rs
+++ b/src/noseek.rs
@@ -4,8 +4,6 @@
 use alloc::vec::Vec;
 
 use crate::no_std_io::{Read, Result, Seek, SeekFrom, Write};
-#[cfg(feature = "std")]
-use no_std_io::io::ErrorKind;
 
 /// A wrapper that provides a limited implementation of
 /// [`Seek`] for unseekable [`Read`] and [`Write`] streams.
@@ -50,10 +48,7 @@ impl<T> Seek for NoSeek<T> {
             SeekFrom::Current(0) => Ok(self.pos),
             // https://github.com/rust-lang/rust/issues/86442
             #[cfg(feature = "std")]
-            _ => Err(std::io::Error::new(
-                ErrorKind::Other,
-                "seek on unseekable file",
-            )),
+            _ => Err(std::io::Error::other("seek on unseekable file")),
             #[cfg(not(feature = "std"))]
             _ => panic!("seek on unseekable file"),
         }


Test patch:
diff --git a/tests/test_attributes/test_temp_value_with_cond.rs b/tests/test_attributes/test_temp_value_with_cond.rs
index 30abcb2..48e34be 100644
--- a/tests/test_attributes/test_temp_value_with_cond.rs
+++ b/tests/test_attributes/test_temp_value_with_cond.rs
@@ -9,12 +9,12 @@ fn test_temp_value_with_cond() {
         #[deku(bits = "5", temp, temp_value = "0")]
         spare1: u8,
         #[deku(bits = "1", temp, temp_value = "if payload1.is_some() {1} else {0}")]
-        fspec1_8: u8,
+        fspec1_6: u8,
         #[deku(bits = "1", temp, temp_value = "if payload2.is_some() {1} else {0}")]
         fspec1_7: u8,
         #[deku(bits = "1", temp, temp_value = "0")]
         fspec1_fx: u8,
-        #[deku(skip, cond = "*fspec1_8 != 0x01", default = "None")]
+        #[deku(skip, cond = "*fspec1_6 != 0x01", default = "None")]
         payload1: Option<u32>,
         #[deku(skip, cond = "*fspec1_7 != 0x01", default = "None")]
         payload2: Option<u32>,
@@ -61,3 +61,155 @@ fn test_temp_value_with_cond() {
         assert_eq!(d, d2);
     }
 }
+
+#[test]
+fn test_temp_value_with_cond_depending_on_skippable_entries() {
+    // This test defines two control bytes, to control
+    // if some payload is attached:
+    // - byte 0 (fspec1):
+    //   - bit 6+7 control the presence of payload1 & payload2
+    //   - bit 8 controls if fspec2 is present (else fspec2 is interpreted as 0)
+    // - optional byte 1 (fspec2):
+    //   - bit 6+7 control the presence of payload3 & payload4
+    // - payload1-payload4 (as Option)
+    //
+    // Notes:
+    // - fspec1 & fspec2 are not present in the public API (temp)
+    // - payload1 and payload2 depend on temp bits 6/7 of fspec1
+    // - payload2 and payload3 depend on skippable bit 6/7 of fspec2 (cond+skip)
+
+    #[deku_derive(DekuRead, DekuWrite)]
+    #[derive(Debug, PartialEq)]
+    #[deku(endian = "big")]
+    struct TestStruct {
+        // fspec1
+        // ------
+        #[deku(bits = "5", temp, temp_value = "0")]
+        spare1: u8,
+        #[deku(bits = "1", temp, temp_value = "if payload1.is_some() {1} else {0}")]
+        fspec1_6: u8,
+        #[deku(bits = "1", temp, temp_value = "if payload2.is_some() {1} else {0}")]
+        fspec1_7: u8,
+        #[deku(
+            bits = "1",
+            temp,
+            temp_value = "if payload3.is_some()||payload4.is_some() {1} else {0}"
+        )]
+        fspec1_fx: u8,
+
+        // fspec2
+        // ------
+        #[deku(
+            skip,
+            cond = "*fspec1_fx != 0x01",
+            default = "0",
+            bits = "5",
+            temp,
+            temp_value = "0"
+        )]
+        spare2: u8,
+        #[deku(
+            skip,
+            cond = "*fspec1_fx != 0x01",
+            default = "0",
+            bits = "1",
+            temp,
+            temp_value = "if payload3.is_some() {1} else {0}"
+        )]
+        fspec2_6: u8,
+        #[deku(
+            skip,
+            cond = "*fspec1_fx != 0x01",
+            default = "0",
+            bits = "1",
+            temp,
+            temp_value = "if payload4.is_some() {1} else {0}"
+        )]
+        fspec2_7: u8,
+        #[deku(
+            skip,
+            cond = "*fspec1_fx != 0x01",
+            default = "0",
+            bits = "1",
+            temp,
+            temp_value = "0"
+        )]
+        fspec2_fx: u8,
+
+        // payload
+        // -------
+        #[deku(skip, cond = "*fspec1_6 != 0x01", default = "None")]
+        payload1: Option<u32>,
+        #[deku(skip, cond = "*fspec1_7 != 0x01", default = "None")]
+        payload2: Option<u32>,
+
+        #[deku(skip, cond = "*fspec2_6 != 0x01", default = "None")]
+        payload3: Option<u32>,
+        #[deku(skip, cond = "*fspec2_7 != 0x01", default = "None")]
+        payload4: Option<u32>,
+    }
+
+    {
+        let d = TestStruct {
+            payload1: Some(1),
+            payload2: Some(2),
+            payload3: Some(3),
+            payload4: Some(4),
+        };
+        let bytes = d.to_bytes().unwrap();
+        assert_eq!(
+            bytes,
+            vec![7, 6, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 3, 0, 0, 0, 4]
+        );
+        let d2 = TestStruct::from_bytes((&bytes, 0)).unwrap().1;
+        assert_eq!(d, d2);
+    }
+    {
+        let d = TestStruct {
+            payload1: None,
+            payload2: Some(2),
+            payload3: None,
+            payload4: None,
+        };
+        let bytes = d.to_bytes().unwrap();
+        assert_eq!(bytes, vec![2, 0, 0, 0, 2]);
+        let d2 = TestStruct::from_bytes((&bytes, 0)).unwrap().1;
+        assert_eq!(d, d2);
+    }
+    {
+        let d = TestStruct {
+            payload1: None,
+            payload2: None,
+            payload3: None,
+            payload4: Some(4),
+        };
+        let bytes = d.to_bytes().unwrap();
+        assert_eq!(bytes, vec![1, 2, 0, 0, 0, 4]);
+        let d2 = TestStruct::from_bytes((&bytes, 0)).unwrap().1;
+        assert_eq!(d, d2);
+    }
+    {
+        let d = TestStruct {
+            payload1: None,
+            payload2: None,
+            payload3: Some(3),
+            payload4: None,
+        };
+        let bytes = d.to_bytes().unwrap();
+        assert_eq!(bytes, vec![1, 4, 0, 0, 0, 3]);
+        let d2 = TestStruct::from_bytes((&bytes, 0)).unwrap().1;
+        assert_eq!(d, d2);
+    }
+    {
+        let d = TestStruct {
+            payload1: None,
+            payload2: Some(2),
+            payload3: Some(3),
+            payload4: None,
+        };
+        let bytes = d.to_bytes().unwrap();
+        assert_eq!(bytes, vec![3, 4, 0, 0, 0, 2, 0, 0, 0, 3]);
+        let d2 = TestStruct::from_bytes((&bytes, 0)).unwrap().1;
+        assert_eq!(d, d2);
+    }
+}