ACP: Referencing existing safety contracts in unsafe API documentation

Proposal

Problem statement

The standard library contains unsafe APIs whose safety requirements are identical to, derived from, or composed of the safety requirements of related APIs.

For example, many unsafe methods on NonNull<T> correspond to unsafe functions with the same name in the ptr module. And the unsafe methods on NonNull<T> describe the safety requirements by referring to the corresponding ptr functions:

    /// See [`ptr::write`] for safety concerns and examples.
    ///
    /// [`ptr::write`]: crate::ptr::write()
    #[rustc_const_stable(feature = "const_ptr_write", since = "1.83.0")]
    pub const unsafe fn write(self, val: T)
    where
        T: Sized,
    {
        // SAFETY: the caller must uphold the safety contract for `write`.
        unsafe { ptr::write(self.as_ptr(), val) }
    }

Some unsafe APIs only use a reference to the authoritative safety documentation:

/// path: alloc/alloc.rs
/// # Safety
///
/// See [`GlobalAlloc::dealloc`].
#[stable(feature = "global_alloc", since = "1.28.0")]
#[inline]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub unsafe fn dealloc(ptr: *mut u8, layout: Layout) {
    // SAFETY: Upheld by caller.
    unsafe { dealloc_nonnull(NonNull::new_unchecked(ptr), layout) }
}

The two cases are acceptable because they both indicate the safety requirements. However, the safety documentation of the following APIs could be made more explicit:

• DisjointBitOr::disjoint_bitor
• FunnelShift::unchecked_funnel_shl
• FunnelShift::unchecked_funnel_shr

Also, the documentation of the atomic operation APIs in intrinsic module could likewise be improved.

• atomic_and
• atomic_cxchg
• atomic_cxchgweak
• atomic_load
• atomic_max
• atomic_min
• atomic_nand
• atomic_or
• atomic_store
• atomic_umax
• atomic_umin
• atomic_xadd
• atomic_xchg
• atomic_xor
• atomic_xsub

Motivating examples or use cases

The first three APIs use the same discription which don't indicate safety directly:

    /// See [`super::unchecked_funnel_shl`]; we just need the trait indirection to handle
    /// different types since calling intrinsics with generics doesn't work.
    unsafe fn unchecked_funnel_shl(self, right: Self, shift: u32) -> Self;

Also, the trait methods use different parameter name from the corresponding intrinsic functions.

The atomic operation APIs in intrinsic are marked unsafe, but their documentation does not state their safety requirements:

/// Loads the current value of the pointer.
/// `T` must be an integer or pointer type.
///
/// The stabilized version of this intrinsic is available on the
/// [`atomic`] types via the `load` method. For example, [`AtomicBool::load`].
#[rustc_intrinsic]
#[rustc_nounwind]
pub const unsafe fn atomic_load<T: Copy, const ORD: AtomicOrdering, const VOLATILE: bool>(
    src: *const T,
) -> T;

A simple idea is that since this function is unsafe, a # Safety section needs to be added to explain why it is unsafe(even without a # Safety section, it needs a reason about why it is unsafe).

For these atomic operations, they pass raw pointer as argument so the caller need to ensure that this pointer is valid. When VOLATILE is false, it is equivalent to Atomic<T>::from_ptr() followed by Atomic<T>::load().

Solution sketch

For the first three APIs, we can add a # Safety section that makes it explicit that the linked documentation defines the safety contract:

    /// We just need the trait indirection to handle different
    /// types since calling intrinsics with generics doesn't work.
    ///
    /// # Safety
    /// See [`super::disjoint_bitor`].
    unsafe fn disjoint_bitor(self, other: Self) -> Self;

Optionaly, the documentation may also specify the correspondence between parameters. For example, self corresponds to a, and other corresponds to b.

For these atomic APIs, the # Safety section can refer to the corresponding atomic APIs instead of duplicating their safety requirements:

/// Loads the current value of the pointer.
/// `T` must be an integer or pointer type.
///
/// # Safety
///
/// * If `VOLATILE` is `true`, this is equivalent to [Atomic::load_volatile].
///   Refer to the documentation of that method for safety requirements.
///
/// * If `VOLATILE` is `false`, this is equivalent to [Atomic::from_ptr] followed
///   by [Atomic::load]. Refer to the documentation of [Atomic::from_ptr] for safety requirements.
///
/// The stabilized version of this intrinsic is available on the
/// [`atomic`] types via the `load` method. For example, [`AtomicBool::load`].
///
/// [Atomic::load_volatile]: AtomicI32::load_volatile
/// [Atomic::from_ptr]: AtomicI32::from_ptr
/// [Atomic::load]: AtomicI32::load
#[rustc_intrinsic]
#[rustc_nounwind]
pub const unsafe fn atomic_load<T: Copy, const ORD: AtomicOrdering, const VOLATILE: bool>(
    src: *const T,
) -> T;

Alternatives

Adding a # Safety section is a small documentation change. But I think it would help make the presentation of safety requirements throughout the standard library more consistent.

An ideal situation is: For every unsafe API, we can use a # Safety to denote the point this API must ensure or must not violate; At every call site, we can discharge every point to ensure the soundness. If the same safety requirement already exists in other place, just refer it in # Safety is easy.

Links and related work

I have provided two PRs concerning the APIs discussed above:

• Add safety section for DisjointBitor::disjoint_bitor
• Add safety section for atomic_load/store

I have also opened several PRs that improve safety documentation in the Rust
standard library. All of the following PRs have been merged:

• Add safety comments in alloc::Wtf8
• doc: document safety requirements for core WTF-8
• Clarify safety requirements for SIMD shl/shr and masked load/store
• Fix safety doc in intrinsics::simd
• Add align requirement in _mm_stream_si32 and _mm_stream_si64
• Add supplementary information for get_unchecked(mut)
• Fix inconsistent safety requirement in VecDeque::nonoverlapping_ranges
• Add safety section for SliceIndex::get_unchecked(mut)

Start by locating the listed DisjointBitOr, FunnelShift, and atomic intrinsic APIs in the core intrinsics sources, and compare them with the linked corresponding atomic and pointer APIs. Done means the unsafe APIs explicitly identify their safety contracts through appropriate # Safety sections and references without duplicating unrelated requirements.