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

On Mon Jul 14, 2025 at 4:59 PM CEST, Boqun Feng wrote:
> 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.

Sounds good.

>> 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.

You could move those docs to the `Ordering` trait :) But I think having
an ordering module is fine.

---
Cheers,
Benno