Comply to `cond` attribute while encoding

[fbergero]

Should cond attribute make it so deku does not encode conditional fields even if they are Some(_)?

Specifically, shouldn't field_b be excluded from encoding here:

TestStruct {
            field_a: 0x02,
            field_b: Some(0x02),
        },

from https://github.com/sharksforarms/deku/blob/master/tests/test_attributes/test_cond.rs#L33.

Perhaps we need a cond_write attribute?

[sharksforarms]

take a look at using skip + cond: https://docs.rs/deku/latest/deku/attributes/index.html#skip

[fbergero]

Yeah, what I'm trying to achieve is the following:

    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
    struct TestStruct {
        field_a: u8,
        #[deku(cond = "*field_a == 0x01", assert = "field_b.is_some() || *field_a != 0x01")]
        field_b: Option<u8>,
    }

    let ret_read = TestStruct {
            field_a: 0x01,
            field_b: None,
    };

    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
    assert_eq!(vec![0x1], ret_write);

to ensure that if field_a == 0x01 then, field_b has to be present. This passes without the assert and fails with it.

This one using skip also passes, which I think it shouldn't?

 #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
    struct TestStruct {
        field_a: u8,
        #[deku(cond = "*field_a != 0x01", skip)]
        field_b: Option<u8>,
    }

    let ret_read = TestStruct {
            field_a: 0x01,
            field_b: None,
    };

    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
    assert_eq!(vec![0x1], ret_write);

All that said, would the assert be the right way to express my intention here?

[sharksforarms]

Both seem to accomplish what you're looking for: if field_a == 0x01, field_b must be Some

I created the example below based on what you provided, it helps when a full example is provided with asserts to show intent, much easier to help out.

Feel free to modify the example below to express what you're looking for, the following passes:

use deku::prelude::*;
use std::convert::TryFrom;

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStructAssert {
    field_a: u8,
    #[deku(
        cond = "*field_a == 0x01",
        // assert doesn't do anything here, because we will only read if `cond` returns true
        // which is ONLY when field_a == 0x01, so field_b.is_some() is always true
        assert = "field_b.is_some() || *field_a != 0x01"
    )]
    field_b: Option<u8>,
}

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStructSkip {
    field_a: u8,
    #[deku(cond = "*field_a != 0x01", skip)]
    field_b: Option<u8>,
}

fn main() {
    {
        let test_data: &[u8] = &[0x03];
        let ret_read = TestStructAssert::try_from(test_data).unwrap();

        assert_eq!(
            ret_read,
            TestStructAssert {
                field_a: 0x03, // field_a is not 0x01
                field_b: None, // so field_b is not Some
            }
        );

        let ret_write: Vec<u8> = ret_read.try_into().unwrap();
        assert_eq!(vec![0x03], ret_write);
    }

    {
        let test_data: &[u8] = &[0x01, 0x02];
        let ret_read = TestStructAssert::try_from(test_data).unwrap();

        assert_eq!(
            ret_read,
            TestStructAssert {
                field_a: 0x01,       // field_a is 0x01
                field_b: Some(0x02), // so field_b is Some
            }
        );

        let ret_write: Vec<u8> = ret_read.try_into().unwrap();
        assert_eq!(vec![0x01, 0x02], ret_write);
    }

    {
        let test_data: &[u8] = &[0x03];
        let ret_read = TestStructSkip::try_from(test_data).unwrap();

        assert_eq!(
            ret_read,
            TestStructSkip {
                field_a: 0x03, // field_a is not 0x01
                field_b: None, // so field_b is not Some
            }
        );

        let ret_write: Vec<u8> = ret_read.try_into().unwrap();
        assert_eq!(vec![0x03], ret_write);
    }

    {
        let test_data: &[u8] = &[0x01, 0x02];
        let ret_read = TestStructSkip::try_from(test_data).unwrap();

        assert_eq!(
            ret_read,
            TestStructSkip {
                field_a: 0x01,       // field_a is 0x01
                field_b: Some(0x02), // so field_b is Some
            }
        );

        let ret_write: Vec<u8> = ret_read.try_into().unwrap();
        assert_eq!(vec![0x01, 0x02], ret_write);
    }
}

[fbergero]

If you try this example you'll notice that the one with skip encodes the struct without errors but the other form does fail (since field_b is None)

use deku::prelude::*;
use std::convert::TryFrom;

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStructAssert {
    field_a: u8,
    #[deku(
        cond = "*field_a == 0x01",
        // assert doesn't do anything here, because we will only read if `cond` returns true
        // which is ONLY when field_a == 0x01, so field_b.is_some() is always true
        assert = "field_b.is_some() || *field_a != 0x01"
    )]
    field_b: Option<u8>,
}

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStructSkip {
    field_a: u8,
    #[deku(cond = "*field_a != 0x01", skip)]
    field_b: Option<u8>,
}

fn main() {

    {
        let test_data = TestStructSkip {
                field_a: 0x01, // field_a is not 0x01
                field_b: None, // so field_b is not Some
        };

        let ret_write: Vec<u8> = test_data.try_into().unwrap();
        assert_eq!(vec![0x01], ret_write);
    }

    {
        let test_data = 
            TestStructAssert {
                field_a: 0x01, // field_a is 0x01
                field_b: None, // so field_b is not Some
            };

        let ret_write: Vec<u8> = test_data.try_into().unwrap();
        assert_eq!(vec![0x01], ret_write);
    }
}

This is what I wanted deku to enforce.

[sharksforarms]

@fbergero ah I see, you want to enforce when writing. Yes, assert would be how you'd interrupt the reading/writing of bytes if a condition is not met. Does this satisfy your requirements? Does the skip case make sense to you on why it succeeds? Since it's an Option type, field_a == 0x01 and so the condition is false, so we write the option (which is None, so nothing to write)

[fbergero]

Yeah this satisfies my requirement. And also I do understand why the skip works as is. What is odd is that in this case the DekuRead would fail to parse the encoded version with skip.

use deku::prelude::*;
use std::convert::TryFrom;

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStructSkip {
    field_a: u8,
    #[deku(cond = "*field_a != 0x01", skip)]
    field_b: Option<u8>,
}

fn main() {

    {
        let test_data = TestStructSkip {
                field_a: 0x01, // field_a is not 0x01
                field_b: None, // so field_b is not Some
        };

        let ret_write: Vec<u8> = test_data.try_into().unwrap();
        TestStructSkip::try_from(&ret_write[..]).unwrap();
    }
}

In my case encoding that Struct without a field_b=None would make the binary representation invalid.

I'll close this anyhow. Thanks for your help.