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.

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

Jump to bottom

clarify safety documentation of ptr::swap and ptr::copy #114794

Merged
merged 3 commits into from
Sep 5, 2023
Merged

clarify safety documentation of ptr::swap and ptr::copy #114794

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9c15c5
clarify safety documentation of ptr::swap and ptr::copy
RalfJung Aug 14, 2023
0188b9c
try to clarify wording
RalfJung Aug 21, 2023
4684ffa
if -> when
RalfJung Sep 5, 2023
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
3 changes: 3 additions & 0 deletions library/core/src/intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2709,6 +2709,9 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
///
/// * `dst` must be [valid] for writes of `count * size_of::<T>()` bytes.
///
/// * `src` must remain valid for reads even after `dst` is written, and vice versa.
/// (In other words, there cannot be aliasing restrictions on the use of these pointers.)
///
Copy link
Member

@m-ou-se m-ou-se Aug 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't quite understand what this meant in practice until I checked out #81005.

Maybe there's a different way of wording this?

Would it work to not add this bullet point, but instead add "during the entire call to swap()" to the first two bullet points? (Maybe with a clarifying note saying that this means that the src and dst regions may not overlap.)

Copy link
Member Author

Choose a reason for hiding this comment

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

They may overlap though, the pointers just must allow overlapping access. So for instance passing ptr and ptr.add(N) is fine even if things overlap. However passing &mut *ptr twice generates two fresh mutable references and now if things overlap, that lead to UB due to aliasing violations.

It's hard to precisely say this while the aliasing model is still being determined.

add "during the entire call to swap()" to the first two bullet points?

If we do that I think we have to say what happens during the call. So we could say something like

  • src must be [valid] for reads of count * size_of::<T>() bytes, and must remain valid even if dst is written to for count * size_of::<T>() bytes.

But that seems very similar to the current wording, so I am not sure if it is clear enough.

/// * Both `src` and `dst` must be properly aligned.
///
/// Like [`read`], `copy` creates a bitwise copy of `T`, regardless of
Expand Down
3 changes: 3 additions & 0 deletions library/core/src/ptr/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,9 @@ pub const fn slice_from_raw_parts_mut<T>(data: *mut T, len: usize) -> *mut [T] {
///
/// * Both `x` and `y` must be [valid] for both reads and writes.
///
/// * `x` must remain valid for reads and writes even after `y` is read/written, and vice versa.
/// (In other words, there cannot be aliasing restrictions on the use of these pointers.)
///
/// * Both `x` and `y` must be properly aligned.
///
/// Note that even if `T` has size `0`, the pointers must be non-null and properly aligned.
Expand Down
Loading