Writing default values of fields

[SebastianHambura]

I wanted to be able to define default values at the field level, and not only at the struct level, so I started working on this feature.

#[bitfield(u32, introspect, forbid_overlaps, default = use_field_defaults)]
pub struct WithDefaultFieldValues {
    #[bits(0..=2, rw, default = 4)] // <- decimal ok
    val3: u3,
    //#[bits(2..=16, rw, default = 0xFFFF)] //<- Will raise an error
    #[bits(3..=16, rw, default = 0xBEF)] // Accepts 0x and 0h (as base16 indicator)
    val2: u14,
    #[bits(17..=23, rw, default = 0d42)] // Accepts 0d (as base10 indicator)
    val1: u7,
    #[bits(24..=31, rw, default = -1)] // Can parse negative values
    val0: i8,
}

Is this something that you/the community would find interesting ? If yes, I can try to continue working on it to make it more polished, if not I'll just keep it as an internal tool for me.

Currently, what this does is if there is a default for a field, it creates a pub const fieldname_DEFAULT, and then if there is default = use_field_defaults at the struct level, it creates a pub const DEFAULTS_FROM_FIELDS = Self::ZERO.with_field1(field1_DEFAULT).with_field2(field2_DEFAULT) etc...
To make that work, I always generate the setter functions inside a module (with pub if it's a write field), so that internally we can always call with_field function.

I think if the field_overlaps, the behaviour is undefined, and I haven't checked how it behaves with enums and arrays.

It's my first time working on a macro library, so any feedback is welcome :)

[danlehmann]

Hey, thanks for your contribution. First of all, this does solve a real usecase - which is that sometimes it is quite annoying to have to calculate the default value yourself as that's exactly the type of thing this library is supposed to solve.

I have however three fundamental concerns.

1. As you mentioned, there are quite some corner cases. For example, fields are allowed to overlap. Or you are even allowed to have two fields that use the exact same set of bits. Also it's possible to skip fields - what is their value? We could narrow down the scope somewhat - the problem is probably quite similar to the Builder struct which had to solve this.
2. This adds a lot of new syntax. And whenever we allow new syntax we essentially have to support it forever. I am not sure this is the best syntax for this problem. For example, maybe this would feel more natural?

pub struct WithDefaultFieldValues {
    #[bits(0..=2, rw)] // <- decimal ok
    val3: u3 = 4,
    //#[bits(2..=16, rw, default = 0xFFFF)] //<- Will raise an error
    #[bits(3..=16, rw)] // Accepts 0x and 0h (as base16 indicator)
    val2: u14 = 0xBEF,
    #[bits(17..=23, rw)] // Accepts 0d (as base10 indicator)
    val1: u7 = 0xd42,
    #[bits(24..=31, rw)] // Can parse negative values
    val0: i8 = -1,
}

3. In my experience, default values (in general) aren't a great idea. We've been using bitbybit in a very large codebase and we've pretty much decided that default values tend to be quite confusing. When you see Foo::DEFAULT, you really don't quite know what you're getting (0? some chip default?). Instead, what we ended up doing is to define named consts to give names to defaults. It looks like this:

#[bitfield(u32)]
pub struct RiscVExtensions {
  // .
}

impl RiscVExtensions {
  const IMA: Self = Self::ZERO.with_m(true).with_a(true);
  const IMAC: Self = Self::IMA.with_c(true);
}

While this is some more code, it allows having multiple defaults with proper names. That really helps with the intention of the code.

[SebastianHambura]

Hi, thanks for your feedback on this !

If I remember correctly, I started with your proposed solution of const + builder pattern, but it's not possible to give a read-only field a non-0 default value without having to make the math to write down the full default value of the register. How do you currently handle theses cases ?

About your comments:

1. Currently I use the builder pattern to generate the final constant value, so it's linked to the builder pattern scope. I would point out though that we could check if there are some bits that have conflicting default values (in case of overlapping bitranges). Also we could raise an error if the user asks for a default value, but doesn't provide an explicit one for the register or some fields are skipped/don't have a default value.
2. I like that proposed syntax, and it seems to be aligned with where Rust will/might be going Tracking issue for RFC 3681: Default field values rust-lang/rust#132162
3. I'll admit I just stared using this crate, and my projects are still quite small. What I don't really like about this option, is that then the default value for a field is "far away" from where the rest of the information of the field is: the documentation, bitrange, r/w options of the field are grouped in one spot, but the default value is all the way in the impl block. It does sound like a good solution to give name to some default states/config of the register though.