Message-ID: <1C5C477A-2CE8-4B25-B968-416B89EA617A@nvidia.com>
Date: Mon, 26 Jan 2026 22:25:54 -0500
From: Joel Fernandes <joelagnelf@...dia.com>
To: Yury Norov <ynorov@...dia.com>
Cc: Alexandre Courbot <acourbot@...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 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.

[...]
> > > 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?

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

The user simply learns not to use reserved words, and the compiler enforces
this clearly. The same applies here.

> 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.

--
Joel Fernandes

Powered by blists - more mailing lists