Message-Id: <DFBFMTS0ZRDB.30O3L4GMYW4XJ@kernel.org>
Date: Tue, 30 Dec 2025 10:09:51 +0100
From: "Benno Lossin" <lossin@...nel.org>
To: "Jesung Yang" <y.j3ms.n@...il.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Boqun Feng" <boqun.feng@...il.com>,
 "Gary Guo" <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Andreas Hindborg" <a.hindborg@...nel.org>,
 "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>,
 "Danilo Krummrich" <dakr@...nel.org>, "Alexandre Courbot"
 <acourbot@...dia.com>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, <nouveau@...ts.freedesktop.org>
Subject: Re: [PATCH v4 1/4] rust: macros: add derive macro for `Into`

On Mon Dec 29, 2025 at 1:29 PM CET, Jesung Yang wrote:
> On Sat, Dec 27, 2025 at 1:57 PM Benno Lossin <lossin@...nel.org> wrote:
>> On Fri Dec 26, 2025 at 10:36 AM CET, Jesung Yang wrote:
>> > On Fri, Dec 26, 2025 at 2:40 AM Benno Lossin <lossin@...nel.org> wrote:
>> >> On Thu Dec 25, 2025 at 9:37 AM CET, Jesung Yang via B4 Relay wrote:
>> >> > +    // Note on field-less `repr(C)` enums (quote from [1]):
>> >> > +    //
>> >> > +    //   In C, enums with discriminants that do not all fit into an `int` or all fit into an
>> >> > +    //   `unsigned int` are a portability hazard: such enums are only permitted since C23, and not
>> >> > +    //   supported e.g. by MSVC.
>> >> > +    //
>> >> > +    //   Furthermore, Rust interprets the discriminant values of `repr(C)` enums as expressions of
>> >> > +    //   type `isize`. This makes it impossible to implement the C23 behavior of enums where the
>> >> > +    //   enum discriminants have no predefined type and instead the enum uses a type large enough
>> >> > +    //   to hold all discriminants.
>> >> > +    //
>> >> > +    //   Therefore, `repr(C)` enums in Rust require that either all discriminants to fit into a C
>> >> > +    //   `int` or they all fit into an `unsigned int`.
>> >> > +    //
>> >> > +    // As such, `isize` is a reasonable representation for `repr(C)` enums, as it covers the range
>> >> > +    //  of both `int` and `unsigned int`.
>> >> > +    //
>> >> > +    // For more information, see:
>> >> > +    // - https://github.com/rust-lang/rust/issues/124403
>> >> > +    // - https://github.com/rust-lang/rust/pull/147017
>> >> > +    // - https://github.com/rust-lang/rust/blob/2ca7bcd03b87b52f7055a59b817443b0ac4a530d/compiler/rustc_lint_defs/src/builtin.rs#L5251-L5263 [1]
>> >> > +
>> >> > +    // Extract the representation passed by `#[repr(...)]` if present. If nothing is
>> >> > +    // specified, the default is `Rust` representation, which uses `isize` for its
>> >> > +    // discriminant type.
>> >> > +    // See: https://doc.rust-lang.org/reference/items/enumerations.html#r-items.enum.discriminant.repr-rust
>> >>
>> >> I think we should error when no `#[repr({integer})]` attribute is
>> >> specified.
>> >
>> > Not a blocker but just out of curiosity: are you concerned that the
>> > default size might change in the future, leading to silent side
>> > effects?
>>
>> `isize` already changes size when you switch between 64 and 32 bit
>> architectures. I think the author of an enum they want to convert into
>> integers should think about which size the enum should be.
>>
>> They already can choose `repr(isize)` if that is correct in that case.
>> As a default, I would have choosen `i32` (but that conflicts with Rust's
>> default, so we can't do it).
>
> On second thought, I've realized that enforcing an error when
> `#[repr({integer})]` is missing would prevent users from deriving
> traits for `#[repr(C)]` enums. This is because rustc currently rejects
> the combined `#[repr(C, {integer})]` syntax. For example, a user might

It's only rejected for field-less enums, but those are what we're
interested here.

> want to do this:
>
>     #[derive(Into)]
>     #[into(u8)]
>     #[repr(C)]
>     enum Foo {
>         A,
>         B,
>     }
>
> In this case, the code wouldn't compile if we strictly require
> `#[repr({integer})]`, even if the user carefully picked `u8`, keeping
> the enum's size (more precisely, the discriminant range) in mind.
>
> Since we already perform a compile-time check to ensure all
> discriminants fit within the types specified in the helper attributes,
> I believe `#[repr({integer})]` isn't strictly required. Even with
> `isize` changing size across architectures, our compile-time check
> remains effective and will catch any overflows.

Hmm that's true.

> To sum up, I see three options:
>
>     1. Drop support for `#[repr(C)]` enums entirely.
>     2. Special-case `#[repr(C)]` enums: allow them to default to
>        `isize`, otherwise require `#[repr({integer})]`.
>     3. Permit missing `#[repr({integer})]` generally.
>
> I am personally leaning toward Option 3, since our existing
> compile-time check provides a sufficient safety margin to allow this
> flexibility.
>
> Thoughts on these?

Looking at the nomicon documentation [1] again, I found the following:

    repr(C) is equivalent to one of repr(u*) (see the next section) for
    fieldless enums. The chosen size and sign is the default enum size
    and sign for the target platform's C application binary interface
    (ABI). Note that enum representation in C is implementation defined,
    so this is really a "best guess". In particular, this may be
    incorrect when the C code of interest is compiled with certain
    flags.

Which to me reads as "don't use `repr(C)`, if you want to know which
repr the enum gets". Especially the last part is concerning to me, as
the kernel uses lots of (bespoke) compiler flags. So I'm thinking we
should just drop `repr(C)` enum support. Any thoughts from others?

[1]: https://doc.rust-lang.org/nomicon/other-reprs.html

Cheers,
Benno

Powered by blists - more mailing lists