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

Refactor unsafe implementation of std::mem::replace #117189

Closed
wants to merge 2 commits into from
Closed

Refactor unsafe implementation of std::mem::replace #117189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a81b92
Refactor unsafe implementation of std::mem::replace
chfogelman Oct 25, 2023
e1fe09c
Update aliasing_mut4 miri test
chfogelman Oct 25, 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
12 changes: 3 additions & 9 deletions library/core/src/mem/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,15 +908,9 @@ pub fn take<T: Default>(dest: &mut T) -> T {
#[must_use = "if you don't need the old value, you can just assign the new value directly"]
#[rustc_const_unstable(feature = "const_replace", issue = "83164")]
#[cfg_attr(not(test), rustc_diagnostic_item = "mem_replace")]
pub const fn replace<T>(dest: &mut T, src: T) -> T {
// SAFETY: We read from `dest` but directly write `src` into it afterwards,
// such that the old value is not duplicated. Nothing is dropped and
// nothing here can panic.
unsafe {
let result = ptr::read(dest);
ptr::write(dest, src);
result
}
pub const fn replace<T>(dest: &mut T, mut src: T) -> T {
swap(dest, &mut src);
src
Copy link
Member

Choose a reason for hiding this comment

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

You were essentially trying to revert #83022 here.

Maybe we should have a comment explaining why this is not calling swap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think a comment is a good idea. It's not immediately clear that swap was being avoided for performance reasons.

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to make a PR for that? :)

}

/// Disposes of a value.
Expand Down
6 changes: 4 additions & 2 deletions src/tools/miri/tests/fail/both_borrows/aliasing_mut4.tree.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: write access through <TAG> is forbidden
--> RUSTLIB/core/src/mem/mod.rs:LL:CC
|
LL | ptr::write(dest, src);
| ^^^^^^^^^^^^^^^^^^^^^ write access through <TAG> is forbidden
LL | ptr::write(x, b);
| ^^^^^^^^^^^^^^^^ write access through <TAG> is forbidden
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental
= help: the accessed tag <TAG> is foreign to the protected tag <TAG> (i.e., it is not a child)
Expand All @@ -19,6 +19,8 @@ help: the protected tag <TAG> was created here, in the initial state Frozen
LL | pub fn safe(x: &i32, y: &mut Cell<i32>) {
| ^
= note: BACKTRACE (of the first span):
= note: inside `std::mem::swap_simple::<i32>` at RUSTLIB/core/src/mem/mod.rs:LL:CC
= note: inside `std::mem::swap::<i32>` at RUSTLIB/core/src/mem/mod.rs:LL:CC
= note: inside `std::mem::replace::<i32>` at RUSTLIB/core/src/mem/mod.rs:LL:CC
= note: inside `std::cell::Cell::<i32>::replace` at RUSTLIB/core/src/cell.rs:LL:CC
= note: inside `std::cell::Cell::<i32>::set` at RUSTLIB/core/src/cell.rs:LL:CC
Expand Down
Loading