let_underscore_drop complains about idiomatic `let _ = ignorable_result`;

[birkenfeld]

Lint name: let_underscore_drop

I tried this code:

#![allow(unused)]
#![deny(clippy::let_underscore_drop)]

struct Test;

impl Test {
    fn causes_error(&self) -> Result<(), std::io::Error> {
        todo!()
    }
    
    fn bug(&self) {
        let _ = self.causes_error();
    }
}

I expected to see this happen: no clippy errors

Instead, this happened: clippy wants to keep the Result around until end of scope, while let _ = is the normally suggested way to deal with results where there is no useful Ok and the Err needs to be ignored.

[edit: this originally suggested that the Drop bound is wrongly inferred.]

Meta

Rust version: Nightly on Playground (2021-11-19 a77da2d)

[smoelius]

Maybe this is correct? Repr::Custom wraps a Box and Box implements Drop.
https://doc.rust-lang.org/src/std/io/error.rs.html#74
https://doc.rust-lang.org/std/boxed/struct.Box.html#impl-Drop

[birkenfeld]

You're right, the Drop bound doesn't work properly. std::mem::needs_drop() agrees with you.

However, I will then reformulate the issue: it is idiomatic (at least I thought so) to discard Results that need not be handled with let _ = result; The semantics is also what's desired: no need to keep the Error around until end of scope.

So the lint should maybe just get an exception for the Result case?

[xFrednet]

I am not convinced that Result should get a pass out of the box, there are probably cases where the error should be keep. On the other hand, this would also be a bit weird. I'm not quite sure what the best course of action would be, we could introduce a configuration value for it. 🤔

[birkenfeld]

Sure it is only pedantic, but clippy directly contradicts idiomatic use here. And I'd be very interested to see an error with an intended scope-like effect :)

[xFrednet]

Thinking about it more, I agree that it would make sense to allow this lint for the Result type, at least if only the Error type implements drop. Maybe we could still have a configuration value, but the default configuration could allow Result by default. :)

[doy]

would it be possible to determine whether the only Drop implementation found is for Box, and just ignore that case? that feels like it would cover most issues here, and Box::drop seems irrelevant to the issue that this lint is trying to catch.