-
Notifications
You must be signed in to change notification settings - Fork 24
Description
Proposal
Problem statement
AtomicPtr has unstable methods that allow mutating it like other atomic types:
impl<T> AtomicPtr<T> {
pub fn fetch_ptr_add(&self, val: usize, order: Ordering) -> *mut T;
pub fn fetch_ptr_sub(&self, val: usize, order: Ordering) -> *mut T;
pub fn fetch_byte_add(&self, val: usize, order: Ordering) -> *mut T;
pub fn fetch_byte_sub(&self, val: usize, order: Ordering) -> *mut T;
pub fn fetch_or(&self, val: usize, order: Ordering) -> *mut T;
pub fn fetch_and(&self, val: usize, order: Ordering) -> *mut T;
pub fn fetch_xor(&self, val: usize, order: Ordering) -> *mut T;
}These methods were added in order to remove some int2ptr casts (as part of the strict provenance initiative) that were forced because AtomicPtr did not have these APIs and AtomicUsize was used instead.
fetch_ptr_add and fetch_ptr_sub names are confusing and inconsistent:
ptrimplies that the second argument is a pointer (like in case ofsub_ptrfor example), but it isn'tpointer::{add,sub}don't have theptrpart- the
ptrwas meant to suggest that these methods work in units ofT(as opposed tofetch_byte_add), but it is already the default for pointers
Motivation, use-cases
The motivation is to make the naming less confusing, more consistent and easier to understand.
The search for fetch_(add|sub)\(.+\) as \* in public repositories doesn't yield many results, so I'm assuming using AtomicUsize as AtomicPtr with fetch_{add,sub} is not a common practice and we don't have almost any data of the use cases.
I do not know if this is because using AtomicUsize as AtomicPtr is not nice or if this is because fetch_add for pointers is not that useful.
Solution sketches
I propose just dropping _ptr part, renaming the methods to fetch_add and fetch_sub.
An alternative would be to come out with a better naming, however it would still be inconsistent with pointer methods (and I couldn't come out with anything which makes sense).
Additionally we should provide good documentation that clearly states (with examples) how these methods work.
Links and related work
- Tracking issue: Tracking Issue for arithmetic and certain bitwise ops on
AtomicPtrrust#99108 - Implementation PR: Allow arithmetic and certain bitwise ops on AtomicPtr rust#96935
- Comment that suggested
fetch_addis ambiguous/footgunny and eventually lead tofetch_ptr_addname Allow arithmetic and certain bitwise ops on AtomicPtr rust#96935 (comment)
- Comment that suggested
- Original ACP: Arithmetic and bitwise ops on AtomicPtr #60
- PR that does the proposed rename: Remove
ptrfromAtomicPtr::fetch_{add,sub}method names rust#102970
What happens now?
This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.