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.

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

mem::replace and mem::swap say they don't copy, but they do #35935

Closed
durka opened this issue Aug 23, 2016 · 6 comments
Closed

mem::replace and mem::swap say they don't copy, but they do #35935

durka opened this issue Aug 23, 2016 · 6 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@durka
Copy link
Contributor

durka commented Aug 23, 2016

mem::replace:

Replaces the value at a mutable location with a new one, returning the old value, without deinitializing or copying either one.

mem::swap:

Swap the values at two mutable locations of the same type, without deinitializing or copying either one.

However, replace calls swap which uses copy_nonoverlapping, making the docs technically false. Should it say "without cloning" or "without deep-copying" instead?

@hanna-kruppe
Copy link
Contributor

It moves. Moves are in the end implemented by copying bytes, but that's not what "copy" means in most Rust contexts.

@bluss
Copy link
Member

bluss commented Aug 23, 2016

They don't copy in Rust semantics, but we should rewrite them to talk about move semantics, which will make it easier for everyone to understand.

These functions are not unsafe or tricky or anything, so we really don't need to reassure users about how they work (they will obey Rust's regular rules, of course)?

replace

Move out the value at a mutable location and return it, and move the new value in to replace it.

swap

Swap the values at two mutable locations.

Not sure what to even say about swap. Maybe:

Each value is moved to the other's location.

@brson brson added A-docs E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Aug 24, 2016
@brson
Copy link
Contributor

brson commented Aug 24, 2016

All that needs to happen here is update docs per @bluss.

@durka
Copy link
Contributor Author

durka commented Aug 24, 2016

Yeah I wasn't saying the implementations had any problem.

matthew-piziak added a commit to matthew-piziak/rust that referenced this issue Sep 9, 2016
This update was prompted by @durka and the exact words are from @bluss.

Part of rust-lang#35935.

r? @brson
@kmcallister
Copy link
Contributor

I also changed this wording in #36357.

@steveklabnik
Copy link
Member

Thanks to @kmcallister 's work, giving this a close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

6 participants