Re: [PATCH v5 6/9] rust: bitfield: Add KUNIT tests for bitfield

["Alexandre Courbot" <acourbot@nvidia.com>]

On Thu Oct 2, 2025 at 11:16 AM JST, Elle Rhumsaa wrote:
> On 10/2/25 1:41 AM, Alexandre Courbot wrote:
>
>> On Tue Sep 30, 2025 at 11:45 PM JST, Joel Fernandes wrote:
>>> Add KUNIT tests to make sure the macro is working correctly.
>>>
>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>> ---
>>>   rust/kernel/bitfield.rs | 321 ++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 321 insertions(+)
>>>
>>> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
>>> index fed19918c3b9..9a20bcd2eb60 100644
>>> --- a/rust/kernel/bitfield.rs
>>> +++ b/rust/kernel/bitfield.rs
>>> @@ -402,3 +402,324 @@ fn default() -> Self {
>>>           }
>>>       };
>>>   }
>>> +
>>> +#[::kernel::macros::kunit_tests(kernel_bitfield)]
>>> +mod tests {
>>> +    use core::convert::TryFrom;
>>> +
>>> +    // Enum types for testing => and ?=> conversions
>>> +    #[derive(Debug, Clone, Copy, PartialEq)]
>>> +    enum MemoryType {
>>> +        Unmapped = 0,
>>> +        Normal = 1,
>>> +        Device = 2,
>>> +        Reserved = 3,
>>> +    }
>>> +
>>> +    impl Default for MemoryType {
>>> +        fn default() -> Self {
>>> +            MemoryType::Unmapped
>>> +        }
>>> +    }
>> Tip: you can add `Default` to the `#[derive]` marker of `MemoryType` and
>> mark the variant you want as default with `#[default]` instead of
>> providing a full impl block:
>>
>>      #[derive(Debug, Default, Clone, Copy, PartialEq)]
>>      enum MemoryType {
>>          #[default]
>>          Unmapped = 0,
>>          Normal = 1,
>>          Device = 2,
>>          Reserved = 3,
>>      }
>
> I would alternatively recommend to provide a `MemoryType::new` impl with 
> a `const` definition:
>
> ```rust
> impl MemoryType {
>      pub const fn new() -> Self {
>
>          Self::Unmapped
>
>      }
> }
>
> impl Default for MemoryType {
>      fn default() -> Self {
>          Self::new()
>      }
> }
> ```
>
> This pattern allows using `MemoryType::new()` in `const` contexts, while 
> also providing the `Default` impl using the same default. It's somewhat 
> of a workaround until we get `const` traits.

That's an elegant pattern generally speaking, but I don't think we would
benefit from using it in these unit tests.