significant_drop_in_scrutinee: Fix false positives due to false drops of place expressions

[lrh2000]

Place expressions do not really create temporaries, so they will not create significant drops. For example, the following code snippet is quite good (#8963):

fn main() {
    let x = std::sync::Mutex::new(vec![1, 2, 3]);
    let x_guard = x.lock().unwrap();
    match x_guard[0] {
        1 => println!("1!"),
        x => println!("{x}"),
    }
    drop(x_guard); // Some "usage"
}

Also, the previous logic thinks that references like &MutexGuard<_>/Ref<'_, MutexGuard<'_, _>> have significant drops, which is simply not true, so it is fixed together in this PR.

Fixes #8963
Fixes #9072

changelog: [significant_drop_in_scrutinee]: Fix false positives due to false drops of place expressions.

r? @blyxyas

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @blyxyas (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[blyxyas]

Also, the previous logic thinks that references like &MutexGuard<>/Ref<', MutexGuard<'_, _>> have significant drops, which is simply not true, so it is fixed together in this PR.

MutexGuard<_> Has a significant drop (the attribute) (the drop function itself), I'm not sure why do you think that it doesn't have one. Could you elaborate? Maybe I'm not understanding something correctly, as mutexes are quite a deep topic.

[lrh2000]

MutexGuard<_> Has a significant drop (the attribute) (the drop function itself), I'm not sure why do you think that it doesn't have one. Could you elaborate? Maybe I'm not understanding something correctly, as mutexes are quite a deep topic.

It's MutexGuard<_> that has a significant drop, not &MutexGuard<_> (note that it's a reference).

For example, I've added a unit test for this:

struct Guard<'a, T>(MutexGuard<'a, T>);

impl<'a, T> Guard<'a, T> {
    fn guard(&self) -> &MutexGuard<T> {
        &self.0
    }
}

    match guard.guard().len() {
        0 => println!("empty"),
        _ => println!("not empty"),
    }

guard.guard().len() does not even create a MutexGuard temporary, so it has nothing to do with the significant drop check.

Actually, I think it's rare for this to happen in real-world scenarios (which is why no one has complained about it). However, I do think that the previous logic does not seem correct in this sense.

[blyxyas]

LGTM, thanks! ❤️
Sorry for the delay, I'm currently working on a pretty big change that will optimize the linting module in Rust (both Clippy and builtin lints) by about 50%. So I've been focusing on that optimization. (I also wanted to make sure that everything's right)

[blyxyas]

@bors r+

[bors]

📌 Commit 509ca90 has been approved by blyxyas

It is now in the queue for this repository.

[bors]

⌛ Testing commit 509ca90 with merge d6991ab...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas
Pushing d6991ab to master...

[lrh2000]

Sorry for the delay, I'm currently working on a pretty big change that will optimize the linting module in Rust (both Clippy and builtin lints) by about 50%. So I've been focusing on that optimization. (I also wanted to make sure that everything's right)

Thanks for the explanation. That's pretty cool 👍, and I completely understand that you're busy, so the review may be delayed (though I certainly want things to move faster). Also, thanks for the review!