Message-ID: <aXhD4sI6MHdAvgNT@yury>
Date: Mon, 26 Jan 2026 23:49:38 -0500
From: Yury Norov <ynorov@...dia.com>
To: Joel Fernandes <joelagnelf@...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 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? 
 
> [...]
> > > > 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.

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

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

Can you share more about the other potential users?

Thanks,
Yury

Powered by blists - more mailing lists