From: Joel Fernandes <joelagnelf@nvidia.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
dri-devel@lists.freedesktop.org, dakr@kernel.org,
acourbot@nvidia.com, Alistair Popple <apopple@nvidia.com>,
Miguel Ojeda <ojeda@kernel.org>,
Alex Gaynor <alex.gaynor@gmail.com>,
Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>,
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>,
David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
John Hubbard <jhubbard@nvidia.com>, Timur Tabi <ttabi@nvidia.com>,
joel@joelfernandes.org, Elle Rhumsaa <elle@weathered-steel.dev>,
Yury Norov <yury.norov@gmail.com>,
Daniel Almeida <daniel.almeida@collabora.com>,
nouveau@lists.freedesktop.org
Subject: Re: [PATCH v4 6/6] rust: bitfield: Use 'as' operator for setter type conversion
Date: Mon, 29 Sep 2025 16:46:41 -0400 [thread overview]
Message-ID: <20250929204641.GA3056955@joelbox2> (raw)
In-Reply-To: <CANiq72k3kE-6KPkKwiDLgfkGHCQj4a2K7h9c4T13WMa5b4BAnQ@mail.gmail.com>
On Mon, Sep 29, 2025 at 03:59:32PM +0200, Miguel Ojeda wrote:
> On Sat, Sep 20, 2025 at 8:23 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
> >
> > The bitfield macro's setter currently uses the From trait for type
> > conversion, which is overly restrictive and prevents use cases such as
> > narrowing conversions (e.g., u32 storage size to u8 field size) which
> > aren't supported by From.
>
> Being restrictive is a good thing -- it would be nice to know more
> context about this change, like Alexandre points out.
Sure, I replied to that thread. Lets discuss there as well about the usecase.
> In particular, the line:
>
> .set_nibble(0x12345678_u32) // truncated to 0x8
>
> sounds fairly alarming, and not what we usually want. Why cannot the
> caller cast on their side, if they really want that?
The setter function generated in this example accepts a u32.
Actually my test case here is not good, I will fix it. In the new v5 series I
am going to post, set_nibble(0x12345678) will actually fail because the value
passed exceeds 4 bit range.
So in reality, there will be no truncation at all, this is just a bad
test case (I developed the failure mode for when the value passed exceeds
the bit range, only by v5).
> We avoid `as` for similar reasons and nowadays enable some Clippy
> warnings to prevent its use where not needed.
Understood, I can add a comment here to explain why we do it. It is just to
make the code compile, in reality we're not really truncating anything. The
issue solved in this patch is the following line wont compile when narrowing
from 32 bit to 8 bit (even if the value passed does not exceed 8 bits):
let val = (<$storage>::from(value) << SHIFT) & MASK;
> (By the way, please follow our usual coding conventions for comments.)
Sorry about that, will do.
Thanks!
- Joel
prev parent reply other threads:[~2025-09-29 20:46 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-20 18:22 [PATCH v4 0/6] Introduce bitfield and move register macro to rust/kernel/ Joel Fernandes
2025-09-20 18:22 ` [PATCH v4 1/6] nova-core: bitfield: Move bitfield-specific code from register! into new macro Joel Fernandes
2025-09-21 9:36 ` Greg KH
2025-09-21 9:59 ` Miguel Ojeda
2025-09-21 11:23 ` Greg KH
2025-09-21 12:33 ` Benno Lossin
2025-09-21 12:45 ` Greg KH
2025-09-21 13:47 ` Danilo Krummrich
2025-09-23 6:38 ` Behme Dirk (XC-CP/ESD1)
2025-09-24 10:52 ` Greg KH
2025-09-24 11:28 ` Danilo Krummrich
2025-09-24 12:04 ` Greg KH
2025-09-24 14:38 ` Yury Norov
2025-09-24 15:53 ` Danilo Krummrich
2025-09-24 17:46 ` Joel Fernandes
2025-09-24 20:01 ` Elle Rhumsaa
2025-09-25 7:05 ` Joel Fernandes
2025-09-23 22:24 ` Joel Fernandes
2025-09-24 10:40 ` Greg KH
2025-09-29 19:26 ` Joel Fernandes
2025-09-29 19:37 ` Greg KH
2025-09-29 19:45 ` Joel Fernandes
2025-09-29 20:25 ` Danilo Krummrich
2025-09-21 13:49 ` Danilo Krummrich
2025-09-29 6:16 ` Alexandre Courbot
2025-09-29 19:36 ` Joel Fernandes
2025-09-20 18:22 ` [PATCH v4 2/6] nova-core: bitfield: Add support for different storage widths Joel Fernandes
2025-09-29 6:22 ` Alexandre Courbot
2025-09-29 19:47 ` Joel Fernandes
2025-09-20 18:22 ` [PATCH v4 3/6] nova-core: bitfield: Add support for custom visiblity Joel Fernandes
2025-09-29 6:28 ` Alexandre Courbot
2025-09-29 20:20 ` Joel Fernandes
2025-09-20 18:22 ` [PATCH v4 4/6] rust: Move register and bitfield macros out of Nova Joel Fernandes
2025-09-29 6:30 ` Alexandre Courbot
2025-09-20 18:22 ` [PATCH v4 5/6] rust: Add KUNIT tests for bitfield Joel Fernandes
2025-09-20 18:22 ` [PATCH v4 6/6] rust: bitfield: Use 'as' operator for setter type conversion Joel Fernandes
2025-09-29 6:47 ` Alexandre Courbot
2025-09-29 20:32 ` Joel Fernandes
2025-09-29 13:59 ` Miguel Ojeda
2025-09-29 14:44 ` Alexandre Courbot
2025-09-29 15:17 ` Miguel Ojeda
2025-09-30 12:03 ` Joel Fernandes
2025-09-29 15:26 ` Yury Norov
2025-09-29 20:46 ` Joel Fernandes [this message]
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=20250929204641.GA3056955@joelbox2 \
--to=joelagnelf@nvidia.com \
--cc=a.hindborg@kernel.org \
--cc=acourbot@nvidia.com \
--cc=airlied@gmail.com \
--cc=alex.gaynor@gmail.com \
--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=dri-devel@lists.freedesktop.org \
--cc=elle@weathered-steel.dev \
--cc=gary@garyguo.net \
--cc=jhubbard@nvidia.com \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=mripard@kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=tmgross@umich.edu \
--cc=ttabi@nvidia.com \
--cc=tzimmermann@suse.de \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).