Message-Id: <DFZA6JOT6OVO.FWRPGW2GN9Z0@nvidia.com>
Date: Tue, 27 Jan 2026 18:57:29 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Yury Norov" <ynorov@...dia.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Boqun Feng" <boqun.feng@...il.com>,
 "Gary Guo" <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Danilo Krummrich" <dakr@...nel.org>,
 "Yury Norov" <yury.norov@...il.com>, "John Hubbard" <jhubbard@...dia.com>,
 "Alistair Popple" <apopple@...dia.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>, "Edwin Peer"
 <epeer@...dia.com>, "Eliot Courtney" <ecourtney@...dia.com>, "Daniel
 Almeida" <daniel.almeida@...labora.com>, "Dirk Behme"
 <dirk.behme@...bosch.com>, "Steven Price" <steven.price@....com>,
 <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/6] rust: add `bitfield!` macro

On Tue Jan 27, 2026 at 11:55 AM JST, Yury Norov wrote:
<snip>
>> >> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>> >> +/// let color = Rgb::default()
>> >> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>> >> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>> >> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>> >
>> > Is there a way to just say:
>> >
>> >     let color = Rgb::default().
>> >             .set_red(0x10)
>> >             .set_green(0x1f)
>> >             .set_blue(0x18)
>> >
>> > I think it should be the default style. Later in the patch you say: 
>> >
>> >     Each field is internally represented as a [`Bounded`]
>> >
>> > So, let's keep implementation decoupled from an interface?
>> 
>> That is unfortunately not feasible, but the syntax above should seldomly
>> be used outside of examples.
>
> The above short syntax is definitely more desired over that wordy and
> non-trivial version that exposes implementation internals.
>
> A regular user doesn't care of the exact mechanism that protects the
> bitfields. He wants to just assign numbers to the fields, and let
> your machinery to take care of the integrity. 

So while we cannot achieve exactly the short syntax above (which has its
drawbacks as well, such as the inability to operate in const context),
we can introduce a new setter than works with a const argument and
spares us the need to invoke `Bounded::new` ourselves:

    let color = Rgb::default().
            .with_red::<0x10>()
            .with_green::<0x1f>()
            .with_blue::<0x18>()

This setter knows which type (and thus which Bounded constructor) to
work with, and is much more concise. It also makes it clear that it
operates in const context, i.e. `color` will directly be
set to the final value. Actually let me check whether we can make the
whole chain `const`.

<snip>
>> >
>> >> +///
>> >> +/// // Convert to/from the backing storage type.
>> >> +/// let raw: u16 = color.into();
>> >
>> > What about: 
>> >
>> >         bitfield! {
>> >             pub struct Rgb(u16) {
>> >                 15:11 blue;
>> >                 10:5  set_blue;
>> >                 4:0   into;
>> >             }
>> >         }
>> >
>> > What color.set_blue() and color.into() would mean? Even if they work,
>> > I think, to stay on safe side there should be a more conventional set
>> > of accessors: color.get(into), color.set(set_blue, 0xff) and son on.
>> 
>> This would just not build.
>
> I know it wouldn't. I am just trying to understand corner cases that may
> (and would!) frustrate people for decades. 
>
> I understand that this implementation works just great for the registers,
> but my role is to make sure that it would work equally well for everyone. 
> Now, for example, Rust, contrary to C in Linux, actively uses camel case. 
> So, the blue vs Blue vs BLUE restriction is a very valid concern. The
> same for reserved words like 'into'. As long as the API matures, the
> number of such special words would only grow. The .shr() and .shl() that
> you add in this series are the quite good examples.

The strong Rust naming conventions are actually (partially) shielding us
from name clashing here.

In Rust, fields are by convention typed in snake_case. So the blue vs
Blue vs BLUE example would only be relevant if one breaks these
conventions.

That being said, we could also avoid the capitalization of the field
constants to avoid name clashing. Or we could have a const method
returning the shift, range and masks of a field as a structure. But we
cannot avoid adding new elements to the namespace, unless we start doing
contorted (and probably quite verbose) things, like having an enum type
describing the fields and a unique setter function using it to know
which field to set.

>
> Let's make a step back and just list all limitations that come with this
> implementation.

We should at the very least mention in the documentation what members
are generated so users know the limits of what they can do, yes.

>
> Again, this all is relevant for a basic generic data structure. If we
> consider it a supporting layer for the registers, everything is totally
> fine. In that case, we should just give it a more specific name, and
> probably place in an different directory, closer to IO APIs.  

v2 and v3 of this patchset actually embed the bitfield rules into the
register macro. This is designed to be temporary, but gives us more time
to iron the details you pointed out before extracting the `bitfield!`
macro and making it widely available.

In any case, I will include the `with_` setters in v4 so we can use that
in the register macro already.

Powered by blists - more mailing lists