Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Atomic*::from_mut. #74532

Merged
merged 3 commits into from
Sep 15, 2020
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
98 changes: 97 additions & 1 deletion library/core/src/sync/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ use self::Ordering::*;
use crate::cell::UnsafeCell;
use crate::fmt;
use crate::intrinsics;
use crate::mem::align_of;

use crate::hint::spin_loop;

Expand Down Expand Up @@ -327,6 +328,27 @@ impl AtomicBool {
unsafe { &mut *(self.v.get() as *mut bool) }
}

/// Get atomic access to a `&mut bool`.
///
/// # Examples
///
/// ```
/// #![feature(atomic_from_mut)]
/// use std::sync::atomic::{AtomicBool, Ordering};
///
/// let mut some_bool = true;
/// let a = AtomicBool::from_mut(&mut some_bool);
/// a.store(false, Ordering::Relaxed);
/// assert_eq!(some_bool, false);
/// ```
#[inline]
#[unstable(feature = "atomic_from_mut", issue = "76314")]
pub fn from_mut(v: &mut bool) -> &Self {
// SAFETY: the mutable reference guarantees unique ownership, and
// alignment of both `bool` and `Self` is 1.
unsafe { &*(v as *mut bool as *mut Self) }
}

/// Consumes the atomic and returns the contained value.
///
/// This is safe because passing `self` by value guarantees that no other threads are
Expand Down Expand Up @@ -820,6 +842,30 @@ impl<T> AtomicPtr<T> {
unsafe { &mut *self.p.get() }
}

/// Get atomic access to a pointer.
///
/// # Examples
///
/// ```
/// #![feature(atomic_from_mut)]
/// use std::sync::atomic::{AtomicPtr, Ordering};
///
/// let mut some_ptr = &mut 123 as *mut i32;
/// let a = AtomicPtr::from_mut(&mut some_ptr);
/// a.store(&mut 456, Ordering::Relaxed);
/// assert_eq!(unsafe { *some_ptr }, 456);
/// ```
#[inline]
#[unstable(feature = "atomic_from_mut", issue = "76314")]
pub fn from_mut(v: &mut *mut T) -> &Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems a little strange to me, since *mut T is already a pointer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like AtomicU16::from_mut will take a &mut u16, AtomicPtr<T>::from_mut should take a &mut to the non-atomic type: &mut *mut T. The already existing AtomicPtr<T>::get_mut() also returns a &mut *mut T: https://doc.rust-lang.org/stable/core/sync/atomic/struct.AtomicPtr.html#method.get_mut

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KodrAus The pointer itself is what atomic here, not what it points to, so when using is you actually use a pointer to the atomic pointer (otherwise you'll be copying the atomic pointer)

let [] = [(); align_of::<AtomicPtr<()>>() - align_of::<*mut ()>()];
Copy link
Member

@RalfJung RalfJung Sep 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will assert that their alignment is equal right? But shouldn't we rather test that Self alignment is no stricter than that of the pointer? You can use the static_assert macro to write any kind of boolean test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, if you know of any other place that uses the such an array length hack to encode a static assertion, please let me know -- we should add a FIXME at least.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_mut already assumes that the alignment of the non-atomic type is ≤ the alignment of the atomic variant. from_mut technically only needs ≥, but things would be quite broken if it was > instead of just =.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the static_assert macro to write any kind of boolean test.

Can I depend on that in core? I don't think core has any dependencies right now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, you cannot directly use it.

But personally I feel like it'd be worth copying it to libcore and then using it there as an internal macro. That said, it seems you will have to rework this entire thing anyway so let's see where that goes.

// SAFETY:
// - the mutable reference guarantees unique ownership.
// - the alignment of `*mut T` and `Self` is the same on all platforms
// supported by rust, as verified above.
unsafe { &*(v as *mut *mut T as *mut Self) }
}

/// Consumes the atomic and returns the contained value.
///
/// This is safe because passing `self` by value guarantees that no other threads are
Expand Down Expand Up @@ -1104,6 +1150,12 @@ impl<T> From<*mut T> for AtomicPtr<T> {
}
}

macro_rules! if_not_8_bit {
(u8, $($tt:tt)*) => { "" };
(i8, $($tt:tt)*) => { "" };
($_:ident, $($tt:tt)*) => { $($tt)* };
}

#[cfg(target_has_atomic_load_store = "8")]
macro_rules! atomic_int {
($cfg_cas:meta,
Expand All @@ -1115,7 +1167,8 @@ macro_rules! atomic_int {
$stable_nand:meta,
$const_stable:meta,
$stable_init_const:meta,
$s_int_type:expr, $int_ref:expr,
$(from_mut: cfg($from_mut_cfg:meta),)?
$s_int_type:literal, $int_ref:expr,
$extra_feature:expr,
$min_fn:ident, $max_fn:ident,
$align:expr,
Expand Down Expand Up @@ -1226,6 +1279,45 @@ assert_eq!(some_var.load(Ordering::SeqCst), 5);
}
}

doc_comment! {
concat!("Get atomic access to a `&mut ", stringify!($int_type), "`.

",
if_not_8_bit! {
$int_type,
concat!(
"**Note:** This function is only available on targets where `",
stringify!($int_type), "` has an alignment of ", $align, " bytes."
)
},
"

# Examples

```
#![feature(atomic_from_mut)]
", $extra_feature, "use std::sync::atomic::{", stringify!($atomic_type), ", Ordering};

let mut some_int = 123;
let a = ", stringify!($atomic_type), "::from_mut(&mut some_int);
a.store(100, Ordering::Relaxed);
assert_eq!(some_int, 100);
```
"),
#[inline]
$(#[cfg($from_mut_cfg)])?
#[unstable(feature = "atomic_from_mut", issue = "76314")]
pub fn from_mut(v: &mut $int_type) -> &Self {
let [] = [(); align_of::<Self>() - align_of::<$int_type>()];
// SAFETY:
// - the mutable reference guarantees unique ownership.
// - the alignment of `$int_type` and `Self` is the
// same on all platforms enabled by `$from_mut_cfg`
// as verified above.
unsafe { &*(v as *mut $int_type as *mut Self) }
}
}

doc_comment! {
concat!("Consumes the atomic and returns the contained value.

Expand Down Expand Up @@ -1984,6 +2076,7 @@ atomic_int! {
stable(feature = "integer_atomics_stable", since = "1.34.0"),
rustc_const_stable(feature = "const_integer_atomics", since = "1.34.0"),
unstable(feature = "integer_atomics", issue = "32976"),
from_mut: cfg(not(target_arch = "x86")),
"i64", "../../../std/primitive.i64.html",
"",
atomic_min, atomic_max,
Expand All @@ -2002,6 +2095,7 @@ atomic_int! {
stable(feature = "integer_atomics_stable", since = "1.34.0"),
rustc_const_stable(feature = "const_integer_atomics", since = "1.34.0"),
unstable(feature = "integer_atomics", issue = "32976"),
from_mut: cfg(not(target_arch = "x86")),
"u64", "../../../std/primitive.u64.html",
"",
atomic_umin, atomic_umax,
Expand All @@ -2020,6 +2114,7 @@ atomic_int! {
unstable(feature = "integer_atomics", issue = "32976"),
rustc_const_stable(feature = "const_integer_atomics", since = "1.34.0"),
unstable(feature = "integer_atomics", issue = "32976"),
from_mut: cfg(not(target_arch = "x86_64")),
"i128", "../../../std/primitive.i128.html",
"#![feature(integer_atomics)]\n\n",
atomic_min, atomic_max,
Expand All @@ -2038,6 +2133,7 @@ atomic_int! {
unstable(feature = "integer_atomics", issue = "32976"),
rustc_const_stable(feature = "const_integer_atomics", since = "1.34.0"),
unstable(feature = "integer_atomics", issue = "32976"),
from_mut: cfg(not(target_arch = "x86_64")),
"u128", "../../../std/primitive.u128.html",
"#![feature(integer_atomics)]\n\n",
atomic_umin, atomic_umax,
Expand Down