[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DFZTVMUBOHH2.39WYY2O0F48NH@nvidia.com>
Date: Wed, 28 Jan 2026 10:23:36 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Gary Guo" <gary@...yguo.net>
Cc: "Joel Fernandes" <joelagnelf@...dia.com>, "Yury Norov"
<ynorov@...dia.com>, "Miguel Ojeda" <ojeda@...nel.org>, "Boqun Feng"
<boqun.feng@...il.com>, 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 Wed Jan 28, 2026 at 12:02 AM JST, Gary Guo wrote:
> On Tue Jan 27, 2026 at 3:25 AM GMT, 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.
>
> I think this could be the way forward, if we also get type inference working
> properly.
>
> Rgb::default()
> .set_read(bounded!(0x10))
> .set_green(bounded!(0x1f))
> .set_blue(bounded!(0x18))
>
> is roughly the limit that I find acceptable (`Bounded::<u16, _>::new::<0x10>()`
> is something way too verbose so I find it unacceptable).
>
> I still think if we can get
>
> Rgb::default()
> .set_read(0x10)
> .set_green(0x1f)
> .set_blue(0x18)
>
> to work with implicit `build_assert!` check it'll be ideal, although I
> understand the concern about the fragility of `build_assert!()`, especially when
> Clippy is used.
>
> I am planning to at least improve the diagnostics when `build_assert!` is used
> incorrectly and the build error actually occurs, so hopefully in the long run it
> can once again become a tool that we can rely on, but in the meantime,
> if all it needed is an extra `bounded!()` call, it doesn't bother me that much
> versus the full turbofish.
I think having a dedicated const setter method is better though. On top
of not making use of `build_assert!`, it can also be used in const
contexts to build const values, something that should be pretty useful
once we extract the `bitfield!` macro for wider use.
Powered by blists - more mailing lists