Message-ID: <aXpRor7ULMnYG_p_@yury>
Date: Wed, 28 Jan 2026 13:12:50 -0500
From: Yury Norov <ynorov@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Gary Guo <gary@...yguo.net>, Joel Fernandes <joelagnelf@...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 11:02:03PM +0900, Alexandre Courbot wrote:
> On Wed Jan 28, 2026 at 1:33 PM JST, Yury Norov wrote:
> > On Wed, Jan 28, 2026 at 10:23:36AM +0900, Alexandre Courbot wrote:
> >> tatus: O
> >> Content-Length: 4095
> >> Lines: 108
> >> 
> >> 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 agree, this version is on the edge. It probably may be acceptable
> > because it highlights that the numbers passed in setters are some
> > special numbers. But yeah, it's a weak excuse.
> >
> > If it was C, it could be just as simple as 
> >
> >         #define set_red(v) __set_red(bounded(v))
> >
> > So...
> >
> > I'm not a rust professional, but I've been told many times that macro
> > rules in rust are so powerful that they can do any magic, even mimic
> > another languages.
> >
> > For fun, I asked AI to draw an example where rust structure is
> > initialized just like normal python does, and that's what I've got:
> >
> >   struct Foo {
> >       bar: i32,
> >       baz: String,
> >   }
> >   
> >   // Your specific constructor logic
> >   fn construct_bar(v: i32) -> i32 { v * 2 }
> >   fn construct_baz(v: i32) -> String { v.to_string() }
> >   
> >   // Helper macro to select the right function for a single field
> >   macro_rules! get_ctor {
> >       (bar, $val:expr) => { construct_bar($val) };
> >       (baz, $val:expr) => { construct_baz($val) };
> >   }
> >   
> >   macro_rules! python_init {
> >       ($t:ident { $($field:ident = $val:expr),* $(,)? }) => {
> >           $t {
> >               // For each field, we call the dispatcher separately
> >               $($field: get_ctor!($field, $val)),*
> >           }
> >       };
> >   }
> >   
> >   fn main() {
> >       let foo = python_init!(Foo { bar = 10, baz = 500 });
> >   
> >       println!("bar: {}", foo.bar); // Output: 20
> >       println!("baz: {}", foo.baz); // Output: "500"
> >   }
> >
> > Indeed it's possible!
> 
> Oh yeah you can do all sorts of crazy sh** with Rust macros. :)
> 
> >
> > Again, I'm not a rust professional and I can't evaluate quality of the
> > AI-generated code, neither I can ensure there's no nasty pitfalls.
> >
> > But as a user, I can say that 
> >         
> >         let rgb = bitfield!(Rgb { red: 0x10, green: 0x1f, blue: 0x18 })
> >
> > would be way more readable than this beast:
> >
> >    let color = Rgb::default()
> >        .set_red(Bounded::<u16, _>::new::<0x10>())
> >        .set_green(Bounded::<u16, _>::new::<0x1f>())
> >        .set_blue(Bounded::<u16, _>::new::<0x18>());
> 
> Without having tested the idea, a macro wrapping the whole bitfield (and
> not just trying to create a bounded) looks doable. Of course, it would
> have to rely on some underlying mechanism to set the fields, which could
> be the abomination above, or something a bit more convenient.

As soon as it's not exposed, it's fine.
 
> It looks like we are converging towards introducing the
> `with_const_field` setter for now with registers ; when we extract the
> `bitfield!` I think I would like to entertain the introduction of a
> macro close to what you suggested above.

Good. Happy you find it useful.

Thanks,
Yury

Powered by blists - more mailing lists