Skip to content

Incorrect suggestion for has_significant_drop #10413

@Thomasdezeeuw

Description

@Thomasdezeeuw

Summary

The has_significant_drop lint only (seems to) considers the first usage of a locked value and suggests an incorrect correction of the code.

Lint Name

has_significant_drop

Reproducer

The code comes from https://github.com/Thomasdezeeuw/heph/blob/8be6cfa97d6870cc7e086e63e32608976c6d76f1/inbox/src/lib.rs#L519-L529. The relevant part:

    // self.registered_waker: Option<task::Waker>
    // self.channel.sender_wakers: Mutex<Vec<task::Waker>>
        if let Some(waker) = self.registered_waker.take() {
            let mut sender_wakers = self.channel.sender_wakers.lock().unwrap();
            let idx = sender_wakers.iter().position(|w| w.will_wake(&waker));
            if let Some(idx) = idx {
                let waker = sender_wakers.swap_remove(idx);
                unlock(sender_wakers);
                drop(waker);
            }
        }

The suggestion Clippy makes:

error: temporary with significant `Drop` can be early dropped
   --> inbox/src/lib.rs:523:21
    |
522 |           if let Some(waker) = self.registered_waker.take() {
    |  ___________________________________________________________-
523 | |             let mut sender_wakers = self.channel.sender_wakers.lock().unwrap();
    | |                     ^^^^^^^^^^^^^
524 | |             let idx = sender_wakers.iter().position(|w| w.will_wake(&waker));
525 | |             if let Some(idx) = idx {
...   |
529 | |             }
530 | |         }
    | |_________- temporary `sender_wakers` is currently being dropped at the end of its contained scope
    |
    = 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: `-D clippy::significant-drop-tightening` implied by `-D clippy::nursery`
help: merge the temporary construction with its single usage
    |
523 ~
524 +             let idx = self.channel.sender_wakers.lock().unwrap().position(|w| w.will_wake(&waker));
    |
help: remove separated single usage
    |
524 -             let idx = sender_wakers.iter().position(|w| w.will_wake(&waker));
524 +
    |

We can't move the lock into the line with idx as sender_wakers (the locked value) is used later on. Clippy however doesn't seem to see/understand this.

I guess the basic problem is that Clippy is only considering a single use of the locked value, while in the code above their are two uses.

Version

rustc 1.69.0-nightly (d962ea578 2023-02-26)
binary: rustc
commit-hash: d962ea57899d64dc8a57040142c6b498a57c8064
commit-date: 2023-02-26
host: x86_64-apple-darwin
release: 1.69.0-nightly
LLVM version: 15.0.7

Additional Labels

@rustbot label+I-suggestion-causes-error

Metadata

Metadata

Assignees

Labels

C-bugCategory: Clippy is not doing the correct thingI-false-positiveIssue: The lint was triggered on code it shouldn't have

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions