Skip to content

Update documentation of as_ptr function of Atomic$Int to clarify circumstances of usage #139637

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 21 additions & 24 deletions library/core/src/sync/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1113,15 +1113,12 @@ impl AtomicBool {

/// Returns a mutable pointer to the underlying [`bool`].
///
/// Doing non-atomic reads and writes on the resulting boolean can be a data race.
/// This method is mostly useful for FFI, where the function signature may use
/// `*mut bool` instead of `&AtomicBool`.
/// Note that doing non-atomic reads or writes on the resulting integer can be
/// Undefined Behavior due to a data race; see the [memory model section] for further information.
///
/// Returning an `*mut` pointer from a shared reference to this atomic is safe because the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wanted to clarify one thing, would it be a good idea to note about interior mutability too?

Copy link
Member

Choose a reason for hiding this comment

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

No, I would say interior mutability is an implementation detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

/// atomic types work with interior mutability. All modifications of an atomic change the value
/// through a shared reference, and can do so safely as long as they use atomic operations. Any
/// use of the returned raw pointer requires an `unsafe` block and still has to uphold the same
/// restriction: operations on it must be atomic.
/// This method is mostly useful for FFI, where the function signature may use
/// `*mut bool` instead of `&AtomicBool`. All modifications of an atomic change the value
/// through a shared reference, and can do so safely as long as they use atomic operations.
///
/// # Examples
///
Expand All @@ -1134,11 +1131,14 @@ impl AtomicBool {
/// }
///
/// let mut atomic = AtomicBool::new(true);
///
/// // SAFETY: `my_atomic_op` only uses atomic operations so it will not lead to a data race.
/// unsafe {
/// my_atomic_op(atomic.as_ptr());
/// }
/// # }
/// ```
/// [memory model section]: self#memory-model-for-atomic-accesses
#[inline]
#[stable(feature = "atomic_as_ptr", since = "1.70.0")]
#[rustc_const_stable(feature = "atomic_as_ptr", since = "1.70.0")]
Expand Down Expand Up @@ -2306,15 +2306,12 @@ impl<T> AtomicPtr<T> {

/// Returns a mutable pointer to the underlying pointer.
///
/// Doing non-atomic reads and writes on the resulting pointer can be a data race.
/// This method is mostly useful for FFI, where the function signature may use
/// `*mut *mut T` instead of `&AtomicPtr<T>`.
/// Note that doing non-atomic reads or writes on the resulting integer can be
/// Undefined Behavior due to a data race; see the [memory model section] for further information.
///
/// Returning an `*mut` pointer from a shared reference to this atomic is safe because the
/// atomic types work with interior mutability. All modifications of an atomic change the value
/// through a shared reference, and can do so safely as long as they use atomic operations. Any
/// use of the returned raw pointer requires an `unsafe` block and still has to uphold the same
/// restriction: operations on it must be atomic.
/// This method is mostly useful for FFI, where the function signature may use
/// `*mut *mut T` instead of `&AtomicPtr<T>`. All modifications of an atomic change the value
/// through a shared reference, and can do so safely as long as they use atomic operations.
///
/// # Examples
///
Expand All @@ -2333,6 +2330,7 @@ impl<T> AtomicPtr<T> {
/// my_atomic_op(atomic.as_ptr());
/// }
/// ```
/// [memory model section]: self#memory-model-for-atomic-accesses
#[inline]
#[stable(feature = "atomic_as_ptr", since = "1.70.0")]
#[rustc_const_stable(feature = "atomic_as_ptr", since = "1.70.0")]
Expand Down Expand Up @@ -3398,15 +3396,13 @@ macro_rules! atomic_int {

/// Returns a mutable pointer to the underlying integer.
///
/// Doing non-atomic reads and writes on the resulting integer can be a data race.
/// Note that doing non-atomic reads or writes on the resulting integer can be
/// Undefined Behavior due to a data race; see the [memory model section] for further information.
///
/// This method is mostly useful for FFI, where the function signature may use
#[doc = concat!("`*mut ", stringify!($int_type), "` instead of `&", stringify!($atomic_type), "`.")]
///
/// Returning an `*mut` pointer from a shared reference to this atomic is safe because the
/// atomic types work with interior mutability. All modifications of an atomic change the value
/// through a shared reference, and can do so safely as long as they use atomic operations. Any
/// use of the returned raw pointer requires an `unsafe` block and still has to uphold the same
/// restriction: operations on it must be atomic.
/// All modifications of an atomic change the value through a shared reference, and can do so safely
/// as long as they use atomic operations.
Copy link
Member

Choose a reason for hiding this comment

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

I don't get the first part of this sentence... what else would they do?

Copy link
Member

@RalfJung RalfJung Apr 22, 2025

Choose a reason for hiding this comment

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

Github is being silly and doesn't let me make this a suggestion, so let me post this manually -- how about:

            /// Note that doing non-atomic reads or writes on the resulting integer can be
            /// Undefined Behavior due to a data race; see the [memory model section] for further information.
            ///
            /// This method is mostly useful for FFI, where the function signature may use
            #[doc = concat!("`*mut ", stringify!($int_type), "` instead of `&", stringify!($atomic_type), "`.")]

As I said before, this should link to the module-level memory model docs. You can probably use an intra-doc link like that... crate::sync::atomic#memory-model-for-atomic-accesses should work.

Comment on lines +3404 to +3405
Copy link
Member

Choose a reason for hiding this comment

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

What is the point of this last sentence? It did not exist in my suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was not added. That is the consequence of removing the sentences surrounding it in this commit:

  1. The "interior mutability" detail
  2. The "unsafe" block detail

Copy link
Member

Choose a reason for hiding this comment

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

I guess I wasn't clear enough then when posting my suggestion -- please remove this sentence. I've already told you a week ago that by removing the sentence before this, you made this sentence here entirely void of context and confusing.

///
/// # Examples
///
Expand All @@ -3420,12 +3416,13 @@ macro_rules! atomic_int {
///
#[doc = concat!("let atomic = ", stringify!($atomic_type), "::new(1);")]
///
/// // SAFETY: Safe as long as `my_atomic_op` is atomic.
/// // SAFETY: `my_atomic_op` only uses atomic operations so it will not lead to a data race.
/// unsafe {
/// my_atomic_op(atomic.as_ptr());
/// }
/// # }
/// ```
/// [memory model section]: self#memory-model-for-atomic-accesses
#[inline]
#[stable(feature = "atomic_as_ptr", since = "1.70.0")]
#[rustc_const_stable(feature = "atomic_as_ptr", since = "1.70.0")]
Expand Down
Loading