On Mon, Sep 08, 2025 at 12:40:17PM +0900, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> > Add support for custom visiblity to allow for users to control visibility
> > of the structure and helpers.
> >
> > Signed-off-by: Joel Fernandes <[email protected]>
> > ---
> > drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> > drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> > 2 files changed, 31 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/nova-core/bitstruct.rs
> > b/drivers/gpu/nova-core/bitstruct.rs
> > index 068334c86981..1047c5c17e2d 100644
> > --- a/drivers/gpu/nova-core/bitstruct.rs
> > +++ b/drivers/gpu/nova-core/bitstruct.rs
> > @@ -9,7 +9,7 @@
> > ///
> > /// ```rust
> > /// bitstruct! {
> > -/// struct ControlReg: u32 {
> > +/// pub struct ControlReg: u32 {
> > /// 3:0 mode as u8 ?=> Mode;
> > /// 7:4 state as u8 => State;
> > /// }
>
> Maybe mention in the documentation that the field accessors are given
> the same visibility as the type - otherwise one might be led into
> thinking that they can specify visibility for individual fields as well
> (I'm wondering whether we might ever want that in the future?).
Sure, good idea, done.
> > @@ -34,21 +34,21 @@
> > /// and returns the result. This is useful with fields for which not all
> > values are valid.
> > macro_rules! bitstruct {
> > // Main entry point - defines the bitfield struct with fields
> > - (struct $name:ident : $storage:ty $(, $comment:literal)? {
> > $($fields:tt)* }) => {
> > - bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
> > + ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? {
> > $($fields:tt)* }) => {
> > + bitstruct!(@core $name $vis $storage $(, $comment)? { $($fields)*
> > });
> > };
> >
> > // All rules below are helpers.
> >
> > // Defines the wrapper `$name` type, as well as its relevant
> > implementations (`Debug`,
> > // `Default`, `BitOr`, and conversion to the value type) and field
> > accessor methods.
> > - (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)*
> > }) => {
> > + (@core $name:ident $vis:vis $storage:ty $(, $comment:literal)? {
> > $($fields:tt)* }) => {
>
> Being very nitpicky here, but for consistency why not put `$vis` before
> `$name` since it is the order they are given by the caller?
Perfect comment, changed it.
> > $(
> > #[doc=$comment]
> > )?
> > #[repr(transparent)]
> > #[derive(Clone, Copy)]
> > - pub(crate) struct $name($storage);
> > + $vis struct $name($vis $storage);
>
> `$storage` should probably be kept private - we already have accessors
> for it, and the visibility parameter is for the outer type, not its
> internals.
Already done for next revision. Thanks,
thanks,
- Joel
