Message-ID: <CAC=eVgSKwqxpEPPNv++ipHGJpNE9TxjdWK91N+wDR=L3RibK2A@mail.gmail.com>
Date: Sat, 3 Jan 2026 03:15:10 +0200
From: Kari Argillander <kari.argillander@...il.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Filipe Xavier <felipeaggger@...il.com>, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, 
	Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org, felipe_life@...e.com, 
	linux-kernel@...r.kernel.org, Lyude Paul <lyude@...hat.com>
Subject: Re: [PATCH v7] rust: add new macro for common bitmap operations

On 2.1.2026 16.44 Daniel Almeida (daniel.almeida@...labora.com) wrote:
> > On 1 Jan 2026, at 18:39, Kari Argillander <kari.argillander@...il.com> wrote:
> >
> > to 1.1.2026 klo 22.23 Daniel Almeida (daniel.almeida@...labora.com) kirjoitti:
> >>> On 1 Jan 2026, at 16:14, Kari Argillander <kari.argillander@...il.com> wrote:
> >>>
> >>> On Thu, 1 Jan 2026 at 20:21, Filipe Xavier <felipeaggger@...il.com> wrote:
> >>>
> >>>> +/// // Combine multiple permissions using operation OR (`|`).
> >>>> +/// let read_write: Permissions = Permission::Read | Permission::Write;
> >>>> +///
> >>>> +/// assert!(read_write.contains(Permission::Read));
> >>>> +/// assert!(read_write.contains(Permission::Write));
> >>>> +/// assert!(!read_write.contains(Permission::Execute));
> >>>> +/// assert!(read_write.contains_any(Permission::Read | Permission::Execute));
> >>>> +/// assert!(read_write.contains_all(Permission::Read | Permission::Write));
> >>>> +///
> >>>> +/// // Removing a permission with operation AND (`&`).
> >>>> +/// let read_only: Permissions = read_write & Permission::Read;
> >>>> +/// assert!(read_only.contains(Permission::Read));
> >>>> +/// assert!(!read_only.contains(Permission::Write));
> >>>> +///
> >>>> +/// // Toggling permissions with XOR (`^`).
> >>>> +/// let toggled: Permissions = read_only ^ Permission::Read;
> >>>> +/// assert!(!toggled.contains(Permission::Read));
> >>>> +///
> >>>> +/// // Inverting permissions with negation (`!`).
> >>>> +/// let negated = !read_only;
> >>>> +/// assert!(negated.contains(Permission::Write));
> >>>> +/// assert!(!negated.contains(Permission::Read));
> >>>
> >>> I really like the OR (`|`) operator. Personally, I don’t like anything
> >>> else. When
> >>> a function or expression gets longer, something like let negated = !read_only;
> >>> already feels strange to read.
> >>>
> >>> My suggestion is that everything else should be functions, so we would write:
> >>>
> >>> ```rust
> >>>   let xxx = Permission::Read | Permission::Write;
> >>>   xxx.remove(Permission::Write | Permission::Read);
> >>>   xxx.toggle(Permission::Execute);
> >>>   xxx.invert();
> >>> ```
> >>>
> >>> This way we avoid expressions like:
> >>>
> >>> ```rust
> >>>   let a = !(!xxx | yyy ^ zzz);
> >>> ```
> >>>
> >> “
> >>> At that point, no one understands what is really happening anymore. People start
> >>> wondering whether operator precedence or evaluation order matters, and may even
> >>> have to go read the source code to figure it out.
>
>
> Then don’t write convoluted expressions…it’s not something you can
> fix at the language level.

I disagree. Rust already fix lot of problems at language level. This sounds
to me it is the same as "Then don't write memory bugs, it's not something
you can fix at the language level". And Rust still did it.

> What’s your take on the boolean operators, by the way?

They are very nice. I do not see how this is relevant.

> >> I disagree. How would that look like given your suggestion?
> >>
> >>>
> >>> By only supporting OR (|) as an operator and expressing everything else as
> >>> explicit function calls, the code becomes much easier to read, review, and
> >>> maintain.
> >>>
> >>>   Argillander
> >>
> >> How is A | B readable but not A & B or A ^ B?
> >
> > Because that is just a way to express multiple / group of Flags and nothing
> > more. And thinking of this it could be even & operator. So we only allow
>
> So A | B is “just a way to express multiple flags” instead of the much
> more obvious A OR B? Same for A & B .
>
> > creating / enabling / disabling / toggling for a group of flags. Everything
> > still needs to be explicit. So we would end up with something like
> >
> > ```rust
> >    Flags::new(A | B);
>
> This is _much_ more straightforward:
>
> +/// // Combine multiple permissions using operation OR (`|`).
> +/// let read_write: Permissions = Permission::Read | Permission::Write;
>
> >    toggle(A | B);
> >    disable(A | B); // Or remove() (I do not have opinion)
> >    enable(A | B);
>
> Now we have to break extremely straightforward operations into multiple
> function calls. This is not ergonomic.
>
> >
> >    // I would even consider & operator here as then you read it quite nicely
> >    // toggle (A and B)
> >    toggle(A & B);
> > ```
>
> Sorry, I don’t buy this syntax at all. What’s the difference between A | B vs
> A & B in your proposal?

Following is a problem in my opinion. Rust use | for OR in match statements.
So if we do bitflag operations it looks quite strange when we do this:

```
    const WRITE_READ: Permissions = Permission::Write | Permission::Read;
    match permissions {
        Permissions::None => println!("None"),
        Permissions::All => println!("All"),
        Permission::Write | Permission::Read => println!("Write or read"),
        WRITE_READ=>  println!("Write and read"),
        _ => {},
    }
```

This has actually been documented in rust bitflags [1]. I also think we should
document this somewhere because this is not very obvious in Rust.

[1]: https://docs.rs/bitflags/latest/bitflags/macro.bitflags_match.html

> >
> > Most annoying thing with flags is that if you need to enable 8 flags then some
> > operator becomes a must. Still using those in everywhere is error prone and
>
> So you’re proposing both operators and these functions?
>
> > confusing. So I totally agreed that we need some way to express group of flags
> > without always doing something like "t.enable(Flag::A).enable(Flag::B)"
> >
> > If this way is used it is really nice that every function get's documentation.
>
> We do *not* need documentation for “toggle()”, “invert()” and friends.
>
>
> > If we only allow | or & there is very little chance that someone mix what those
> > means as it is always documented function call. Using operators too freely feels
>
> No kernel developer is ever going to get confused by basic bitwise operations.

Most developers just need to write drivers.

> > very C++ in my opinion. Rust is very explicit language so let's not change that
>
> How is C++ relevant here? Bitwise ops are used all over the place in C too.

Sorry for my bad words here. I meant C++ operator overloads, not
bitwise operations
themself. Those are used very freely in many places So you actually do
not know what
even "+ operators" do anymore.

> > but let's still make it usable for our cases.
>
> The language specifically allows us to build this API, so I see no problem with
> the current approach. Your implementation is, in my humble opinion, a downgrade
> w.r.t to what has been developed so far: it changes a syntax that everyone
> understands in order to solve a problem that doesn’t really exist.

bitwise operations are backed in C. In Rust you really have to know
that now this
struct supports bitwise operations. That is not so obvious in Rust
that you can do
those operations for non-integer types.

But probably there is no point continue discussion as two people has nacked
my suggestion so fast and I'm totally ok with it. Still thanks for
taking time on this.

    Argillander

Powered by blists - more mailing lists