All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Yury Norov" <ynorov@nvidia.com>
Cc: "Gary Guo" <gary@garyguo.net>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Yury Norov" <yury.norov@gmail.com>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>, "Edwin Peer" <epeer@nvidia.com>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Dirk Behme" <dirk.behme@de.bosch.com>,
	"Steven Price" <steven.price@arm.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/6] rust: add `bitfield!` macro
Date: Wed, 28 Jan 2026 23:02:03 +0900	[thread overview]
Message-ID: <DG0A0CPEYCIR.1ZMN457FUXZXM@nvidia.com> (raw)
In-Reply-To: <aXmRqzkKq0bAQaV4@yury>

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

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.

  reply	other threads:[~2026-01-28 14:02 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-20  6:17 [PATCH 0/6] rust: add `bitfield!` and `register!` macros Alexandre Courbot
2026-01-20  6:17 ` [PATCH 1/6] rust: num: add `shr` and `shl` methods to `Bounded` Alexandre Courbot
2026-01-20  8:44   ` Alice Ryhl
2026-01-20 12:53     ` Alexandre Courbot
2026-01-20 16:12   ` kernel test robot
2026-01-21  8:15   ` Yury Norov
2026-01-21 10:10     ` Alice Ryhl
2026-01-20  6:17 ` [PATCH 2/6] rust: num: add `as_bool` method to `Bounded<_, 1>` Alexandre Courbot
2026-01-20  8:45   ` Alice Ryhl
2026-01-20  6:17 ` [PATCH 3/6] rust: add `bitfield!` macro Alexandre Courbot
2026-01-20 11:45   ` Dirk Behme
2026-01-20 12:37     ` Miguel Ojeda
2026-01-20 12:47       ` Dirk Behme
2026-01-20 13:08         ` Miguel Ojeda
2026-01-20 13:20           ` Alexandre Courbot
2026-01-20 21:02             ` Miguel Ojeda
2026-01-20 12:51     ` Alexandre Courbot
2026-01-21  9:16   ` Yury Norov
2026-01-26 13:35     ` Alexandre Courbot
2026-01-27  2:55       ` Yury Norov
2026-01-27  3:25         ` Joel Fernandes
2026-01-27  4:49           ` Yury Norov
2026-01-27 10:41             ` Alexandre Courbot
2026-01-27 10:55               ` Miguel Ojeda
2026-01-28  5:27               ` Yury Norov
2026-01-28 14:12                 ` Alexandre Courbot
2026-01-28 18:05                   ` Yury Norov
2026-01-29 13:40                     ` Alexandre Courbot
2026-01-29 15:12                       ` Miguel Ojeda
2026-01-27 11:00             ` Joel Fernandes
2026-01-27 15:02           ` Gary Guo
2026-01-28  1:23             ` Alexandre Courbot
2026-01-28  4:33               ` Yury Norov
2026-01-28 14:02                 ` Alexandre Courbot [this message]
2026-01-28 18:12                   ` Yury Norov
2026-01-27  9:57         ` Alexandre Courbot
2026-01-27 21:03           ` John Hubbard
2026-01-27 21:10             ` Gary Guo
2026-01-27 21:22               ` John Hubbard
2026-01-28  1:28               ` Alexandre Courbot
2026-01-28  1:41                 ` John Hubbard
2026-01-20  6:17 ` [PATCH 4/6] rust: bitfield: Add KUNIT tests for bitfield Alexandre Courbot
2026-01-20  6:17 ` [PATCH 5/6] rust: io: add `register!` macro Alexandre Courbot
2026-01-20  6:17 ` [PATCH FOR REFERENCE 6/6] gpu: nova-core: use the kernel `register!` and `bitfield!` macros Alexandre Courbot
2026-01-20 13:14 ` [PATCH 0/6] rust: add `bitfield!` and `register!` macros Miguel Ojeda
2026-01-20 13:38   ` Danilo Krummrich
2026-01-20 13:50     ` Miguel Ojeda
2026-01-20 14:18       ` Danilo Krummrich
2026-01-20 14:57         ` Miguel Ojeda
2026-01-20 15:27           ` Danilo Krummrich
2026-01-20 15:48             ` Miguel Ojeda
2026-01-20 20:01               ` Danilo Krummrich
2026-01-20 20:31                 ` Miguel Ojeda
2026-01-21  5:57                   ` Yury Norov
2026-01-21  6:55                     ` Alexandre Courbot
2026-01-26 14:03                     ` Joel Fernandes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DG0A0CPEYCIR.1ZMN457FUXZXM@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dirk.behme@de.bosch.com \
    --cc=ecourtney@nvidia.com \
    --cc=epeer@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=steven.price@arm.com \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    --cc=ynorov@nvidia.com \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.