Add Vec::retain_mut #34265

shepmaster · 2016-06-14T02:30:53Z

No description provided.

rust-highfive · 2016-06-14T02:30:58Z

(rust_highfive has picked a reviewer for you, use r? to override)

shepmaster · 2016-06-14T02:32:58Z

I really wanted to just change Vec::retain's closure to accept a &mut T, but that is sadly backwards incompatible. It's a shame that you can't pattern-match a mutable reference with just a &.

Since I had the main bulk of the work, I figured I'd send in a PR and see what we thought of it. Does something like this require a RFC, or is it tiny enough to sneak in?

shepmaster · 2016-06-14T02:40:22Z

For reference, this was originally spurred from this Stack Overflow question. Conceptually, if you have a Vec<Monster> and want to apply some damage to all the monsters and remove those with 0 health, you only need to iterate once through the vector.

TimNN · 2016-06-14T07:53:53Z

src/libcollections/vec.rs

@@ -684,6 +684,44 @@ impl<T> Vec<T> {
        }
    }

+    /// Retains only the elements specified by the predicate.
+    ///
+    /// In other words, remove all elements `e` such that `f(&e)` returns false.


This should probably be f(&mut e).

TimNN · 2016-06-14T07:59:05Z

Since retain_mut is strictly more powerful than retain, I believe it should be possible to implement retain in terms of retain_mut, to reduce code duplication.

oli-obk · 2016-06-14T08:40:36Z

this has been discussed: #25477
and has an RFC: rust-lang/rfcs#1353

shepmaster · 2016-06-14T12:00:09Z

possible to implement retain in terms of retain_mut

I believe you'd have to make an intermediate form that abstracts over the mutability. Otherwise in one direction you give up the mutability and the other direction needs to manifest mutability out of thin air.

this has [...] an RFC

@oli-obk Ah, thanks! Obviously my searching was not very thorough at all..

Since the implementation isn't the hard part, I'm going to close this as the RFC shows that there's much more to discuss.

TimNN · 2016-06-14T12:02:48Z

@shepmaster couldn't you just define retain(&mut self, mut f) as self.retain_mut(|e| f(e))?

shepmaster · 2016-06-14T12:47:52Z

@TimNN Hmm, quite right. Not sure what I was thinking 😸

…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`

…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``

…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```

…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````

…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`````

…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``````

Add Vec::retain_mut

c963043

rust-highfive assigned aturon Jun 14, 2016

TimNN reviewed Jun 14, 2016
View reviewed changes

shepmaster closed this Jun 14, 2016

shepmaster deleted the retain-mut branch September 24, 2016 13:58

RalfJung mentioned this pull request Jul 31, 2017

Tracking issue for Vec::extract_if and LinkedList::extract_if #43244

Open

3 tasks

real-felix mentioned this pull request Sep 21, 2017

Vec::retain could (should?) give &mut T to the closure, not just &T #25477

Closed

jonas-schievink mentioned this pull request Mar 16, 2021

Add Vec::retain_mut #83218

Closed

GuillaumeGomez mentioned this pull request Nov 10, 2021

Add Vec::retain_mut #90772

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Vec::retain_mut #34265

Add Vec::retain_mut #34265

shepmaster commented Jun 14, 2016

rust-highfive commented Jun 14, 2016

shepmaster commented Jun 14, 2016

shepmaster commented Jun 14, 2016

TimNN Jun 14, 2016

TimNN commented Jun 14, 2016

oli-obk commented Jun 14, 2016

shepmaster commented Jun 14, 2016

TimNN commented Jun 14, 2016

shepmaster commented Jun 14, 2016

Add Vec::retain_mut #34265

Add Vec::retain_mut #34265

Conversation

shepmaster commented Jun 14, 2016

rust-highfive commented Jun 14, 2016

shepmaster commented Jun 14, 2016

shepmaster commented Jun 14, 2016

TimNN Jun 14, 2016

Choose a reason for hiding this comment

TimNN commented Jun 14, 2016

oli-obk commented Jun 14, 2016

shepmaster commented Jun 14, 2016

TimNN commented Jun 14, 2016

shepmaster commented Jun 14, 2016