core: make some fundamental atomic functions generic#153407
core: make some fundamental atomic functions generic#153407joboet wants to merge 2 commits intorust-lang:mainfrom
Conversation
|
@bors try jobs=i686-gnu-1,i686-gnu-2 |
This comment has been minimized.
This comment has been minimized.
core: make some fundamental atomic functions generic try-job: i686-gnu-1 try-job: i686-gnu-2
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
💔 Test for a3a652d failed: CI. Failed jobs:
|
f804a05 to
a6a05f8
Compare
|
r? @scottmcm rustbot has assigned @scottmcm. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| pub const fn new(v: T) -> Atomic<T> { | ||
| // SAFETY: | ||
| // `Atomic<T>` is essentially a transparent wrapper around `T`. | ||
| unsafe { transmute_unchecked(v) } |
There was a problem hiding this comment.
Do these really need to be transmutes? Feels like it at least needs better justification that "essentially" -- and to do this AtomicPrimitive should have a # Safety section listing what you can rely on from it in order to rely on that here.
Maybe AtomicPrimitive should have an into_storage or similar method on it instead? Because for lots of these things, new should just be the obvious Atomic { v: UnsafeCell::new(v) }. (Is it only bool that does anything different from that?
There was a problem hiding this comment.
Maybe
AtomicPrimitiveshould have aninto_storageor similar method on it instead? Because for lots of these things,newshould just be the obviousAtomic { v: UnsafeCell::new(v) }. (Is it only bool that does anything different from that?
T::Storage needs to be layout-compatible with T in any case since get_mut relies on that, too. Adding an into_storage without adding functions for references seems inconsistent to me. And in any case, this is a perma-unstable sealed API that will need design changes before becoming unsealed anyway.
I'm somewhat open to expand the safety section though...
| pub const fn into_inner(self) -> T { | ||
| // SAFETY: | ||
| // * `Atomic<T>` is essentially a transparent wrapper around `T`. | ||
| unsafe { transmute_unchecked(self) } |
There was a problem hiding this comment.
...and from_inner too, I guess, on AtomicPrimitive.
CC #130539
This makes the basic
Atomiccreation/destructuring operations (new,into_inner,from_ptr,as_ptr,From<T>,Default) generic overTinstead of defining them for each primitive. All other operations need a bit more design work, so I've left them out for now.