significant_drop_tightening lint doesn't trigger when manually dropping

[m4rch3n1ng]

Summary

when manually dropping the lock the lint doesn't recognize, that it could be dropped earlier. it would be nice to lint that as well.

Lint Name

significant_drop_tightening

Reproducer

I tried this code:

use std::sync::Mutex;

static THING: Mutex<u32> = Mutex::new(0);

#[warn(clippy::significant_drop_tightening)]
fn main() {
	let mut thing = THING.lock().unwrap();
	*thing = 1;
	*thing += 1;

	println!("this is here to demonstrate the issue");
	println!("this lint does get triggered without the drop");

	drop(thing);
}

I expected to see this happen:

warning: temporary with significant `Drop` can be early dropped
  --> src/main.rs:7:10
   |
6  |   fn main() {
7  |       let mut thing = THING.lock().unwrap();
   |               ^^^^^
8  |       *thing = 1;
9  |       *thing += 1;
...   
14 |       drop(thing);
   |       ^^^^^^^^^^^
15 |   }
   |  _- temporary `thing` is currently being dropped later than necessary
   |
   = note: this might lead to unnecessary resource contention
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_tightening
note: the lint level is defined here
  --> src/main.rs:5:8
   |
5  | #[warn(clippy::significant_drop_tightening)]
   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: drop the temporary after the end of its last usage
   |
9  ~     *thing += 1;
10 +  drop(thing);
...
14 -     drop(thing);
   |

Instead, this happened:

nothing

More info:

if i remove the drop clippy correctly complains that it is dropped at the end of the scope and could be dropped earlier.

Version

No response

[samueltardieu]

Note that this is consistent with the lint description: "Searches for elements marked with #[clippy::has_significant_drop] that could be early dropped but are in fact dropped at the end of their scopes".

Here, the explicit use of drop(thing) effectively drops thing before the end of its scope, hence not triggering the lint.

[SpriteOvO]

Here, the explicit use of drop(thing) effectively drops thing before the end of its scope, hence not triggering the lint.

In fact, the explicit use of drop is not effective, since dropping before printlns can be more effective, isn't it?

Link to related issue #9399.

[samueltardieu]

since dropping before printlns can be more effective

Sure, I was just explaining why I think the lint doesn't trigger here (which would make it not a false negative).

[Jarcho]

This is intentional behavior. The main point of the lint is to catch places where a side effect is occurring silently, and it could happen earlier if done explicitly. Given that the programmer has chosen an explicit point by calling drop linting here will increase the rate of false positives (where moving the drop earlier is semantically incorrect).

This could be done through a config option, but I'm against doing it by default.

[m4rch3n1ng]

that makes sense, i thought it would make sense to also lint against this, since i had a deadlock where this would have missed it. the drop() in that case was just before the return and had to be dropped for the return code to compile due to partial move rules, but the deadlock was fixable by just tightening the drop.