Add Vec::retain_mut #90772
Merged
Add Vec::retain_mut #90772
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GuillaumeGomez
added
the
T-libs-api
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
That sounds very reasonable to me. Feel free to open a tracking issue for it. |
3 tasks
GuillaumeGomez
force-pushed
the
vec-retain-mut
branch
from
f7d0075
to
aed51f5
Compare
|
Opened the tracking issue (#90829) and updated the PR. |
This comment has been minimized.
This comment has been minimized.
GuillaumeGomez
force-pushed
the
vec-retain-mut
branch
from
aed51f5
to
c15b55a
Compare
|
CI is happy too. :) |
|
@bors r+ |
|
📌 Commit c15b55a has been approved by |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 16, 2021
…shtriplett Add Vec::retain_mut This is to continue the discussion started in rust-lang#83218. Original comment was: > Take 2 of rust-lang#34265, since I needed this today. The reason I think why we should add `retain_mut` is for coherency and for discoverability. For example we have `chunks` and `chunks_mut` or `get` and `get_mut` or `iter` and `iter_mut`, etc. When looking for mutable `retain`, I would expect `retain_mut` to exist. It took me a while to find out about `drain_filter`. So even if it provides an API close to `drain_filter`, just for the discoverability, I think it's worth it. cc `@m-ou-se` `@jonas-schievink` `@Mark-Simulacrum`
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 16, 2021
…shtriplett Add Vec::retain_mut This is to continue the discussion started in rust-lang#83218. Original comment was: > Take 2 of rust-lang#34265, since I needed this today. The reason I think why we should add `retain_mut` is for coherency and for discoverability. For example we have `chunks` and `chunks_mut` or `get` and `get_mut` or `iter` and `iter_mut`, etc. When looking for mutable `retain`, I would expect `retain_mut` to exist. It took me a while to find out about `drain_filter`. So even if it provides an API close to `drain_filter`, just for the discoverability, I think it's worth it. cc ``@m-ou-se`` ``@jonas-schievink`` ``@Mark-Simulacrum``
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 16, 2021
…shtriplett Add Vec::retain_mut This is to continue the discussion started in rust-lang#83218. Original comment was: > Take 2 of rust-lang#34265, since I needed this today. The reason I think why we should add `retain_mut` is for coherency and for discoverability. For example we have `chunks` and `chunks_mut` or `get` and `get_mut` or `iter` and `iter_mut`, etc. When looking for mutable `retain`, I would expect `retain_mut` to exist. It took me a while to find out about `drain_filter`. So even if it provides an API close to `drain_filter`, just for the discoverability, I think it's worth it. cc ```@m-ou-se``` ```@jonas-schievink``` ```@Mark-Simulacrum```
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 16, 2021
…shtriplett Add Vec::retain_mut This is to continue the discussion started in rust-lang#83218. Original comment was: > Take 2 of rust-lang#34265, since I needed this today. The reason I think why we should add `retain_mut` is for coherency and for discoverability. For example we have `chunks` and `chunks_mut` or `get` and `get_mut` or `iter` and `iter_mut`, etc. When looking for mutable `retain`, I would expect `retain_mut` to exist. It took me a while to find out about `drain_filter`. So even if it provides an API close to `drain_filter`, just for the discoverability, I think it's worth it. cc ````@m-ou-se```` ````@jonas-schievink```` ````@Mark-Simulacrum````
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 17, 2021
…shtriplett Add Vec::retain_mut This is to continue the discussion started in rust-lang#83218. Original comment was: > Take 2 of rust-lang#34265, since I needed this today. The reason I think why we should add `retain_mut` is for coherency and for discoverability. For example we have `chunks` and `chunks_mut` or `get` and `get_mut` or `iter` and `iter_mut`, etc. When looking for mutable `retain`, I would expect `retain_mut` to exist. It took me a while to find out about `drain_filter`. So even if it provides an API close to `drain_filter`, just for the discoverability, I think it's worth it. cc `````@m-ou-se````` `````@jonas-schievink````` `````@Mark-Simulacrum`````
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 17, 2021
…askrgr Rollup of 8 pull requests Successful merges: - rust-lang#89610 (warn on must_use use on async fn's) - rust-lang#90667 (Improve diagnostics when a static lifetime is expected) - rust-lang#90687 (Permit const panics in stable const contexts in stdlib) - rust-lang#90772 (Add Vec::retain_mut) - rust-lang#90861 (Print escaped string if char literal has multiple characters, but only one printable character) - rust-lang#90884 (Fix span for non-satisfied trivial trait bounds) - rust-lang#90900 (Remove workaround for the forward progress handling in LLVM) - rust-lang#90901 (Improve ManuallyDrop suggestion) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
Eventually my retain_mut crate can go deprecated 🎉 Also do you have a plan to add this to VecDeque as well? |
|
I can as well. I didn't pay attention to the fact that it wasn't only |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Dec 4, 2021
…, r=m-ou-se Implement VecDeque::retain_mut Part of rust-lang#90829. In rust-lang#90772, someone suggested that `retain_mut` should also be implemented on `VecDeque`. I think that it follows the same logic (coherency). So first: is it ok? Second: should I create a new feature for it or can we put it into the same one? r? `@joshtriplett`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is to continue the discussion started in #83218.
Original comment was:
The reason I think why we should add
retain_mutis for coherency and for discoverability. For example we havechunksandchunks_mutorgetandget_mutoriteranditer_mut, etc. When looking for mutableretain, I would expectretain_mutto exist. It took me a while to find out aboutdrain_filter. So even if it provides an API close todrain_filter, just for the discoverability, I think it's worth it.cc @m-ou-se @jonas-schievink @Mark-Simulacrum