[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CA+tqQ4+Xp_Uv+O32JgCyN0vB-AJEaJdUWoWDOx0nTogeiDbj6w@mail.gmail.com>
Date: Mon, 29 Dec 2025 21:29:49 +0900
From: Jesung Yang <y.j3ms.n@...il.com>
To: Benno Lossin <lossin@...nel.org>
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 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
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.
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?
Best regards,
Jesung
Powered by blists - more mailing lists