Description
What it does
Warn when passing a Result to drop(). For example, the following trivial example should warn:
fn foo() -> Result<String, ()> {
Err(())
}
fn main() {
drop(foo()); // warning: result not checked
}This isn't a very common situation, in part because calling drop() explicitly just isn't very common. However, here's the situation that motivated this for me. Let's say I have some type that's primarily created for its side effects. (Sometimes a global Mutex is used this way, but in my case it's some random embedded thing that puts my hardware in some state that doesn't really matter here.) Your code might look like this:
struct ThingWithDropSideEffects {}
impl ThingWithDropSideEffects {
fn new() -> Self {
Self {}
}
}
impl Drop for ThingWithDropSideEffects {
fn drop(&mut self) {
println!("side effect!");
}
}
fn main() {
let thing = ThingWithDropSideEffects::new();
println!("do some work...");
drop(thing);
}I like this style over just a single line like let _thing = ..., because the explicit drop is kind of like an assertion about how long the value needs to live. It makes it a little less likely that someone might come along and delete my thing without understanding the side effects. (Of course I usually comment cases like this too.) However, this style has a practical downside that I ran into today. If the new function is changed to return a Result, this style silently drops the Result without checking it:
struct ThingWithDropSideEffects {}
impl ThingWithDropSideEffects {
fn new() -> Result<Self, ()> {
Err(())
}
}
impl Drop for ThingWithDropSideEffects {
fn drop(&mut self) {
println!("side effect!");
}
}
fn main() {
let thing = ThingWithDropSideEffects::new();
println!("do some work...");
drop(thing); // woops, this drops the error
}I wonder whether this is a good case for a Clippy lint? If so, I'm not sure whether it should be a lint on Result in particular or a lint on must_use values more broadly.
Lint Name
No response
Category
suspicious
Advantage
No response
Drawbacks
I'm sure some people use drop(...) as a way to silence must_use warnings on values they don't want to use. This might get in the way of that? On the other hand, I do think _ = ... is a better idiom for that. This is definitely something I'd want to get folks' feedback on.
Example
see above