Skip to content

Clarify as_uninit_mut may point to uninitialized memory #158804

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.

Sign up for GitHub

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
yilin0518 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Clarify as_uninit_mut may point to uninitialized memory #158804

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions library/core/src/ptr/mut_ptr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,8 @@ impl<T: PointeeSized> *mut T {
///
/// When calling this method, you have to ensure that *either* the pointer is null *or*
/// the pointer is [convertible to a reference](crate::ptr#pointer-to-reference-conversion).
/// Note that because the created reference is to `MaybeUninit<T>`, the
/// source pointer can point to uninitialized memory.

@hkBst hkBst Jul 8, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This formulation seems backwards to me, and this information also seems already to be included in "In contrast to [as_mut], this does not require that the value has to be initialized." which is a few lines above this.

View changes since the review

@yilin0518 yilin0518 Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The purpose of the change I made is let the safety documentation of this API consistent with other APIs(*mut T::as_uninit_ref, *const T::as_uninit_ref and std::ptr::NonNull::as_uninit_mut). All these APIs has this pattern in their safety documentation. Although "In contrast to [as_mut], this does not require that the value has to be initialized." has implied the uninitiazlition, it seems that explicitly emphasize this in #Safety is better. And thank you for your review and feedback!

@hkBst hkBst Jul 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. Also, you're welcome to the review and thanks for trying to improve the docs and their consistency in particular.

I'm not sure I agree this needs to be repeated in the SAFETY section, as it is not a safety condition that will have to be dispatched in a safety comment using these methods. The MaybeUninit type already makes it very difficult to accidentally mishandle MaybeUninit<T> as T.

Anyway, assuming something like this is included in the SAFETY section, I would prefer a reformulation. From "Note that because the created reference is to MaybeUninit<T>, the source pointer can point to uninitialized memory." to perhaps one of:

  • Remember that the returned reference may refer to an uninitialized T.
  • Note: returned reference may refer to an uninitialized T.
    Or if the point is instead about the argument:
  • self may point to uninitialized memory.

What do you think?

@yilin0518 yilin0518 Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

From "Note that because the created reference is to MaybeUninit<T>, the source pointer can point to uninitialized memory." to perhaps one of:

  • Remember that the returned reference may refer to an uninitialized T.
  • Note: returned reference may refer to an uninitialized T.
    Or if the point is instead about the argument:
  • self may point to uninitialized memory.

What do you think?

Well I think reformulation is optional, but adding the uninitialization hint in SAFETY section is better than in other place, because SAFETY section needs to remind developers that what they should care about when using this API.

But I think the first discription is great if needing reformulation. It is more concise. And the other point we should care is that the documentation of these APIs need modification if reformulation is accepted. I can finfish the work and apply in this PR.

///
/// # Panics during const evaluation
///
Expand Down
Loading