Re: [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support

[Gary Guo <gary@garyguo.net>]

On Tue, 30 Dec 2025 08:17:13 +0800
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Mon, Dec 29, 2025 at 04:36:16PM +0000, Gary Guo wrote:
> > On Mon, 29 Dec 2025 19:13:11 +0800
> > Boqun Feng <boqun.feng@gmail.com> wrote:
> >   
> > > On Sun, Dec 28, 2025 at 09:05:44PM +0900, FUJITA Tomonori wrote:  
> > > > Rework the internal AtomicOps macro plumbing to generate per-type
> > > > implementations from a mapping list.
> > > > 
> > > > Capture the trait definition once and reuse it for both declaration
> > > > and per-type impl expansion to reduce duplication and keep future
> > > > extensions simple.
> > > > 
> > > > This is a preparatory refactor for enabling i8/i16 atomics cleanly.
> > > > 
> > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>    
> > > 
> > > Thanks! I have an idea that uses proc-macro to generate the Atomic*Ops
> > > impls, e.g.
> > > 
> > >     #[atomic_ops(i8, i16, i32, i64)]
> > >     pub trait AtomicBasicOps {
> > >         #[variant(acquire)]
> > >         fn read(a: &AtomicRepr<Self>) -> Self {
> > > 	    unsafe { binding_call!(a.as_ptr().cast()) }
> > > 	}
> > >     }  
> > 
> > Unless the proc macro is generally applicable to a wide range of subsystem
> > abstractions, I would still prefer to use declarative macros.
> >   
> 
> But the tt-muncher style is quite challenging for a boarder audience to
> understand and change (if necessary) the declarative macros. IMO, if we
> can have mod-specific proc macros (i.e. macros that are only usable in
> atomic mod) then it should be fine. Thoughts?

I don't think our current macro impl is too bad (not really tt-muncher I
think?), and it's still very readable to me.  Proc macros can also be
challenging for a broader audience, as it requires you to read parser code
:)

Unless the macro is complex enough (e.g. pin-init) or has very broad usage
in the kernel, I think it's still best to limit to declarative macros.

Best,
Gary

> 
> Regards,
> Boqun
> 
> > Best,
> > Gary
> >   
> > > 
> > > But I think the current solution in your patch suffices as a temporary
> > > solution at least.
> > > 
> > > Regards,
> > > Boqun