Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova

[Joel Fernandes <joelagnelf@nvidia.com>]

On 9/7/2025 2:14 PM, Miguel Ojeda wrote:
> Hi Joel,
> 
> I didn't check the macros, but a couple nits I noticed in this patch
> in particular given it moved it to `kernel`...
> 
> On Wed, Sep 3, 2025 at 11:54 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>
>> +//! A library that provides support for defining bit fields in Rust
> 
> What about just "Support for defining bit fields in ..." or similar?
> 
>> +//! structures. Also used from things that need bitfields like register macro.

Ok, I changed it to:
"Also used from things that need bitfields like [`register!`] macro." for next
revision.

> 
> The "register macro" part sounds like it should be formatted with
> Markdown plus an intra-doc link.
> 
>> -            ::kernel::build_assert!(
>> +            build_assert!(
> 
> Is this path unqualified for some reason? Does it mean the user needs
> to have imported the prelude?

Yes, for register macro importing prelude is required (I commented more below).

> 
>> +pub use super::{bitstruct, register};
> 
> Please justify in the commit message why we want them in the prelude,
> e.g. are they expected to be common? Does it simplify some code? etc.
> 
The issue I ran into is, without adding it to prelude, the users of register!
macro will have to import both bitfield! and register! macros explictly, even
though they're only using register!. I tried to make it work without adding to
prelude, but couldn't:

  use kernel::{bitfield, register};

Also not adding it to prelude, means register! macro has to invoke bitfield with
$crate prefixed  ($crate::bitfield).

I think the prelude-way is clean, but let me know if there's any other trick I
can try.

I will also add this rationale to the commit message as you suggested.

Thanks!

 - Joel