-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
std: Add more docs to std::mem::replace
.
#13797
Conversation
std::mem::replace
.std::mem::replace
.
std::mem::replace
.std::mem::replace
.
😍 |
* This is primarily used to work around various limitations of the borrow | ||
* checker. For example, some method may want to consume `self.to_be_consumed` | ||
* field and give it to `self.alter_others()` method which wouldn't read or | ||
* alter that field but would alter others. The normal approach wouldn't work: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this could be re-worded without accusing the borrow checker of faultiness? I personally primarily use this function because it's a fundamental building block of ownership, not necessarily to work around the borrow checker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I tried some variations on the topic sentence (e.g. "this is used to move values around the mutable location without any invalidation") but that was ultimately unsuccessful. After all, most uses of std::mem::replace
in the standard library fit to this description, in such that they can be replaced with plain assignments if the borrow checker is a lot smarter. Any suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps something along the lines of:
This is primarily used for transferring and swapping ownership of a value in a mutable slot. For example, this function allows consumption of one field of a struct by replacing it with another value. The normal approach doesn't always work:
I wouldn't always classify these as a borrow checker limitation (although some definitely may be). Your example below is not sound if you can consume ownership of one field and then call a method on the structure in general (which may attempt to read the field).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds better, I'll try that. The soundness problem can be prohibited by much finer-grained static analyses though. I feel any static analyses have their own limitations and users eventually have to learn about them (much like that they have to learn about the characteristics of type system).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's true, @nikomatsakis has been talking recently about allowing the following, which I would be quite excited about! (an example of a limitation of today's borrow checker).
fn replace<T>(slot: &mut T, t: T) -> T {
let ret = *slot;
*slot = t;
return ret;
}
This looks great, thanks! I always love some more delicious documentation. |
* } | ||
* ``` | ||
* | ||
* The borrow checker does not recognize that `self.alter_others()` wouldn't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this just be "the compiler", because it's not strictly the borrow checker that's doing this check. (Maybe "In general, the compiler cannot recognize that self.alter_others() doesn't touch
self.to_be_consumed`".)
I've changed the example as per the suggestion by @huonw. |
…ichton Inspired by @steveklabnik's [comment](http://www.reddit.com/r/rust/comments/240p9s/eli5_stdmemreplace/ch2gxw8), this PR adds the practical use cases to the documentation of `std::mem::replace`. Caveat: We need a `compile-fail` equivalent for doctest. :p
Fix rust-lang#13795 changelog: [`shadow_same`]: detect shadowing as a pattern field
Inspired by @steveklabnik's comment, this PR adds the practical use cases to the documentation of
std::mem::replace
.Caveat: We need a
compile-fail
equivalent for doctest. :p