Skip to content

ACP: Referencing existing safety contracts in unsafe API documentation #887

Open

Description

@yilin0518

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:

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

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:

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

Activity

  1. added
    api-change-proposalA proposal to add or alter unstable APIs in the standard libraries
    on Sep 19, 2026
  2. RalfJung commented on Sep 20, 2026

    @RalfJung
    Member

    Note that these are all internal APIs. So this isn't really an ACP (API change proposal) as no public API is affected.

    It's more a question for t-libs about how they'd like to do safety comments on internal unsafe functions.

  3. hxuhack commented on Sep 25, 2026

    @hxuhack
    Note that these are all internal APIs. 
    

    How can we determine whether an API is considered internal or public?

    If an API is intended for internal use only, why not use pub(crate) for it, or introduce a new visibility mechanism (e.g., pub(std)) if this distinction cannot be expressed with the current visibility system?

  4. RalfJung commented on Sep 25, 2026

    @RalfJung
    Member

    How can we determine whether an API is considered internal or public?

    If you need to enable an internal feature to call the API, it's internal.
    The compiler will tell you when you enable an internal feature.
    core_intrinsics is an internal feature.

  5. hxuhack commented on Sep 25, 2026

    @hxuhack

    I understand that they can only be accessed by enabling the feature gate, but this applies not only to the standard library, but also to third-party crates. So I'm not sure whether it is appropriate to consider them internal APIs.

  6. RalfJung commented on Sep 25, 2026

    @RalfJung
    Member
  7. hxuhack commented on Sep 25, 2026

    @hxuhack

    It might be because I wasn't aware that APIs protected by compiler gates are considered internal.

  8. RalfJung commented on Sep 25, 2026

    @RalfJung
    Member

    That is not what I said. I said

    If you need to enable an internal feature to call the API, it's internal.
    The compiler will tell you when you enable an internal feature.
    core_intrinsics is an internal feature.

    The compiler prints this when you enable core_intrinsics:

    warning: the feature `core_intrinsics` is internal to the compiler or standard library
     --> src/main.rs:1:12
      |
    1 | #![feature(core_intrinsics)]
      |            ^^^^^^^^^^^^^^^
      |
      = note: using it is strongly discouraged
      = note: `#[warn(internal_features)]` on by default
    

    So, it's easy to tell that these APIs are internal.

  9. hxuhack commented on Sep 25, 2026

    @hxuhack

    Thanks, that clarifies the distinction.
    However, it might not be obvious from the API documentation alone that these APIs are considered internal. Is that true?

  10. RalfJung commented on Sep 25, 2026

    @RalfJung
    Member
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions