Re: [PATCH v7 3/9] rust: sync: atomic: Add ordering annotation types

On Mon, Jul 14, 2025 at 12:10:46PM +0200, Benno Lossin wrote:
> On Mon Jul 14, 2025 at 7:36 AM CEST, Boqun Feng wrote:
> > Preparation for atomic primitives. Instead of a suffix like _acquire, a
> > method parameter along with the corresponding generic parameter will be
> > used to specify the ordering of an atomic operations. For example,
> > atomic load() can be defined as:
> >
> > 	impl<T: ...> Atomic<T> {
> > 	    pub fn load<O: AcquireOrRelaxed>(&self, _o: O) -> T { ... }
> > 	}
> >
> > and acquire users would do:
> >
> > 	let r = x.load(Acquire);
> >
> > relaxed users:
> >
> > 	let r = x.load(Relaxed);
> >
> > doing the following:
> >
> > 	let r = x.load(Release);
> >
> > will cause a compiler error.
> >
> > Compared to suffixes, it's easier to tell what ordering variants an
> > operation has, and it also make it easier to unify the implementation of
> > all ordering variants in one method via generic. The `TYPE` associate
> > const is for generic function to pick up the particular implementation
> > specified by an ordering annotation.
> >
> > Reviewed-by: Alice Ryhl <[email protected]>
> > Signed-off-by: Boqun Feng <[email protected]>
> > ---
> > Benno, please take a good and if you want to provide your Reviewed-by
> > for this one. I didn't apply your Reviewed-by because I used
> > `ordering::Any` instead of `AnyOrdering`, I think you're Ok with it [1],
> > but I could be wrong. Thanks!
> >
> > [1]: https://lore.kernel.org/rust-for-linux/[email protected]/
> 
> > +/// The trait bound for annotating operations that support any ordering.
> > +pub trait Any: internal::Sealed {
> 
> How about we just name this `Ordering`? Because that's what it is :)
> 

Seems OK to me, I then also followed Gary's suggestion:

	https://lore.kernel.org/rust-for-linux/[email protected]/

and dropped `RelaxedOnly` trait.

> That sadly means you can't do
> 
>     fn foo<Ordering: Ordering>() {}
>            --------  ^^^^^^^^ not a trait
>            |
>            found this type parameter
> 
> But you can still do
> 
>     fn foo<O: Ordering>(_: O) {}
> 
> If we don't have the ordering module public and instead re-export from

Keeping ordering mod public helps rustdoc readers to find the module and
read the module documentation (where is the best place to explain each
ordering), and also I made `Relaxed`, `Acquire`, `Release` and `Full`
refer to the module documentation in their doc, making `ordering` mod
private would cause rustdoc issues.

Regards,
Boqun

> atomic, you could also write:
> 
>     fn foo<Ordering: atomic::Ordering>(_: Ordering) {}
> 
> If you want it to be extra clear. What do you think?
> 
> ---
> Cheers,
> Benno
> 
> > +    /// Describes the exact memory ordering.
> > +    const TYPE: OrderingType;
> > +}