Refactor unsafe implementation of std::mem::replace #117189
Conversation
The function now defers to mem::swap, which both eliminates the use of `unsafe` in the function, and also allows it (and mem::take) to benefit from any performance enhancements to mem::swap, such as the current use of ptr::swap_nonoverlapping for large types.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cuviper (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
The Miri subtree was changed cc @rust-lang/miri |
|
Seems that there are more performance implications than I expected, as evidenced by the failure of |
| } | ||
| pub const fn replace<T>(dest: &mut T, mut src: T) -> T { | ||
| swap(dest, &mut src); | ||
| src |
There was a problem hiding this comment.
You were essentially trying to revert #83022 here.
Maybe we should have a comment explaining why this is not calling swap?
There was a problem hiding this comment.
I think a comment is a good idea. It's not immediately clear that swap was being avoided for performance reasons.
There was a problem hiding this comment.
Do you want to make a PR for that? :)
…t, r=thomcc Explain implementation of mem::replace This adds a comment to explain why `mem::replace` is not implemented in terms of `mem::swap` to prevent [naïfs like me](rust-lang#117189) from trying to "fix" it.
Rollup merge of rust-lang#117243 - chfogelman:replace-not-swap-comment, r=thomcc Explain implementation of mem::replace This adds a comment to explain why `mem::replace` is not implemented in terms of `mem::swap` to prevent [naïfs like me](rust-lang#117189) from trying to "fix" it.
The function now defers to mem::swap, which both eliminates the use of
unsafein the function, and also allows it (and mem::take) to benefit from any performance enhancements to mem::swap, such as the current use of ptr::swap_nonoverlapping for large types.