Repository navigation
mem::replace -> mem::take lint is too aggressive #5586
Description
Clippy will suggest replacing
let x = mem::replace(&mut thing, Thing::default());with:
let x = mem::take(&mut thing);This is great! But it will also suggest replacing
mem::replace(&mut thing, Thing::default());with:
mem::take(&mut thing);This is not great. The construction using mem::replace in the second case is much clearer IMO as it indicates that the function is being executed for its side effects. The construction using mem::take looks like a mistake.
Would you folks be open to making the lint not apply in the second case?
Reacted by Lucas
Activity
flip1995 commentedon May 11, 2020
So basically only suggest using
mem::takewhen the replacedthingis not assigned to anything?benesch commentedon May 11, 2020
Exactly. Arguably clippy should suggest
*thing = Thing::default()in the latter case.Luro02 commentedon May 12, 2020
It might be useful to have a clippy lint that suggests replacing the latter with the former, because like you said the latter looks like a mistake (it is harder to read, because there is an implicit replacement of the field with default that is not obvious by the method name).
taiki-e commentedon May 12, 2020
Given that
mem::replaceis#[must_use](rust-lang/rust#71256), the usage that ignoresmem::replace's return value is probably not the recommended way.benesch commentedon May 12, 2020
Yeah, looks like the upstream recommended way is just to assign:
camsteffen commentedon May 28, 2020
Since
mem::replacewas given#[must_use], perhaps themem_replace_with_defaultlint should simply ignore cases where the result isn't used. The#[must_use]lint will provide the correct suggestion.GabrielBFern commentedon Feb 11, 2024
@rustbot claim
Auto merge of #12278 - GabrielBFern:master, r=Jarcho