diff --git a/clippy_lints/src/mutex_atomic.rs b/clippy_lints/src/mutex_atomic.rs index 7ff8729fe060..d096d965ed32 100644 --- a/clippy_lints/src/mutex_atomic.rs +++ b/clippy_lints/src/mutex_atomic.rs @@ -1,7 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::res::MaybeDef; +use clippy_utils::ty::ty_from_hir_ty; use rustc_errors::Diag; -use rustc_hir::Expr; +use rustc_hir::{Expr, Item, ItemKind, LetStmt}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, IntTy, Ty, UintTy}; use rustc_session::declare_lint_pass; @@ -89,26 +90,43 @@ declare_clippy_lint! { declare_lint_pass!(Mutex => [MUTEX_ATOMIC, MUTEX_INTEGER]); +// NOTE: we don't use `check_expr` because that would make us lint every _use_ of such mutexes, not +// just their definitions impl<'tcx> LateLintPass<'tcx> for Mutex { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - let ty = cx.typeck_results().expr_ty(expr); - if let ty::Adt(_, subst) = ty.kind() - && ty.is_diag_item(cx, sym::Mutex) - && let mutex_param = subst.type_at(0) - && let Some(atomic_name) = get_atomic_name(mutex_param) + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + if !item.span.from_expansion() + && let ItemKind::Static(_, _, ty, body_id) = item.kind { - let msg = "using a `Mutex` where an atomic would do"; - let diag = |diag: &mut Diag<'_, _>| { - diag.help(format!("consider using an `{atomic_name}` instead")); - diag.help( - "if you just want the locking behavior and not the internal type, consider using `Mutex<()>`", - ); - }; - match *mutex_param.kind() { - ty::Uint(t) if t != UintTy::Usize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag), - ty::Int(t) if t != IntTy::Isize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag), - _ => span_lint_and_then(cx, MUTEX_ATOMIC, expr.span, msg, diag), - } + let body = cx.tcx.hir_body(body_id); + let mid_ty = ty_from_hir_ty(cx, ty); + check_expr(cx, body.value.peel_blocks(), mid_ty); + } + } + fn check_local(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx LetStmt<'_>) { + if !stmt.span.from_expansion() + && let Some(init) = stmt.init + { + let mid_ty = cx.typeck_results().expr_ty(init); + check_expr(cx, init.peel_blocks(), mid_ty); + } + } +} + +fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty: Ty<'tcx>) { + if let ty::Adt(_, subst) = ty.kind() + && ty.is_diag_item(cx, sym::Mutex) + && let mutex_param = subst.type_at(0) + && let Some(atomic_name) = get_atomic_name(mutex_param) + { + let msg = "using a `Mutex` where an atomic would do"; + let diag = |diag: &mut Diag<'_, _>| { + diag.help(format!("consider using an `{atomic_name}` instead")); + diag.help("if you just want the locking behavior and not the internal type, consider using `Mutex<()>`"); + }; + match *mutex_param.kind() { + ty::Uint(t) if t != UintTy::Usize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag), + ty::Int(t) if t != IntTy::Isize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag), + _ => span_lint_and_then(cx, MUTEX_ATOMIC, expr.span, msg, diag), } } } diff --git a/tests/ui/mutex_atomic.rs b/tests/ui/mutex_atomic.rs index 7db5c9f274f6..cc53cc3136c6 100644 --- a/tests/ui/mutex_atomic.rs +++ b/tests/ui/mutex_atomic.rs @@ -2,47 +2,64 @@ #![warn(clippy::mutex_atomic)] #![allow(clippy::borrow_as_ptr)] +use std::sync::Mutex; + fn main() { - use std::sync::Mutex; - Mutex::new(true); + let _ = Mutex::new(true); //~^ mutex_atomic - Mutex::new(5usize); + let _ = Mutex::new(5usize); //~^ mutex_atomic - Mutex::new(9isize); + let _ = Mutex::new(9isize); //~^ mutex_atomic let mut x = 4u32; - Mutex::new(&x as *const u32); + let _ = Mutex::new(&x as *const u32); //~^ mutex_atomic - Mutex::new(&mut x as *mut u32); + let _ = Mutex::new(&mut x as *mut u32); //~^ mutex_atomic - Mutex::new(0u32); + let _ = Mutex::new(0u32); //~^ mutex_integer - Mutex::new(0i32); + let _ = Mutex::new(0i32); //~^ mutex_integer - Mutex::new(0f32); // there are no float atomics, so this should not lint - Mutex::new(0u8); + let _ = Mutex::new(0f32); // there are no float atomics, so this should not lint + let _ = Mutex::new(0u8); //~^ mutex_integer - Mutex::new(0i16); + let _ = Mutex::new(0i16); //~^ mutex_integer let _x: Mutex = Mutex::new(0); //~^ mutex_integer const X: i64 = 0; - Mutex::new(X); + let _ = Mutex::new(X); //~^ mutex_integer // there are no 128 atomics, so these two should not lint { - Mutex::new(0u128); + let _ = Mutex::new(0u128); let _x: Mutex = Mutex::new(0); } } + +static MTX: Mutex = Mutex::new(0); +//~^ mutex_integer + +// don't lint on _use_, only declaration +fn issue13378() { + let mut guard = MTX.lock().unwrap(); + *guard += 1; + + let mtx = Mutex::new(0); + //~^ mutex_integer + // This will still lint, since we're reassigning the mutex to a variable -- oh well. + // But realistically something like this won't really come up. + let reassigned = mtx; + //~^ mutex_integer +} diff --git a/tests/ui/mutex_atomic.stderr b/tests/ui/mutex_atomic.stderr index b328461ff0ce..839b641c574a 100644 --- a/tests/ui/mutex_atomic.stderr +++ b/tests/ui/mutex_atomic.stderr @@ -1,8 +1,8 @@ error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:7:5 + --> tests/ui/mutex_atomic.rs:8:13 | -LL | Mutex::new(true); - | ^^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(true); + | ^^^^^^^^^^^^^^^^ | = help: consider using an `AtomicBool` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` @@ -10,46 +10,46 @@ LL | Mutex::new(true); = help: to override `-D warnings` add `#[allow(clippy::mutex_atomic)]` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:10:5 + --> tests/ui/mutex_atomic.rs:11:13 | -LL | Mutex::new(5usize); - | ^^^^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(5usize); + | ^^^^^^^^^^^^^^^^^^ | = help: consider using an `AtomicUsize` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:13:5 + --> tests/ui/mutex_atomic.rs:14:13 | -LL | Mutex::new(9isize); - | ^^^^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(9isize); + | ^^^^^^^^^^^^^^^^^^ | = help: consider using an `AtomicIsize` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:17:5 + --> tests/ui/mutex_atomic.rs:18:13 | -LL | Mutex::new(&x as *const u32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(&x as *const u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider using an `AtomicPtr` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:20:5 + --> tests/ui/mutex_atomic.rs:21:13 | -LL | Mutex::new(&mut x as *mut u32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(&mut x as *mut u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider using an `AtomicPtr` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:23:5 + --> tests/ui/mutex_atomic.rs:24:13 | -LL | Mutex::new(0u32); - | ^^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(0u32); + | ^^^^^^^^^^^^^^^^ | = help: consider using an `AtomicU32` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` @@ -57,34 +57,34 @@ LL | Mutex::new(0u32); = help: to override `-D warnings` add `#[allow(clippy::mutex_integer)]` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:26:5 + --> tests/ui/mutex_atomic.rs:27:13 | -LL | Mutex::new(0i32); - | ^^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(0i32); + | ^^^^^^^^^^^^^^^^ | = help: consider using an `AtomicI32` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:30:5 + --> tests/ui/mutex_atomic.rs:31:13 | -LL | Mutex::new(0u8); - | ^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(0u8); + | ^^^^^^^^^^^^^^^ | = help: consider using an `AtomicU8` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:33:5 + --> tests/ui/mutex_atomic.rs:34:13 | -LL | Mutex::new(0i16); - | ^^^^^^^^^^^^^^^^ +LL | let _ = Mutex::new(0i16); + | ^^^^^^^^^^^^^^^^ | = help: consider using an `AtomicI16` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:36:25 + --> tests/ui/mutex_atomic.rs:37:25 | LL | let _x: Mutex = Mutex::new(0); | ^^^^^^^^^^^^^ @@ -93,13 +93,40 @@ LL | let _x: Mutex = Mutex::new(0); = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:40:5 + --> tests/ui/mutex_atomic.rs:41:13 | -LL | Mutex::new(X); - | ^^^^^^^^^^^^^ +LL | let _ = Mutex::new(X); + | ^^^^^^^^^^^^^ | = help: consider using an `AtomicI64` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` -error: aborting due to 11 previous errors +error: using a `Mutex` where an atomic would do + --> tests/ui/mutex_atomic.rs:51:26 + | +LL | static MTX: Mutex = Mutex::new(0); + | ^^^^^^^^^^^^^ + | + = help: consider using an `AtomicU32` instead + = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` + +error: using a `Mutex` where an atomic would do + --> tests/ui/mutex_atomic.rs:59:15 + | +LL | let mtx = Mutex::new(0); + | ^^^^^^^^^^^^^ + | + = help: consider using an `AtomicI32` instead + = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` + +error: using a `Mutex` where an atomic would do + --> tests/ui/mutex_atomic.rs:63:22 + | +LL | let reassigned = mtx; + | ^^^ + | + = help: consider using an `AtomicI32` instead + = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` + +error: aborting due to 14 previous errors