wrenger / bitfield-struct-rs Goto Github PK

This struct produces a clippy error:

#[bitfield(u16)]
/// X axis output value
pub struct OutX{
#[bits(16)]
val: i16,
}

error: &-masking with zero
--> src/lib.rs:564:1
|
564 | #[bitfield(u16)]
| ^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
= note: #[deny(clippy::bad_bit_mask)] on by default
= note: this error originates in the attribute macro bitfield (in Nightly builds, run with -Z macro-backtrace for more info)

I have debugged it and tracked it down to a debug assert, and propose this change that captures the same logic without causing errors in clippy:

index d0e0c39..669947b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -727,7 +727,7 @@ fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::R
// Bounds check and remove leading ones from negative values
ret.into = quote! {{
#[allow(unused_comparisons)]

•            debug_assert!(if this >= 0 { this & !#mask == 0 } else { !this & !#mask == 0 }, "value out of bounds");
  

•            debug_assert!(if this >= 0 { this | #mask == #mask } else { !(this | #mask) == 0 }, "value out of bounds");
             (this & #mask) as _
         }};
     }
  

The docs state that "The first field occupies the least significant bits".

I have the following snippet from a packet I am trying to parse;

// 0101 .... = length: 5
// .... 1... = a: true
// .... .0.. = b: false
// .... ..01 = flags: 1

Intuitively I would write the bitfield struct in the following order;

    #[bits(4)]
    pub len: u8,
    #[bits(1)]
    pub a: bool,
    #[bits(1)]
    pub b: bool,
    #[bits(2)]
    pub flags: u8,

But that is incorrect as the first field in my struct occupies the most significant bits.

Bellow is an example from the bitfield crate which seems to use the MSB first;

bitfield!{
    struct IpV4Header(MSB0 [u8]);
    u32;
    get_version, _: 3, 0;
    get_ihl, _: 7, 4;
    get_dscp, _: 13, 8;
    get_ecn, _: 15, 14;
    get_total_length, _: 31, 16;
    get_identification, _: 47, 31;
    get_df, _: 49;
    get_mf, _: 50;
    get_fragment_offset, _: 63, 51;
    get_time_to_live, _: 71, 64;
    get_protocol, _: 79, 72;
    get_header_checksum, _: 95, 79;
    get_source_address, _: 127, 96;
    get_destination_address, _: 159, 128;
}

Would it be possible/practical to implement in this crate?

I first want to say, I think this crate is great!

I am currently using it for OS development. When writing drivers for some x86 devices, the memory mapped registers used to control the devices sometimes contain read only bits. When programming the device, the register must be read in, other bits may be modified and then the register is written back (as not to modify the read only bits).

It would be useful to me, to be able to mark some members as read only (for example through [bits(2, read_only = true)], where read_only would be false by default for backwards compatibility.

I think I could implement that myself (add a field to Member and modify impl ToToken for Member I guess) but wanted to ask first

The following test fails when added to /tests/test.rs:

#[test]
fn positive_signed() {
    #[bitfield(u32)]
    struct MyBitfield {
        negative: i32,
    }

    let v1 = MyBitfield::new().with_negative(-3);
    assert_eq!(v1.negative(), -3);

    let v2 = MyBitfield::new().with_negative(0); // <-- panics here
    assert_eq!(v2.negative(), 0);

    let v3 = MyBitfield::new().with_negative(3); // <-- here as well
    assert_eq!(v3.negative(), 3);
}

with:

---- positive_signed stdout ----
thread 'positive_signed' panicked at 'assertion failed: value <= 0xffffffff', tests/test.rs:125:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The problem is that the condition of the debug_assert here

does not work for signed integers. The mask will be a hexadecimal number which rust interprets "literally" (ie the condition is value< = 0xfffffff which Rust sees as value <= -1)

The generated "overflowing field" checks are using std::mem::size_of instead of core::mem::size_of, which precludes declaring bitfields in no_std crates.

Should be an easy fix.

Hello,

I'm getting a compilation error using u16 as the underlying type:

error[E0277]: the trait bound `u8: From<u16>` is not satisfied
   --> bmapi-sys/src/dpcdecl.rs:298:1
    |
298 | #[bitfield(u16)]
    | ^^^^^^^^^^^^^^^^ the trait `From<u16>` is not implemented for `u8`
    |
    = help: the following other types implement trait `From<T>`:
              <u8 as From<NonZeroU8>>
              <u8 as From<bool>>
    = note: required for `u16` to implement `Into<u8>`
    = note: this error originates in the attribute macro `bitfield` (in Nightly builds, run with -Z macro-backtrace for more info)

The struct definition is:

#[bitfield(u16)]
pub struct BMDataHeader {
    #[bits(4)]
    pub kind: u8,
    #[bits(4)]
    pub flags: u8,
    #[bits(4)]
    pub dchn: u8,
    #[bits(4)]
    pub schn: u8
}

How can I fix it?

#[bitfield[u8]]
pub struct PMPcfgIn {
    pub r: bool,
    pub w: bool,
    pub x: bool,
    #[bits(2)]
    pub a: u8,
    #[bits(2)]
    _pad: u8,
    pub l: bool,
}

#[bitfield[u64]]
pub struct PMPcfg {
    pub pmp0cfg: PMPcfgIn, // err, unsupported type
    pub pmp1cfg: u8,
    pub pmp2cfg: u8,
    pub pmp3cfg: u8,
    pub pmp4cfg: u8,
    pub pmp5cfg: u8,
    pub pmp6cfg: u8,
    pub pmp7cfg: u8,
}

I just want to replace u8 with PMPcfgIn in the PMPcfg structure, is there any way to do that?

This seems like an oversight...

First off: thanks for this crate!
The syntax is clean and intuitive, and is a lot easier to use than the other bitfield crates i've seen on crates.io.

Right now, the generated bitfield can only be backed by various integer types, which has 3 pretty major limitations:

1. It limits the size of the bitfield to at most sizeof(u128)
2. It forces the addition of additional padding bytes into types that don't necessarily need them (e.g: a bitfield composed of 3xu8 fields)
3. It enforces alignment on the type, making it difficult to naturally use as part of repr(packed) fields (or repr(C) fields + something like the zerocopy crate)

A clean solution to overcoming all 3 of these limitations would be to support backing the generated bitfield using a [u8; N] array.

Happy to brainstorm specifics of syntax / featureset, but I was thinking it could look something like:

#[bitfield] // no explicit backing type specified
struct Foo {
    foo: u64,
    bar: u64,
    #[bits(2)]
    extra: u8,
    #[bits(6)]
    _reserved: u8 // must include explicit "padding" bits
}

static_assert_eq!(std::mem::size_of::<Foo>(), 8 + 8 + 1);

// no `From`/`Into` for any specific backing integer, but instead have a From/Into `[u8; N]`

Cheers!

The tests negative_pos_overflow and negative_neg_overflow are failing in release mode, while passing otherwise.

$ cargo test -r
[...]
---- negative_pos_overflow stdout ----
note: test did not panic as expected
---- negative_neg_overflow stdout ----
note: test did not panic as expected

failures:
    negative_neg_overflow
    negative_pos_overflow

test result: FAILED. 13 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--test test`

$ cargo test
[...]
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.18s

Version 0.6, commit 1f8620e

Toolchains tried:

• nightly-x86_64-unknown-linux-gnu, rustc 1.79.0-nightly (a07f3eb43 2024-04-11)
• stable-x86_64-unknown-linux-gnu, rustc 1.77.2 (25ef9e3d8 2024-04-09)

Adding pub before a field makes both the getter and the setter public. Is there a way to only make the getter public and keep the setter private?

Currently arrays are not supported as a field type:

#[bitfield(u128, order = Msb)]
struct LmsState {
    history: [i16; 4],
    weights: [i16; 4]
}

This fails, complaining that an explicit bit size is needed. When one is specified, the error becomes missing angle brackets in associated item path. The expansion contains [i16; 4]::from_bits(this), which should be <[i16; 4]>::from_bits(this). A workaround is explicitly specifying the conversion, like:

#[bitfield(u128, order = Msb)]
struct LmsState {
    #[bits(64, from = unpack, into = pack)]
    history: [i16; 4],
    #[bits(64, from = unpack, into = pack)]
    weights: [i16; 4]
}

fn unpack(mut v: u128) -> [i16; 4] {
    let mut r = [0; 4];
    for i in 0..4 {
        r[i] = (v >> 48) as i16;
        v <<= 16;
    }
    r
}

fn pack(v: [i16; 4]) -> u128 {
    v.into_iter().fold(0, |r, v|
        r << 16 | v as u128
    )
}

Also, to specify the bit width of an array, it might help to add a way to specify the width of its elements, like:

#[bitfield(u128, order = Msb)]
struct LmsState {
    #[array_bits(16)]
    history: [i16; 4],
    #[array_bits(16)]
    weights: [i16; 4]
}

Right now, custom types can only be supported by implementing into_bits and from_bits or From and Into. Since we can't implement methods for an external type, it has to be wrapped as another type to be used in a bitfield struct. For example:

enum CustomEnum {
    A = 0,
    B = 1,
    C = 2,
}

struct OptionalCustomEnum(Option<CustomEnum>);

impl OptionalCustomEnum {
    const fn into_bits(self) -> u32 {
        todo!()
    }

    const fn from_bits(value: u32) -> Self {
        todo!()
    }
}

#[bitfield(u32)]
struct MyBitfield {
    #[bits(16)]
    custom: CustomEnum,

    #[bits(16)]
    __: u32,
}

This makes the operations inconvenient:

let mut val = MyBitfield::new().with_custom(OptionalCustomEnum(Some(CustomEnum::A)));
val.set_custom(OptionalCustomEnum(Some(CustomEnum::A)));
let custom = val.custom().0;

// or impl From<Option<CustomEnum>> for OptionalCustomEnum and:
let mut val = MyBitfield::new().with_custom(Some(CustomEnum::A).into());
val.set_custom(Some(CustomEnum::A).into());
let custom = val.custom().0;

If the conversion can be specified through converters instead of traits, for example:

enum CustomEnum {
    A = 0,
    B = 1,
    C = 2,
}

struct CustomEnumConverter;

impl CustomEnumConverter {
    const fn into_bits(value: Option<CustomEnum>) -> u32 {
        todo!()
    }

    const fn from_bits(value: u32) -> Option<CustomEnum> {
        todo!()
    }
}

#[bitfield(u32)]
struct MyBitfield {
    #[bits(16, converter = CustomEnumConverter)]
    custom: Option<CustomEnum>,

    #[bits(16)]
    __: u32,
}

The operations can be simpler and more intuitive:

let mut val = MyBitfield::new().with_custom(Some(CustomEnum::A));
val.set_custom(Some(CustomEnum::A));
let custom = val.custom();

I'm very happy that Debug is now generated, but this is a breaking change--I have types where I have manually implemented Debug that now fail to compile. The semver version should have changed to 0.2.0.

Separately, it would be nice to have an opt-out on a per-type basis, maybe via an attribute. Sometimes I want a custom Debug impl.

Thanks for this cool macro!

This code is panicking with value out of bounds:

#[test]
fn negative_signed() {
    #[bitfield(u64)]
    struct MyBitfield {
        negative: i32,
        #[bits(32)]
        __: u32,
    }

    let _ = MyBitfield::new().with_negative(-1);
}

When I change it to

#[test]
fn negative_signed() {
    #[bitfield(u32)]
    struct MyBitfield {
        negative: i32,
    }

    let f = MyBitfield::new().with_negative(-1);
    assert_eq!(-1, f.negative())
}

then it's working fine.

Im not sure how nested fields should work, is there support for something like this ?

#[bitfield(u8)]
pub struct AccessByte {
    #[bits(1)]
    access: bool,
    
    #[bits(1)]
    rw: bool,

    #[bits(1)]
    direction: bool,

    #[bits(1)]
    executable: bool,

    #[bits(1)]
    descriptor_type: bool,

    #[bits(2)]
    privelege_level: u8,

    #[bits(1)]
    present: bool
}


impl AccessByte {
    pub const fn from_bits(value: u8) -> Self {
        AccessByte(value)
    }

    pub const fn into_bits(self) -> u8 {
        self.0 as u8
    }
}




#[bitfield(u64)]
pub struct GdtEntry {

    #[bits(16)]
    limit: u16,  

    #[bits(24)]
    base: u32,

    #[bits(8)]
    access_byte: AccessByte,

    #[bits(4)]
    limit2: u8,

    #[bits(4)]
    flags: u8,

    #[bits(8)]
    base2: u8

}```

syn recently released version 2.

I'm just auditing all the deps in my Cargo.lock for syn 1, opening issues on projects that haven't switched over yet.

Any chance you could switch over? The quicker everyone moves to version 2, the sooner projects won't have to build both syn 1 and 2 at the same time :)

When #![warn(missing_docs)] is used, bitfield-struct's new implementation doesn't contain a doc comment which results in the lint firing. For example, the following code causes the lint to fire:

#![warn(missing_docs)]

//! My library.

use bitfield_struct::bitfield;

#[bitfield(u64)]
#[derive(Default)]
pub struct MyBitfield {
    /// Look, a field.
    #[bits(1)]
    pub field_a: bool,
    /// Some field.
    #[bits(1)]
    pub field_b: bool,
    /// Another field.
    #[bits(1)]
    pub field_c: bool,
    /// Currently reserved.
    #[bits(61)]
    pub reserved: u64,
}

Looking at rust-analyzer's recursive expansion, it seems like the new function needs a doc comment for the lint not to fire?

// Recursive expansion of bitfield macro
// ======================================

#[derive(Default, Copy, Clone)]
#[repr(transparent)]
pub struct MyBitfield(u64);

impl MyBitfield {
    pub const fn new() -> Self {
        Self(0)
    }
    const FIELD_A_BITS: usize = 1usize;
    const FIELD_A_OFFSET: usize = 0usize;
    #[doc = " Look, a field."]
    #[doc = "\n\nBits: 0..1"]
    pub fn set_field_a(&mut self, value: bool) {
        *self = self.with_field_a(value);
    }
    #[doc = " Look, a field."]
    #[doc = "\n\nBits: 0..1"]
    pub const fn with_field_a(self, value: bool) -> Self {
        Self(self.0 & !(1 << 0usize) | (value as u64) << 0usize)
    }
    #[doc = " Look, a field."]
    #[doc = "\n\nBits: 0..1"]
    pub const fn field_a(&self) -> bool {
        ((self.0 >> 0usize) & 1) != 0
    }
    const FIELD_B_BITS: usize = 1usize;
    const FIELD_B_OFFSET: usize = 1usize;
    #[doc = " Some field."]
    #[doc = "\n\nBits: 1..2"]
    pub fn set_field_b(&mut self, value: bool) {
        *self = self.with_field_b(value);
    }
    #[doc = " Some field."]
    #[doc = "\n\nBits: 1..2"]
    pub const fn with_field_b(self, value: bool) -> Self {
        Self(self.0 & !(1 << 1usize) | (value as u64) << 1usize)
    }
    #[doc = " Some field."]
    #[doc = "\n\nBits: 1..2"]
    pub const fn field_b(&self) -> bool {
        ((self.0 >> 1usize) & 1) != 0
    }
    const FIELD_C_BITS: usize = 1usize;
    const FIELD_C_OFFSET: usize = 2usize;
    #[doc = " Another field."]
    #[doc = "\n\nBits: 2..3"]
    pub fn set_field_c(&mut self, value: bool) {
        *self = self.with_field_c(value);
    }
    #[doc = " Another field."]
    #[doc = "\n\nBits: 2..3"]
    pub const fn with_field_c(self, value: bool) -> Self {
        Self(self.0 & !(1 << 2usize) | (value as u64) << 2usize)
    }
    #[doc = " Another field."]
    #[doc = "\n\nBits: 2..3"]
    pub const fn field_c(&self) -> bool {
        ((self.0 >> 2usize) & 1) != 0
    }
    const RESERVED_BITS: usize = 61usize;
    const RESERVED_OFFSET: usize = 3usize;
    #[doc = " Currently reserved."]
    #[doc = "\n\nBits: 3..64"]
    pub fn set_reserved(&mut self, value: u64) {
        *self = self.with_reserved(value);
    }
    #[doc = " Currently reserved."]
    #[doc = "\n\nBits: 3..64"]
    pub const fn with_reserved(self, value: u64) -> Self {
        #[allow(unused_comparisons)]
        debug_assert!(if value> = 0 {
      value& !0x1fffffffffffffff =  = 0
    }else {
      !value& !0x1fffffffffffffff =  = 0
    },"value out of bounds");
        Self(
            self.0 & !(0x1fffffffffffffff << 3usize)
                | (value as u64 & 0x1fffffffffffffff) << 3usize,
        )
    }
    #[doc = " Currently reserved."]
    #[doc = "\n\nBits: 3..64"]
    pub const fn reserved(&self) -> u64 {
        let shift = u64::BITS - 61u32;
        (((self.0 >> 3usize) as u64) << shift) >> shift
    }
}
impl From<u64> for MyBitfield {
    fn from(v: u64) -> Self {
        Self(v)
    }
}
impl From<MyBitfield> for u64 {
    fn from(v: MyBitfield) -> u64 {
        v.0
    }
}
impl core::fmt::Debug for MyBitfield {
    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
        f.debug_struct("MyBitfield")
            .field("field_a", &self.field_a())
            .field("field_b", &self.field_b())
            .field("field_c", &self.field_c())
            .field("reserved", &self.reserved())
            .finish()
    }
}

I've worked around this by putting the structure inside it's own module where the lint is ignored, then pub re-exporting the bitfield, but it would be nice to not to have to do that workaround.

It looks like raw identifiers are currently not working.

I tried r#type as a field name, which panicked with

custom attribute panicked message: `"with_r#type"` is not a valid identifier

The following sample

use std::marker::PhantomData;

#[bitfield(u32)]
pub struct Foo<T> {
    _unused: u32,

    _marker: PhantomData<T>,
}

Fails to compile with the following error:

error: unsupported type
  --> main.rs:9:14
  |
9 |     _marker: PhantomData<T>,
  |              ^^^^^^^^^^^^^^

While some types should indeed be unsupported, markers probably should be supported, maybe even all zero size types? Not sure how one would go about fixing this other than potentially just extending type_bits to support marker types, but this disqualifies user defined markers, so doesn't seem like the best solution.

Hi,

Thanks for the work on this crate its been very helpful. I was wondering if you thought it would be worth adding an option for the padding to specify pad with 0, or with 1. A coms protocol I am currently looking at pads with ones in some sections, I've generally got around this by specifying some extra enum type with a default value and implementing the From<> traits to make it work. Potentially a match on _0* and _1* which would not break the current implementation.

Hi, I wanted to ask, why the functions mentioned in the title aren't automatically generated for structs generated with bitfield.

I have the following bits I need to parse;

0000 10.. .... .... = a: 2
.... ..00 0000 0000 = b: 0

I would expect the following to work:

#[bitfield(u16)]
pub struct Hdr {
    #[bits(10)]
    pub b: u16,
    #[bits(6)]
    pub a: u16,
}

assert_eq!(hdr.b(), 0); results in

assertion failed: `(left == right)`
  left: `8`,
 right: `0`

assert_eq!(hdr.a(), 2); results in

assertion failed: `(left == right)`
  left: `0`,
 right: `2`

I can however get the correct result for a if I split up b as follows;

#[bitfield(u16)]
pub struct Hdr {
    #[bits(2)]
    pub b1: u16,
    #[bits(6)]
    pub a: u16,
    #[bits(8)]
    pub b2: u16,
}

That doesn't feel right...
What am I doing wrong?

Hi! I started using your crate on embedded and really like it.
The only think I am missing is enum support, I use it to write structs for spi/i2c devices with often pack some modes settings into they registers.
Some examples:

#[bitfield(u8)]
pub struct PowerCtl {
    #[bits(2)]
    mesure_mode: u8,
    autosleep: bool,
    wakeup: bool,
    #[bits(2)]
    low_noise_mode: u8,
    ext_clk: bool,
    #[bits(1)]
    _unused: u8,
}

#[repr(u8)]
pub enum PowerCtlMesureMode {
    Standby = 0b00,
    Measurement = 0b10,
}

#[repr(u8)]
pub enum PowerCtlLowNoiseMode {
    Normal = 0b00,
    Low = 0b01,
    Ultralow = 0b10,
}

It would be nice if that could become this:

#[bitfield(u8)]
pub struct PowerCtl {
    #[bits(2)]
    mesure_mode: PowerCtlMesureMode,
    autosleep: bool,
    wakeup: bool,
    #[bits(2)]
    low_noise_mode: PowerCtlLowNoiseMode,
    ext_clk: bool,
    #[bits(1)]
    _unused: u8,
}

And in this case when reading a enum value from the struct it should return Option<Enum> so when the registers has a value with isn't allowed by the enum it returns None.
I would love to see that!

Hi Lars, thanks for the great library!

I'd like to define types from virtio with defined endianness such as:

le32 { 
  vqn : 16; 
  next_off : 15; 
  next_wrap : 1; 
};

See Virtual I/O Device (VIRTIO) Version 1.2 — 1.4 Structure Specifications for details on the notation.

It would be great if this crate allowed to change the storage type of the struct (independently of the number type used for internal bitshift operations).

I'd envision the macro to work similar to this:

#[bitfield(u32, inner = le32, from = le32::from_ne, into = le32::to_ne)]
pub struct NotificationData {
    pub vqn: u16,
    #[bits(15)]
    pub next_off: u16,
    pub next_wrap: bool,
}

le32 is from the endian-num crate.

The following sample:

use bitfield_struct::bitfield;

#[bitfield(u64)]
struct PageTableEntry {
    /// defaults to 32 bits for u32
    addr: u32,

    /// public field -> public accessor functions
    #[bits(12)]
    pub size: usize,

    /// padding: No accessor functions are generated for fields beginning with `_`.
    #[bits(5)]
    _p: u8,

    #[bits(1)]
    bug: u8,

    /// interpreted as 1 bit flag
    present: bool,

    /// sign extend for signed integers
    #[bits(13)]
    negative: i16,
}

Will fail with:

  --> src/main.rs:3:1
   |
3  | #[bitfield(u64)]
   | ^^^^^^^^^^^^^^^^ expected `u8`, found `bool`
...
17 |     bug: u8,
   |          -- expected `u8` because of return type
   |
   = note: this error originates in the attribute macro `bitfield` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0308`.

The error message when the size of a struct is wrong misspells 'bitfield' as 'bitfiled'. Not an urgent issue but should be an easy fix.

The code for this error message is here

Thanks! This crate is a lifesaver for doing OS development in rust.