optional: allow duplicate if statements

[PetervdPerk-NXP]

I've got the following usecase requiring me to use if twice

     tx_timestamp_length : 1,
     tx_timestamp_40: 40 if tx_timestamp_length = 0,
     tx_timestamp_64: 64 if tx_timestamp_length = 1,

Right now check_optional_fields disallows this while it's working perfectly fine.
This PR removes the check and allows the use of an if condition.

[hchataing]

Hi ! Thank you for your help :)
Removing this check means that you now have to handle the case where fields using the same flag are set inconsistently in a packet before serialization. Here is an example:

packet Test {
  cond : 1,
  _reserved_ : 8,
  a : 8 if cond = 0,
  b : 8 if cond = 1,
}

and in the code :

let test_ok_1 = Test { a : Some(42), b : None };
let test_ok_2 = Test { a : None, b : Some(42) };
let test_nok_1 = Test { a : None, b : None };
let test_nok_2 = Test { a : Some(42), b : Some(42) };

When converting into bytes, test_nok_1, test_nok_2 must fail as there is not a unique way to determine the flag value.
Considering this preventing reuse of the flag was the simplest solution, until a use case for it was found.

[PetervdPerk-NXP]

Wouldn't you be able to check in serialization that you would be not setting the flag or overwriting the flag and thus violating serialization.

let test_ok_1 = Test { a : Some(42), b : None };
# cond gets set 0
let test_ok_2 = Test { a : None, b : Some(42) };
# cond gets set 1
let test_nok_1 = Test { a : None, b : None };
# cond is not set -> violation
let test_nok_2 = Test { a : Some(42), b : Some(42) };
# cond is set twice -> violation

[hchataing]

That's definitely possible, but currently unimplemented.
We need to make this change at the same time the check is removed.

[PetervdPerk-NXP]

I've added the check as suggested, feel free to give some recommendations. I'm quite new to Rust.

The following code will be generated if there are 2 conditions.

        let cond_check = 0;
        cond_check += self.tx_timestamp_40.is_some();
        cond_check += self.tx_timestamp_64.is_some();
        if cond_check == 0 || cond_check == 2 {
            return Err(EncodeError::ConditionNotMet {
                packet: &self.packet_name,
                field: self.tx_timestamp_length(),
                value: cond_check as usize,
            });
        }

[hchataing]

pdl-tests is a good place to add tests for this new semantic: let's say in a file named semantic.rs (currently does not exist).

#[pdl_inline(
    r#"
little_endian_packets

enum Enum16 : 16 {
   X = 0x1234,
   Y = 0x5678,
}

packet Test {
  cond : 1,
  _reserved_ : 8,
  a : 8 if cond = 1,
  b : Enum16 if cond = 0,
}
"#
)]
#[cfg(test)]
mod optional_field {
    #[test]
    fn test_value_0() {
    }

    #[test]
    fn test_value_1() {
    }

    #[test]
    fn test_value_inconsistent() {
    }
}

[PetervdPerk-NXP]

Hi @hchataing thanks for all the suggestions, I've applied them and cleaned the code a bit.

However I'm stuck on the semantic.rs test case.
I'm getting the following error

error[E0599]: no method named `cond` found for reference `&Test` in the current scope
  --> pdl-tests/tests/semantic.rs:17:1
   |
17 |   #[pdl_inline(
   |  _^
18 | |     r#"
19 | | little_endian_packets
...  |
32 | | "#
33 | | )]
   | |__^ method not found in `&Test`
   |
   = note: this error originates in the attribute macro `pdl_inline` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0599, E0609.
For more information about an error, try `rustc --explain E0599`.
error: could not compile `pdl-tests` (test "semantic") due to 3 previous errors

Do you know to get a more verbose error from this?

[PetervdPerk-NXP]

@hchataing I've got the tests working now, could you do another review pass?

[hchataing]

Just a couple of nitpicks in the test, but happy with the result :)
Thanks for your patience !

[hchataing]

Can you please squash the two commits before merging ?
I could do it but I am afraid it would change the author.

[PetervdPerk-NXP]

Can you please squash the two commits before merging ? I could do it but I am afraid it would change the author.

No problem, just squashed the commits and pushed again.

[hchataing]

Great, I will push a new version to crates.io as soon as it is merged.
Thank you for your help :)