All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Gary Guo" <gary@garyguo.net>
Cc: "Joel Fernandes" <joelagnelf@nvidia.com>,
	"Yury Norov" <ynorov@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 10:23:36 +0900	[thread overview]
Message-ID: <DFZTVMUBOHH2.39WYY2O0F48NH@nvidia.com> (raw)
In-Reply-To: <DFZGOCV6VP2N.28M0CWIONBGMW@garyguo.net>

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

  reply	other threads:[~2026-01-28  1:23 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 [this message]
2026-01-28  4:33               ` Yury Norov
2026-01-28 14:02                 ` Alexandre Courbot
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=DFZTVMUBOHH2.39WYY2O0F48NH@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.