rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.28k stars 1.52k forks source link

`clippy::mutex_integer` lints on uses of `Mutex<{integer}>`, not just definitions #13378

Open PatchMixolydic opened 1 week ago

PatchMixolydic commented 1 week ago

Summary

clippy::mutex_integer lints on usages as well as definitions, which seems redundant (and could be unactionable if this happens cross-crate).

Reproducer

I tried this code (playground):

#![allow(dead_code)]
#![warn(clippy::mutex_integer)]

use std::sync::Mutex;

// `Mutex` over `AtomicU32` to avoid a race between decrementing the guard count
// to 0 and enabling interrupts (not that it matters for this example...)
static NUM_INTERRUPTS_DISABLED_GUARDS: Mutex<u32> = Mutex::new(0);

#[non_exhaustive]
struct InterruptsDisabledGuard;

impl Drop for InterruptsDisabledGuard {
    fn drop(&mut self) {
        // Holds the lock for the entire function so that nobody tries to
        // increment the guard count right before interrupts are enabled
        let mut guard_count = NUM_INTERRUPTS_DISABLED_GUARDS.lock().unwrap();
        *guard_count -= 1;

        if *guard_count == 0 {
            println!("Interrupts enabled! Anything could happen!");
        }
    }
}

fn disable_interrupts() -> InterruptsDisabledGuard {
    *NUM_INTERRUPTS_DISABLED_GUARDS.lock().unwrap() += 1;
    println!("Interrupts disabled...");
    InterruptsDisabledGuard
}

I expected to see this happen:

warning: consider using an `AtomicU32` instead of a `Mutex` here; if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 --> src/lib.rs:8:53
  |
8 | static NUM_INTERRUPTS_DISABLED_GUARDS: Mutex<u32> = Mutex::new(0);
  |                                                     ^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer
note: the lint level is defined here
 --> src/lib.rs:2:9
  |
2 | #![warn(clippy::mutex_integer)]
  |         ^^^^^^^^^^^^^^^^^^^^^

Instead, this happened:

warning: consider using an `AtomicU32` instead of a `Mutex` here; if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 --> src/lib.rs:8:53
  |
8 | static NUM_INTERRUPTS_DISABLED_GUARDS: Mutex<u32> = Mutex::new(0);
  |                                                     ^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer
note: the lint level is defined here
 --> src/lib.rs:2:9
  |
2 | #![warn(clippy::mutex_integer)]
  |         ^^^^^^^^^^^^^^^^^^^^^

warning: consider using an `AtomicU32` instead of a `Mutex` here; if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
  --> src/lib.rs:17:31
   |
17 |         let mut guard_count = NUM_INTERRUPTS_DISABLED_GUARDS.lock().unwrap();
   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer

warning: consider using an `AtomicU32` instead of a `Mutex` here; if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
  --> src/lib.rs:27:6
   |
27 |     *NUM_INTERRUPTS_DISABLED_GUARDS.lock().unwrap() += 1;
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer

Version

1.83.0-nightly (2024-09-05 9c01301c52df5d2d7b6f) on the Rust Playground

Additional Labels

No response

PatchMixolydic commented 1 week ago

Slightly off topic, but looking at this diagnostic, "if you just want the locking behavior and not the internal type, consider using Mutex<()>" seems like it should be moved to a help message:


warning: consider using an `AtomicU32` instead of a `Mutex` here
 --> src/lib.rs:8:53
  |
8 | static NUM_INTERRUPTS_DISABLED_GUARDS: Mutex<u32> = Mutex::new(0);
  |                                                     ^^^^^^^^^^^^^
  |
  = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer
note: the lint level is defined here
alex-semenyuk commented 1 week ago

@rustbot label I-false-positive