Improve the documentation of drain members

[ssomers]

hopefully fixes #92765

[rust-highfive]

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

[yaahc]

The suggested changes look good to me. Happy to approve once you feel it's ready to merge.

[ssomers]

Two more tweaks, in binary_heap.rs & set.rs

[ssomers]

I noticed a commit in #92706 deprecating the use of "yielding" for iterators. Well, this commit happens to remove a few more.

[ssomers]

I included "Leaking" paragraphs here, to keep this advanced topic separate from the main description. It possible to move them further away, to the description of the specific iterator type instead (e.g. from Vec::drain to Vec::Drain), but maybe that's too far away.

[bors]

☔ The latest upstream changes (presumably #94024) made this pull request unmergeable. Please resolve the merge conflicts.

[yaahc]

I like that idea. Because I think those running into situations caused by leak, are not going to cater for every possibility covered by their interpretation of the method's description. They're going to look up what happens now and cater for that.

I'm happy with the current version and ready to merge as is, but did you want to make any additional changes to use the # Current Implementation framing?

[yaahc]

Dang github interface. I can't tell what changes you pushed since you force pushed. Can you describe the changes you made so I know where to look rather than having to re-read everything carefully again? Also, for further changes can you push them as independent commits and only rebase to recombine at the end before we merge¹?

Footnotes

1. or not at all ↩

[ssomers]

I like that idea. Because I think those running into situations caused by leak, are not going to cater for every possibility covered by their interpretation of the method's description. They're going to look up what happens now and cater for that.

I'm happy with the current version and ready to merge as is, but did you want to make any additional changes to use the # Current Implementation framing?

I forgot to propagate the change of "disappears" with "goes out of scope" in some places (as The Book calls it), fixed now.

It's fine for me now. As to the current implementation of leakage, I think for Vec::drain that is actually more complicated to describe than what the description now says (I thought it simply cleared the vector, but it leaves the part up to the start of the range), and hardly helps. It seems only useful for String::drain, but then I think why not leave out the possibilities of what else we might one day choose, which would be back to the initial wording in #93769, so I left it as is.

[ssomers]

Dang github interface. I can't tell what changes you pushed since you force pushed.

I've often wondered how reviewers review, and I think I remember eventually applying a trick in an URL to see someone else's PR evolve or so, but I forgot how.

Can you describe the changes you made

I did, even before reading your request. Not saying that I deserve a cookie now, but at least I might be pardoned for changing this PR so often

push them as independent commits and only rebase to recombine at the end before we merge

Seems easier for everyone to me, but long ago I got the opposite message. I suppose because then the reviewer doesn't have to return after the recombine and there's no risk that the recombine isn't what it's supposed to be. It also allows using the github buttons to accept inline small changes.

[yaahc]

Seems easier for everyone to me, but long ago I got the opposite message. I suppose because then the reviewer doesn't have to return after the recombine and there's no risk that the recombine isn't what it's supposed to be. It also allows using the github buttons to accept inline small changes.

Odd, the only time I've seen pushback is when I do a merge commit of master into a PR branch.

I did, even before reading your request. Not saying that I deserve a cookie now, but at least I might be pardoned for changing this PR so often

loool, another race condition¹ it seems.

It's fine for me now. As to the current implementation of leakage, I think for Vec::drain that is actually more complicated to describe than what the description now says (I thought it simply cleared the vector, but it leaves the part up to the start of the range), and hardly helps. It seems only useful for String::drain, but then I think why not leave out the possibilities of what else we might one day choose, which would be back to the initial wording in #93769, so I left it as is.

sounds good! thank you ^_^

@bors r+

Footnotes

1. fearless concurrency my ass ↩

[bors]

📌 Commit a677e60 has been approved by yaahc