All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Daniel Almeida" <daniel.almeida@collabora.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v6] rust: kernel: add support for bits/genmask macros
Date: Tue, 17 Jun 2025 00:08:52 +0900	[thread overview]
Message-ID: <DAO1WDWJTT04.1P4XT0W2XHPNW@nvidia.com> (raw)
In-Reply-To: <9578ECFC-6C59-40E3-9340-A426E8D2328A@collabora.com>

On Mon Jun 16, 2025 at 11:14 PM JST, Daniel Almeida wrote:
>>> +macro_rules! impl_bit_fn {
>>> +    (
>>> +        $checked_name:ident, $unbounded_name:ident, $const_name:ident, $ty:ty
>>> +    ) => {
>>> +        /// Computes `1 << n` if `n` is in bounds, i.e.: if `n` is smaller than
>>> +        /// the maximum number of bits supported by the type.
>>> +        ///
>>> +        /// Returns [`None`] otherwise.
>>> +        #[inline]
>>> +        pub fn $checked_name(n: u32) -> Option<$ty> {
>>> +            (1 as $ty) .checked_shl(n)
>>> +        }
>>> +
>>> +        /// Computes `1 << n` if `n` is in bounds, i.e.: if `n` is smaller than
>>> +        /// the maximum number of bits supported by the type.
>>> +        ///
>>> +        /// Returns `0` otherwise.
>>> +        ///
>>> +        /// This is a convenience, as [`Option::unwrap_or`] cannot be used in
>>> +        /// const-context.
>>> +        #[inline]
>>> +        pub fn $unbounded_name(n: u32) -> $ty {
>>> +            match $checked_name(n) {
>>> +                Some(v) => v,
>>> +                None => 0,
>>> +            }
>> 
>> This could more succintly be `$checked_name(n).unwrap_or(0)` (same
>> remark for `$genmask_unbounded` below).
>> 
>
> Wait, I just realized that $unbounded_name is not ‘const fn’, so we don’t need this function at all?
>
> Users can simply do `unwrap_or` on their own.

Agreed, we can probably drop this.

>> 
>> ... or we make the methods generic against `RangeBounds` and allow both
>> `Range` and `RangeInclusive` to be used. But I'm concerned that callers
>> might use `0..1` thinking it is inclusive while it is not.
>> 
>> Thoughts?
>
> I don't think we can do what you suggested here. I assume that we'd have to
> rely on [0] and friends, and these are not const fn, so they can’t be used in
> the const version of genmask.

You are right, this cannot be used here. It's not a big loss, limiting
the API to inclusive ranges as discussed on the other thread might
actually end up being safer than having two options.


      parent reply	other threads:[~2025-06-16 15:09 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-10 14:14 [PATCH v6] rust: kernel: add support for bits/genmask macros Daniel Almeida
2025-06-10 18:08 ` Miguel Ojeda
2025-06-10 20:52   ` Daniel Almeida
2025-06-14 13:38 ` Alexandre Courbot
2025-06-14 15:06   ` Boqun Feng
2025-06-14 15:56     ` Boqun Feng
2025-06-14 16:05       ` Boqun Feng
2025-06-18 20:58         ` Joel Fernandes
2025-06-20 13:48           ` Daniel Almeida
2025-06-20 20:47             ` Joel Fernandes
2025-06-15 12:59     ` Alexandre Courbot
2025-06-16 14:14   ` Daniel Almeida
2025-06-16 14:29     ` Boqun Feng
2025-06-16 14:42       ` Daniel Almeida
2025-06-16 14:45         ` Daniel Almeida
2025-06-16 14:52           ` Alexandre Courbot
2025-06-16 14:56             ` Daniel Almeida
2025-06-16 15:02               ` Alexandre Courbot
2025-06-16 15:02           ` Boqun Feng
2025-06-16 15:08     ` Alexandre Courbot [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=DAO1WDWJTT04.1P4XT0W2XHPNW@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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.