Re: [PATCH v7 2/9] rust: sync: Add basic atomic operation mapping framework

[Boqun Feng <boqun.feng@gmail.com>]

On Mon, Jul 14, 2025 at 12:03:11PM +0200, Benno Lossin wrote:
[...]
> > +declare_and_impl_atomic_methods!(
> > +    /// Basic atomic operations
> > +    pub trait AtomicHasBasicOps {
> 
> I think we should drop the `Has` from the names. So this one can just be
> `AtomicBasicOps`. Or how about `BasicAtomic`, or `AtomicBase`?
> 

Actually, given your feedback to `ordering::Any` vs `core::any::Any`,
I think it makes more sense to keep the current names ;-) These are only
trait names to describe what operations do an `AtomicImpl` has, and they
should be used only in atomic mod, so naming them a bit longer is not a
problem. And by doing so, we free the better names `BasicAtomic` or
`AtomicBase` for other future usages.

Best I could do is `AtomicBasicOps` or `HasAtomicBasicOps`.

> > +        /// Atomic read (load).
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> > +        /// - `ptr` is valid for reads.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn read[acquire](ptr: *mut Self) -> Self {
> > +            bindings::#call(ptr.cast())
> > +        }
> > +
> > +        /// Atomic set (store).
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> > +        /// - `ptr` is valid for writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn set[release](ptr: *mut Self, v: Self) {
> > +            bindings::#call(ptr.cast(), v)
> > +        }
> > +    }
> > +);
> > +
> > +declare_and_impl_atomic_methods!(
> > +    /// Exchange and compare-and-exchange atomic operations
> > +    pub trait AtomicHasXchgOps {
> 
> Same here `AtomicXchgOps` or `AtomicExchangeOps` or `AtomicExchange`?
> (I would prefer to not abbreviate it to `Xchg`)
> 

The "Xchg" -> "Exchange" part seems fine to me.

> > +        /// Atomic exchange.
> > +        ///
> > +        /// Atomically updates `*ptr` to `v` and returns the old value.
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> > +        /// - `ptr` is valid for reads and writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn xchg[acquire, release, relaxed](ptr: *mut Self, v: Self) -> Self {
> > +            bindings::#call(ptr.cast(), v)
> > +        }
> > +
> > +        /// Atomic compare and exchange.
> > +        ///
> > +        /// If `*ptr` == `*old`, atomically updates `*ptr` to `new`. Otherwise, `*ptr` is not
> > +        /// modified, `*old` is updated to the current value of `*ptr`.
> > +        ///
> > +        /// Return `true` if the update of `*ptr` occured, `false` otherwise.
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> > +        /// - `ptr` is valid for reads and writes.
> > +        /// - `old` is aligned to [`align_of::<Self>()`].
> > +        /// - `old` is valid for reads and writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn try_cmpxchg[acquire, release, relaxed](ptr: *mut Self, old: *mut Self, new: Self) -> bool {
> > +            bindings::#call(ptr.cast(), old, new)
> > +        )}
> > +    }
> > +);
> > +
> > +declare_and_impl_atomic_methods!(
> > +    /// Atomic arithmetic operations
> > +    pub trait AtomicHasArithmeticOps {
> 
> Forgot to rename this one to `Add`? I think `AtomicAdd` sounds best for

No, because at the `AtomicImpl` level, it's easy to know whether a type
has all the arithmetic operations or none (also the `Delta` type should
be known). But I don't have opinions on renaming it to `AtomicAddOps` or
`HasAtomicAddOps`.

Regards,
Boqun

> this one.
> 
> ---
> Cheers,
> Benno
> 
> > +        /// Atomic add (wrapping).
> > +        ///
> > +        /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`.
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to `align_of::<Self>()`.
> > +        /// - `ptr` is valid for reads and writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn add[](ptr: *mut Self, v: Self::Delta) {
> > +            bindings::#call(v, ptr.cast())
> > +        }
> > +
> > +        /// Atomic fetch and add (wrapping).
> > +        ///
> > +        /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`, and returns the value of `*ptr`
> > +        /// before the update.
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to `align_of::<Self>()`.
> > +        /// - `ptr` is valid for reads and writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn fetch_add[acquire, release, relaxed](ptr: *mut Self, v: Self::Delta) -> Self {
> > +            bindings::#call(v, ptr.cast())
> > +        }
> > +    }
> > +);
>