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

drain() doc of vector is unclear #73844

Closed
Stargateur opened this issue Jun 28, 2020 · 6 comments · Fixed by #74652
Closed

drain() doc of vector is unclear #73844

Stargateur opened this issue Jun 28, 2020 · 6 comments · Fixed by #74652
Assignees
Labels
A-collections Area: `std::collection` A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Stargateur
Copy link
Contributor

Stargateur commented Jun 28, 2020

I wanted to move out a number of front element from a vector, like my_vec.drain(..x) but I wonder if this is the right way to do this cause doc is unclear, note 1 and note 2 say the opposite each other:

Creates a draining iterator that removes the specified range in the vector and yields the removed items.

Note 1: The element range is removed even if the iterator is only partially consumed or not consumed at all.

Note 2: It is unspecified how many elements are removed from the vector if the Drain value is leaked.

https://doc.rust-lang.org/std/vec/struct.Vec.html#method.drain

This issue has been assigned to @poliorcetics via this comment.

@jonas-schievink
Copy link
Contributor

"partially consumed" means taking a few elements out and then dropping the Drain value. Note 2 talks specifically about leaking the Drain value.

Is there any way to make this more clear in the docs?

@jonas-schievink jonas-schievink added A-collections Area: `std::collection` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Jun 28, 2020
@Stargateur
Copy link
Contributor Author

Stargateur commented Jun 28, 2020

What leaked mean for you ? How would I leak drain struct ? Drop it ?

@jonas-schievink
Copy link
Contributor

No, leaking means specifically destroying a value without running its destructor, for example via mem::forget

@Stargateur
Copy link
Contributor Author

Stargateur commented Jun 28, 2020

So I think write:

The element range is removed even if the iterator is only partially consumed or not consumed at all.

Note: Be aware that If you leak the iterator (eg: mem::forget), It's cancel this property so it is unspecified how many elements are removed from the vector in this case.

Because in the current form the two note look like they apply both whereas it's two difference case, leaked or not. (I'm not sure leak is the good word here for me leak mean leak memory not forget it, so maybe say "without run it's destructor" is more clear)

@yvt
Copy link
Contributor

yvt commented Jul 4, 2020

I myself am familiar with this usage of "to leak", but I don't know how I picked up that, why this is called leaking, or where it's explained in the docs. I suspect that this usage originates from smart pointers, for which destroying them without running destructors would leak the referenced objects. If we generalize this to something that is not a smart pointer, it's called jargon. Although it's convenient to use (in fact, there are more than ten instances of this usage in the standard library), it's not something we can expect to be understood for sure.

For this reason, I agree with @Stargateur. Replacing the word with something like “destroyed without running its destructor” or at least adding a reference to std::mem::forget in parentheses would make sense.

@poliorcetics
Copy link
Contributor

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: `std::collection` A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants