Re: [PATCH v2] rust: impl_flags: add method to return underlying integer
From: Gary Guo
Date: Sat Jun 06 2026 - 08:23:25 EST
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@xxxxxxxxxx> 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