Message-ID: <F2B7D456-70A0-4A0B-8C4F-7835AF2BC413@nvidia.com>
Date: Tue, 27 Jan 2026 06:00:13 -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 11:49 PM, Yury Norov <ynorov@...dia.com> 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?

Looks like Alex replied with something that does that. So it is good.

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

Disagreed. Here is why: the Bitfield macro is required because we are extending
the language. We could very well get this as a language feature in the future.
In the event of that, it is non sensical to allow users to use reserved
keywords. The same reason you would run into errors on the C side.

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

They are reserved as far as our Bitfield syntax extension goes? Note that this
is a macro that introduces custom syntax so it is more akin to a language
extension than an api. Note well also that macros in rust for Linux have been
used extensively for filling gaps language features (see pin initialization for
instance).

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

Why on earth would anyone want to do this though? It’s utterly nonsensical to
do so and I would rather break the build than encourage bad code. Mixing case
for 2 fields in the same struct is not a usecase anyone should have.

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

I would error the build out. See above for comments on language extensions and
reserved keywords.

>>> 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);

For the right reasons, I do not mind it but it certainly cannot be that we want
to support mixed case of the same field IMO. And the reserved keyword argument
is weak at best, for a language extension.

> I think it should solve the problem with name clashing.
>
> Can you share more about the other potential users?

Sure, take a look at my memory management patches for nova. [1]

[1] https://lore.kernel.org/all/20260120204303.3229303-1-joelagnelf@nvidia.com/

-- 
Joel Fernandes

Powered by blists - more mailing lists