Simplify the unix Weak functionality
#58442
Conversation
If we add a terminating NUL to the name in the `weak!` macro, then `fetch()` can use `CStr::from_bytes_with_nul()` instead of `CString`.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @RalfJung |
| mem::transmute::<&AtomicUsize, Option<&F>>(&self.addr) | ||
| match self.addr.load(Ordering::SeqCst) { | ||
| 0 => None, | ||
| addr => Some(mem::transmute_copy::<usize, F>(&addr)), |
There was a problem hiding this comment.
Please add a comment saying that the sizes are equal because we asserted this earlier. (Took me a bit to see that. As I said, I don't know this code and feel somewhat uncomfortable reviewing it.)
| use marker; | ||
| use mem; | ||
| use sync::atomic::{AtomicUsize, Ordering}; | ||
|
|
||
| macro_rules! weak { | ||
| (fn $name:ident($($t:ty),*) -> $ret:ty) => ( | ||
| static $name: ::sys::weak::Weak<unsafe extern fn($($t),*) -> $ret> = | ||
| ::sys::weak::Weak::new(stringify!($name)); | ||
| ::sys::weak::Weak::new(concat!(stringify!($name), '\0')); | ||
| ) | ||
| } |
There was a problem hiding this comment.
Shouldn't something about this abstraction be unsafe? Like, either the constructor or get or so?
Seems we are using it only with unsafe fn, but the Weak type itself can't know that.
There was a problem hiding this comment.
Sure, I think it would make sense for the constructor to be unsafe, which the macro can hide knowing this is unsafe fn. All uses do go through this macro already though.
|
The concurrency aspect looks fine, but someone who knows the library around this should review. This doesn't look like a safe abstraction to me, so it has to be reviewed knowing how it is used. |
|
r? @alexcrichton perhaps? |
|
📌 Commit 33d80bf has been approved by |
bors
added
S-waiting-on-bors
Simplify the unix `Weak` functionality - We can avoid allocation by adding a NUL to the function name. - We can get `Option<F>` directly, rather than aliasing the inner `AtomicUsize`.
Simplify the unix `Weak` functionality - We can avoid allocation by adding a NUL to the function name. - We can get `Option<F>` directly, rather than aliasing the inner `AtomicUsize`.
Rollup of 6 pull requests Successful merges: - #57364 (Improve parsing diagnostic for negative supertrait bounds) - #58183 (Clarify guarantees for `Box` allocation) - #58442 (Simplify the unix `Weak` functionality) - #58454 (Refactor Windows stdio and remove stdin double buffering ) - #58511 (Const to op simplification) - #58642 (rustdoc: support methods on primitives in intra-doc links) Failed merges: r? @ghost
Option<F>directly, rather than aliasing the innerAtomicUsize.