modular-bitfield
modular-bitfield copied to clipboard
Robbepop
`#[non_exhaustive]` enum support
I recently hit a bug in packet parsing where I had a non-exhaustive enum, representing an Organization Identifier, which is represented using a non-exhaustive enum. This makes sense: its intractable to keep a full list of all possible Organizations, but the ones I care about I want to have enumerated.
However, this panics.
#[derive(BitfieldSpecifier)]
#[non_exhaustive]
#[bits = 16]
pub enum Ouid {
SomeOrg = 0x0001,
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn supports_non_exhaustive() {
Ouid::from_bytes(0).unwrap();
}
}
I think the correct behavior for non_exhaustive tagged Specifier is to not check the fields
Does the example code have an error? Of course it panics if you unwrap and don't have a variant for 0. I tried this with 1 and it works without panic.
@hecatia-elegua
Of course it panics if you unwrap and don't have a variant for 0
The example is with an non-exhaustive enum. I had hoped that, as it has both a defined representation in bits, and is marked non_exhaustive, i might be able to convert where i don't know what other states might exist in future. That is after all what non_exhaustive forces downstream crates to do with this enum
Well you can do that, right? It does return a Result, you just can't unwrap it. What should unwrapping return here, otherwise?
My view is that the conversion from u16 to Ouid in the above example should be infallible, as we're modelling an Ouid which is explicitly marked as non-exhaustive.
As it is, this would be completely fine if the BitSpecifier were only ever consumed in a downstream crate - the non-exhaustive attribute already ensures that code contains a default case when pattern matching the enum. Making it safe when the enum is consumed within the same crate as the bitfield specifier is defined is harder, because the non-exhaustive attribute doesn't enforce the default case, and I'm unsure if there is a mechanism to do so.
match ouid {
Ouid::SomeOrg => /* handle SomeOrg */
_ => /* handle non-defined Ouid */
}
This works, since ouid is already type Ouid.
from_bytes would need to match all unnamed variant-ranges to something, so I still don't know how that would work without panic and without being fallible. Well, we could define some kind of "catch-all" variant, but then we don't really need non_exhaustive. We can't parse u16 as "Nothing", or I'm completely misunderstanding something here.
One idea I had was to use some kind of conditional compilation to define a variant only on the defining crate, which is hidden for downstream crates. Then we could parse into such a variant and downstream crates could not match on it, only match on _.
But afaik that's not possible since the downstream crate can't specify it uses the "no-special-variant" version while the crate it depends on uses the "special-variant" version.
Looking at this more, I don't think there is any way, short of compiling to a seperate crate that gives the framework required to do this safely. I'm not that up on macro magic, but I doubt is sufficiently powerful to generate a new dependency crate at compile time?
from_byteswould need to match all unnamed variant-ranges to something
It could transmute. In this case, where the memory representation is known, and all variants are explicitly given, then I believe this would be safe.
The problem remains that we cannot enforce non-exhaustive matching within the defining crate
Actually, it won't transmute.
If we call Ouid::from_bytes(0) and implement it somewhat like this:
match value {
1 => Ouid::SomeOrg
_ => core::mem::transmute(value)
}
it will just return Ouid::SomeOrg.