Message-Id: <DFZB4ID2H1D6.2ES3VEZQEGEQO@nvidia.com>
Date: Tue, 27 Jan 2026 19:41:50 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Yury Norov" <ynorov@...dia.com>
Cc: "Joel Fernandes" <joelagnelf@...dia.com>, "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>, "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 1:49 PM JST, Yury Norov wrote:
> On Mon, Jan 26, 2026 at 10:25:54PM -0500, Joel Fernandes wrote:
>> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@...dia.com> wrote:
>> > On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
>> > > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
>> > > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
>> > > > > Add a macro for defining bitfield structs with bounds-checked accessors.
>> > > > >
>> > > > > Each field is represented as a `Bounded` of the appropriate bit width,
>> > > > > ensuring field values are never silently truncated.
>> > > > >
>> > > > > Fields can optionally be converted to/from custom types, either fallibly
>> > > > > or infallibly.
>> > > > >
>> > > > > Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
>> > > > > ---
>> > > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>> > > > > rust/kernel/lib.rs      |   1 +
>> > > > > 2 files changed, 504 insertions(+)
>> [...]
>> > > > > +/// // 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.
>> >
>> > Can you please explain in details why that's not feasible, please
>> > do it in commit message. If it's an implementation constraint,
>> > please consider to re-implement.
>> 
>> If the issue is the excessive turbofish syntax, how about a macro? For
>> example:
>> 
>>     let color = Rgb::default()
>>         .set_red(bounded!(u16, 0x10))
>>         .set_green(bounded!(u16, 0x1f))
>>         .set_blue(bounded!(u16, 0x18));
>>
>> This hides the turbofish and Bounded internals while still providing
>> compile-time bounds checking.
>
> No it does not. I clearly see this 'bounded' in your version. Can you
> completely hide it inside the setter? 

Hopefully the `with_` setter I proposed in my other email settles this
issue.

>  
>> [...]
>> > > > What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?
>> > >
>> > > You wouldn't even have the luxury to yse `BLUE_SHIFT` here because where
>> > > would be conflicting definitions and thus a build error.
>> [...]
>> > > > 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.
>> >
>> > Let's make a step back and just list all limitations that come with this
>> > implementation.
>> 
>> Why is a build error not sufficient to alert the user to use better judgement
>> for naming?
>
> There should be no error at all - that's why. Any name that one can
> use for a regular variable should be acceptable as a field name.
>
>> This is no different than using reserved keywords in C. For example, this
>> won't compile:
>> 
>>     int if = 5;      // error: 'if' is a reserved keyword
>>     int return = 3;  // error: 'return' is a reserved keyword
>
> 'Into', 'as_raw', 'shr' and 'shl' *are not* the reserved words in Rust.
> Rust makes difference between upper and lower cases and doesn't break
> build if you do:
>
>         let blue = 1;
>         let BLUE = 2;
>
> So the bitfields should.

We can solve this case if we break the Rust naming conventions for
constants and keep the original case for the field, e.g. `blue_SHIFT`
and `BLUE_SHIFT`.

But again, by convention fields should all be snake_case or it would
look weird in Rust code.

>
>> The user simply learns not to use reserved words, and the compiler enforces
>> this clearly. The same applies here.
>
> Most likely he will learn not to use this API at all. The worst thing about
> those 'reserved' words is that the set of them is not definite and will
> constantly grow.
>
> I'm trying to say that this approach is not scalable. If a random client
> gives name 'invert' to one of his fields, and you'll need to implement a
> method inverting bits, what are you going to do?

Bitfields are limited to a _get, a _set, and an _update methods (and
possibly try_ variants for the last two). If an `invert` method needs to
be implemented, it can be done on top of _update which takes a closure.
So I am pretty confident we won't need to extend the API beyond these
(famous last words).

>
>> > 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.
>> 
>> The Bitfield macro is very much required for non-register use cases too.
>
> Then let's implement it better. Can you comment why the suggested API
> doesn't work for you?
>
>         color.set(blue, 10);
>         color.get(blue);
>
> I think it should solve the problem with name clashing.

That syntax cannot be implemented as it is written. What type is `blue` here?

The closest we could get would be a macro, that would look like

    bitfield_set!(color, blue, 10);

And beyond the scenes it would call some more intricate (and unsightly)
machinery. I'd rather define the constraints clearly for users - they
are not so drastic.

>
> Can you share more about the other potential users?

I know Joel is using bitfield for page table structures, but there are
of course many others. Basically any structure with fields defined as a
subset of its bits is a candidate. Registers just happen to be bitfields
with extra properties for I/O.

Powered by blists - more mailing lists