Re: [PATCH v2] rust: impl_flags: add method to return underlying integer

["Gary Guo" <gary@garyguo.net>]

On Sat Jun 6, 2026 at 12:01 AM BST, Miguel Ojeda wrote:
> On Fri, Jun 5, 2026 at 12:55 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> +/// An optional visibility specifier in the inner-field position of the
>> +/// bitmask struct controls the visibility of the generated `bits`
>> +/// accessor. Declare it `pub` (e.g., `pub struct Permissions(pub u32)`)
>> +/// to expose the raw integer outside the defining module — useful when
>> +/// passing the value to C APIs. Omit it to keep `bits` private.
>
> Hmm... It is clever, but it can be quite confusing. Could we use some
> other way of "tagging" it?
>
> I would also put `()` in `bits`, and make it a intra-doc links, to
> make it extra clear it is the method we are talking about here.
>
> And why do we call this "private", given the `From` as Sashiko points out?

I think I suggested this approach because I don't want people to be able to
easily be able to get integers while enums could be used. But I didn't know that
the `From` impl already exists for flags.

Given the case we could probably just unconditionally expose the `bits` method
(and whatever mechanism we use to avoid exposing bits would also remove the
From/TryFrom/bits/all_bits/...)

Best,
Gary

>
> i.e. your example works if you do (without the patch):
>
>     /// let raw: u32 = read_only.into();
>     /// assert_eq!(raw, Permission::Read as u32);
>
>>  ///     /// Represents multiple permissions.
>>  ///     #[derive(Debug, Clone, Default, Copy, PartialEq, Eq)]
>> -///     pub struct Permissions(u32);
>> +///     pub struct Permissions(pub u32);
>
> I would prefer if we had separate examples for "optional features"
> like this; otherwise, people may think this is the "common" way of
> doing it -- and I assume in cases like this we want users to avoid
> exposing if not needed, no?
>
>> +            /// The visibility of this method is controlled by the optional
>> +            /// visibility specifier in the inner-field position of the
>> +            /// bitmask struct declaration.
>
> I would make this paragraph a comment as Sashiko points out.
>
> Cheers,
> Miguel