new lint: avoid `vec.drain(..).collect()` in favor of `std::mem::take(&mut vec)`

[iliana]

What it does

This lint would see the use of vec.drain(..).collect(), which moves all the items from a Vec<T> into a new Vec<T> via FromIterator, and suggest to instead use std::mem::take(&mut vec).

This is similar to #9339, implemented as clear_with_drain.

Lint Name

collect_with_drain

Category

perf, pedantic

Advantage

• Avoids FromIterator and new allocations.
• (Subjectively) clearer code.

Drawbacks

• By taking the Vec and replacing it with a new one, you've moved the capacity, but the author may have intended to keep the capacity in place and perform the allocation/memcpy. This situation seems rare to me.

Example

// self.vec is a Vec<u32>
fn func(&mut self) -> Vec<u32> {
    self.vec.drain(..).collect()
}

Could be written as:

fn func(&mut self) -> Vec<u32> {
    std::mem::take(&mut self.vec)
}

[y21]

@rustbot claim

Seems like this could work not just for Vec, but also String, HashSet, HashMap, BinaryHeap, VecDeque

[blyxyas]

Why would someone move all elements from one Vec<T> to another Vec<T>?

[dswij]

Why would someone move all elements from one Vec<T> to another Vec<T>?

@blyxyas the buffer example in mem::take docs is a great usecase https://doc.rust-lang.org/std/mem/fn.take.html#examples

[Firaenix]

Welp, I just got hit by the drawback, took me ages to figure out what the issue was.

I have a separate tokio task that batches up logs and writes them to a sqlite DB when capacity of the Vec is full, I was using this clippy hint and it modified my capacity and caused far more frequent flushes than I was expecting

[blyxyas]

Uff, quite the performance hit.

Maybe we could add a visitor to prevent the lint if .capacity() is used on that vector (or maybe anywhere in the module / crate?)

[Firaenix]

Something like that would be nice, just considering this is a semantic change if you use capacity().
However, maybe the lint could suggest to not use capacity and instead set a limit outside of the vec?

Something along the lines of:

let vec_cap = 10;

let my_vec = vec![];

...
if my_vec.len() < vec_cap {
 // Do something 
}
...

std::mem::take(&mut my_vec);