From 85490d1845d67c026d5f09bef4b796b7b6c8f725 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 7 Sep 2025 21:56:03 +0200 Subject: [PATCH 1/6] clean-up --- clippy_lints/src/mutex_atomic.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/mutex_atomic.rs b/clippy_lints/src/mutex_atomic.rs index a93665ef3e9d..306f9dd1b8a7 100644 --- a/clippy_lints/src/mutex_atomic.rs +++ b/clippy_lints/src/mutex_atomic.rs @@ -93,18 +93,17 @@ impl<'tcx> LateLintPass<'tcx> for Mutex { 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) { - let mutex_param = subst.type_at(0); - if let Some(atomic_name) = get_atomic_name(mutex_param) { - let msg = format!( - "consider using an `{atomic_name}` instead of a `Mutex` here; 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(cx, MUTEX_INTEGER, expr.span, msg), - ty::Int(t) if t != IntTy::Isize => span_lint(cx, MUTEX_INTEGER, expr.span, msg), - _ => span_lint(cx, MUTEX_ATOMIC, expr.span, msg), - } + let msg = format!( + "consider using an `{atomic_name}` instead of a `Mutex` here; 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(cx, MUTEX_INTEGER, expr.span, msg), + ty::Int(t) if t != IntTy::Isize => span_lint(cx, MUTEX_INTEGER, expr.span, msg), + _ => span_lint(cx, MUTEX_ATOMIC, expr.span, msg), } } } From 3ae047ee04671f43bac3ee80577618faa0e132fe Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 7 Sep 2025 21:47:25 +0200 Subject: [PATCH 2/6] restructure messages - The main message should point out what's wrong, not directly suggest a solution. - The second part of the message is a separate advice, so it should be emitted separately. --- clippy_lints/src/mutex_atomic.rs | 20 +++++++----- tests/ui/mutex_atomic.stderr | 53 +++++++++++++++++++++++++------- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/mutex_atomic.rs b/clippy_lints/src/mutex_atomic.rs index 306f9dd1b8a7..7ff8729fe060 100644 --- a/clippy_lints/src/mutex_atomic.rs +++ b/clippy_lints/src/mutex_atomic.rs @@ -1,5 +1,6 @@ -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::res::MaybeDef; +use rustc_errors::Diag; use rustc_hir::Expr; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, IntTy, Ty, UintTy}; @@ -96,14 +97,17 @@ impl<'tcx> LateLintPass<'tcx> for Mutex { && let mutex_param = subst.type_at(0) && let Some(atomic_name) = get_atomic_name(mutex_param) { - let msg = format!( - "consider using an `{atomic_name}` instead of a `Mutex` here; if you just want the locking \ - behavior and not the internal type, consider using `Mutex<()>`" - ); + 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(cx, MUTEX_INTEGER, expr.span, msg), - ty::Int(t) if t != IntTy::Isize => span_lint(cx, MUTEX_INTEGER, expr.span, msg), - _ => span_lint(cx, MUTEX_ATOMIC, expr.span, msg), + 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.stderr b/tests/ui/mutex_atomic.stderr index a6d5d60fbf05..b328461ff0ce 100644 --- a/tests/ui/mutex_atomic.stderr +++ b/tests/ui/mutex_atomic.stderr @@ -1,74 +1,105 @@ -error: consider using an `AtomicBool` instead of a `Mutex` here; 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:7:5 | LL | 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<()>` = note: `-D clippy::mutex-atomic` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::mutex_atomic)]` -error: consider using an `AtomicUsize` instead of a `Mutex` here; 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:10:5 | LL | 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: consider using an `AtomicIsize` instead of a `Mutex` here; 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 | LL | 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: consider using an `AtomicPtr` instead of a `Mutex` here; 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 | LL | 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: consider using an `AtomicPtr` instead of a `Mutex` here; 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 | LL | 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: consider using an `AtomicU32` instead of a `Mutex` here; 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 | LL | 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<()>` = note: `-D clippy::mutex-integer` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::mutex_integer)]` -error: consider using an `AtomicI32` instead of a `Mutex` here; 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:26:5 | LL | 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: consider using an `AtomicU8` instead of a `Mutex` here; 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 | LL | 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: consider using an `AtomicI16` instead of a `Mutex` here; 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 | LL | 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: consider using an `AtomicI8` instead of a `Mutex` here; 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 | LL | let _x: Mutex = Mutex::new(0); | ^^^^^^^^^^^^^ + | + = help: consider using an `AtomicI8` instead + = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` -error: consider using an `AtomicI64` instead of a `Mutex` here; 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 | LL | 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 From 38ac3d041c03f6898ea28fddad245c3e29645de3 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 7 Sep 2025 22:45:53 +0200 Subject: [PATCH 3/6] only lint on definitions, not use --- clippy_lints/src/mutex_atomic.rs | 56 +++++++++++++------- tests/ui/mutex_atomic.rs | 43 ++++++++++----- tests/ui/mutex_atomic.stderr | 91 +++++++++++++++++++++----------- 3 files changed, 126 insertions(+), 64 deletions(-) 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 From 99ce6391ddaa2470759443b7dad9ca097d6dc93c Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 7 Sep 2025 23:37:53 +0200 Subject: [PATCH 4/6] suggest replacing `Mutex::new` with `AtomicX::new` --- clippy_lints/src/mutex_atomic.rs | 19 +++++++-- tests/ui/mutex_atomic.rs | 7 ++-- tests/ui/mutex_atomic.stderr | 69 +++++++++++++------------------- 3 files changed, 48 insertions(+), 47 deletions(-) diff --git a/clippy_lints/src/mutex_atomic.rs b/clippy_lints/src/mutex_atomic.rs index d096d965ed32..c92bad1393ca 100644 --- a/clippy_lints/src/mutex_atomic.rs +++ b/clippy_lints/src/mutex_atomic.rs @@ -1,8 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::res::MaybeDef; +use clippy_utils::sugg::Sugg; use clippy_utils::ty::ty_from_hir_ty; -use rustc_errors::Diag; -use rustc_hir::{Expr, Item, ItemKind, LetStmt}; +use rustc_errors::{Applicability, Diag}; +use rustc_hir::{Expr, ExprKind, Item, ItemKind, LetStmt, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, IntTy, Ty, UintTy}; use rustc_session::declare_lint_pass; @@ -120,7 +121,19 @@ fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty: Ty<'tcx>) { { let msg = "using a `Mutex` where an atomic would do"; let diag = |diag: &mut Diag<'_, _>| { - diag.help(format!("consider using an `{atomic_name}` instead")); + // if `expr = Mutex::new(arg)`, we can try emitting a suggestion + if let ExprKind::Call(qpath, [arg]) = expr.kind + && let ExprKind::Path(QPath::TypeRelative(_mutex, new)) = qpath.kind + && new.ident.name == sym::new + { + let mut applicability = Applicability::MaybeIncorrect; + let arg = Sugg::hir_with_applicability(cx, arg, "_", &mut applicability); + + let suggs = vec![(expr.span, format!("std::sync::atomic::{atomic_name}::new({arg})"))]; + diag.multipart_suggestion("try", suggs, applicability); + } else { + 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() { diff --git a/tests/ui/mutex_atomic.rs b/tests/ui/mutex_atomic.rs index cc53cc3136c6..331c09ab1a71 100644 --- a/tests/ui/mutex_atomic.rs +++ b/tests/ui/mutex_atomic.rs @@ -1,3 +1,4 @@ +//@no-rustfix #![warn(clippy::mutex_integer)] #![warn(clippy::mutex_atomic)] #![allow(clippy::borrow_as_ptr)] @@ -48,11 +49,11 @@ fn main() { } } -static MTX: Mutex = Mutex::new(0); -//~^ mutex_integer - // don't lint on _use_, only declaration fn issue13378() { + static MTX: Mutex = Mutex::new(0); + //~^ mutex_integer + let mut guard = MTX.lock().unwrap(); *guard += 1; diff --git a/tests/ui/mutex_atomic.stderr b/tests/ui/mutex_atomic.stderr index 839b641c574a..9afbf72dc67d 100644 --- a/tests/ui/mutex_atomic.stderr +++ b/tests/ui/mutex_atomic.stderr @@ -1,126 +1,113 @@ error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:8:13 + --> tests/ui/mutex_atomic.rs:9:13 | LL | let _ = Mutex::new(true); - | ^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicBool::new(true)` | - = help: consider using an `AtomicBool` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` = note: `-D clippy::mutex-atomic` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::mutex_atomic)]` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:11:13 + --> tests/ui/mutex_atomic.rs:12:13 | LL | let _ = Mutex::new(5usize); - | ^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicUsize::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:14:13 + --> tests/ui/mutex_atomic.rs:15:13 | LL | let _ = Mutex::new(9isize); - | ^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicIsize::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:18:13 + --> tests/ui/mutex_atomic.rs:19:13 | LL | let _ = Mutex::new(&x as *const u32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicPtr::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:21:13 + --> tests/ui/mutex_atomic.rs:22:13 | LL | let _ = Mutex::new(&mut x as *mut u32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicPtr::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:24:13 + --> tests/ui/mutex_atomic.rs:25:13 | LL | let _ = Mutex::new(0u32); - | ^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicU32::new(0u32)` | - = help: consider using an `AtomicU32` instead = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` = note: `-D clippy::mutex-integer` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::mutex_integer)]` error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:27:13 + --> tests/ui/mutex_atomic.rs:28:13 | LL | let _ = Mutex::new(0i32); - | ^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI32::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:31:13 + --> tests/ui/mutex_atomic.rs:32:13 | LL | let _ = Mutex::new(0u8); - | ^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicU8::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:34:13 + --> tests/ui/mutex_atomic.rs:35:13 | LL | let _ = Mutex::new(0i16); - | ^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI16::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:37:25 + --> tests/ui/mutex_atomic.rs:38:25 | LL | let _x: Mutex = Mutex::new(0); - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI8::new(0)` | - = help: consider using an `AtomicI8` 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:41:13 + --> tests/ui/mutex_atomic.rs:42:13 | LL | let _ = Mutex::new(X); - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI64::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: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:51:26 + --> tests/ui/mutex_atomic.rs:54:30 | -LL | static MTX: Mutex = Mutex::new(0); - | ^^^^^^^^^^^^^ +LL | static MTX: Mutex = Mutex::new(0); + | ^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicU32::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 + --> tests/ui/mutex_atomic.rs:60:15 | LL | let mtx = Mutex::new(0); - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI32::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 + --> tests/ui/mutex_atomic.rs:64:22 | LL | let reassigned = mtx; | ^^^ From e5fd5714145bd167ac82d306219e8ae65c8f699e Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Mon, 8 Sep 2025 01:05:38 +0200 Subject: [PATCH 5/6] realize that a test case is incorrect `Mutex<*const _>` doesn't make a lot of sense (there can be no contention over a read-only reference), but `AtomicPtr::new(*const _)` straight up doesn't compile --- clippy_lints/src/mutex_atomic.rs | 4 +++- tests/ui/mutex_atomic.rs | 2 +- tests/ui/mutex_atomic.stderr | 10 +--------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/mutex_atomic.rs b/clippy_lints/src/mutex_atomic.rs index c92bad1393ca..1cb6b901723f 100644 --- a/clippy_lints/src/mutex_atomic.rs +++ b/clippy_lints/src/mutex_atomic.rs @@ -5,6 +5,7 @@ use clippy_utils::ty::ty_from_hir_ty; use rustc_errors::{Applicability, Diag}; use rustc_hir::{Expr, ExprKind, Item, ItemKind, LetStmt, QPath}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::mir::Mutability; use rustc_middle::ty::{self, IntTy, Ty, UintTy}; use rustc_session::declare_lint_pass; use rustc_span::sym; @@ -169,7 +170,8 @@ fn get_atomic_name(ty: Ty<'_>) -> Option<&'static str> { IntTy::I128 => None, } }, - ty::RawPtr(_, _) => Some("AtomicPtr"), + // `AtomicPtr` only accepts `*mut T` + ty::RawPtr(_, Mutability::Mut) => Some("AtomicPtr"), _ => None, } } diff --git a/tests/ui/mutex_atomic.rs b/tests/ui/mutex_atomic.rs index 331c09ab1a71..b907c3081841 100644 --- a/tests/ui/mutex_atomic.rs +++ b/tests/ui/mutex_atomic.rs @@ -16,8 +16,8 @@ fn main() { //~^ mutex_atomic let mut x = 4u32; + // `AtomicPtr` only accepts `*mut T`, so this should not lint let _ = Mutex::new(&x as *const u32); - //~^ mutex_atomic let _ = Mutex::new(&mut x as *mut u32); //~^ mutex_atomic diff --git a/tests/ui/mutex_atomic.stderr b/tests/ui/mutex_atomic.stderr index 9afbf72dc67d..9d8a7ccb33bd 100644 --- a/tests/ui/mutex_atomic.stderr +++ b/tests/ui/mutex_atomic.stderr @@ -24,14 +24,6 @@ LL | let _ = Mutex::new(9isize); | = 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:19:13 - | -LL | let _ = Mutex::new(&x as *const u32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicPtr::new(&x as *const u32)` - | - = 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:22:13 | @@ -115,5 +107,5 @@ 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 +error: aborting due to 13 previous errors From 778da589c6357c489ba1b6f3bb022359ca73435c Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Mon, 8 Sep 2025 00:18:21 +0200 Subject: [PATCH 6/6] suggest adjusting the type ascription --- clippy_lints/src/mutex_atomic.rs | 43 ++++++++++++++--- tests/ui/mutex_atomic.fixed | 67 ++++++++++++++++++++++++++ tests/ui/mutex_atomic.rs | 9 ++-- tests/ui/mutex_atomic.stderr | 55 +++++++++++++++------ tests/ui/mutex_atomic_unfixable.rs | 13 +++++ tests/ui/mutex_atomic_unfixable.stderr | 17 +++++++ 6 files changed, 177 insertions(+), 27 deletions(-) create mode 100644 tests/ui/mutex_atomic.fixed create mode 100644 tests/ui/mutex_atomic_unfixable.rs create mode 100644 tests/ui/mutex_atomic_unfixable.stderr diff --git a/clippy_lints/src/mutex_atomic.rs b/clippy_lints/src/mutex_atomic.rs index 1cb6b901723f..2fef8404f824 100644 --- a/clippy_lints/src/mutex_atomic.rs +++ b/clippy_lints/src/mutex_atomic.rs @@ -1,10 +1,11 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::res::MaybeDef; +use clippy_utils::source::{IntoSpan, SpanRangeExt}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::ty_from_hir_ty; use rustc_errors::{Applicability, Diag}; -use rustc_hir::{Expr, ExprKind, Item, ItemKind, LetStmt, QPath}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_hir::{self as hir, Expr, ExprKind, Item, ItemKind, LetStmt, QPath}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::mir::Mutability; use rustc_middle::ty::{self, IntTy, Ty, UintTy}; use rustc_session::declare_lint_pass; @@ -101,7 +102,7 @@ impl<'tcx> LateLintPass<'tcx> for Mutex { { 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); + check_expr(cx, body.value.peel_blocks(), &TypeAscriptionKind::Required(ty), mid_ty); } } fn check_local(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx LetStmt<'_>) { @@ -109,12 +110,26 @@ impl<'tcx> LateLintPass<'tcx> for Mutex { && let Some(init) = stmt.init { let mid_ty = cx.typeck_results().expr_ty(init); - check_expr(cx, init.peel_blocks(), mid_ty); + check_expr(cx, init.peel_blocks(), &TypeAscriptionKind::Optional(stmt.ty), mid_ty); } } } -fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty: Ty<'tcx>) { +/// Whether the type ascription `: Mutex` (which we'll suggest replacing with `AtomicX`) is +/// required +enum TypeAscriptionKind<'tcx> { + /// Yes; for us, this is the case for statics + Required(&'tcx hir::Ty<'tcx>), + /// No; the ascription might've been necessary in an expression like: + /// ```ignore + /// let mutex: Mutex = Mutex::new(0); + /// ``` + /// to specify the type of `0`, but since `AtomicX` already refers to a concrete type, we won't + /// need this ascription anymore. + Optional(Option<&'tcx hir::Ty<'tcx>>), +} + +fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty_ascription: &TypeAscriptionKind<'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) @@ -129,8 +144,22 @@ fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty: Ty<'tcx>) { { let mut applicability = Applicability::MaybeIncorrect; let arg = Sugg::hir_with_applicability(cx, arg, "_", &mut applicability); - - let suggs = vec![(expr.span, format!("std::sync::atomic::{atomic_name}::new({arg})"))]; + let mut suggs = vec![(expr.span, format!("std::sync::atomic::{atomic_name}::new({arg})"))]; + match ty_ascription { + TypeAscriptionKind::Required(ty_ascription) => { + suggs.push((ty_ascription.span, format!("std::sync::atomic::{atomic_name}"))); + }, + TypeAscriptionKind::Optional(Some(ty_ascription)) => { + // See https://github.com/rust-lang/rust-clippy/pull/15386 for why this is + // required + let colon_ascription = (cx.sess().source_map()) + .span_extend_to_prev_char_before(ty_ascription.span, ':', true) + .with_leading_whitespace(cx) + .into_span(); + suggs.push((colon_ascription, String::new())); + }, + TypeAscriptionKind::Optional(None) => {}, // nothing to remove/replace + } diag.multipart_suggestion("try", suggs, applicability); } else { diag.help(format!("consider using an `{atomic_name}` instead")); diff --git a/tests/ui/mutex_atomic.fixed b/tests/ui/mutex_atomic.fixed new file mode 100644 index 000000000000..e4218726019f --- /dev/null +++ b/tests/ui/mutex_atomic.fixed @@ -0,0 +1,67 @@ +#![warn(clippy::mutex_integer)] +#![warn(clippy::mutex_atomic)] +#![allow(clippy::borrow_as_ptr)] + +use std::sync::Mutex; + +fn main() { + let _ = std::sync::atomic::AtomicBool::new(true); + //~^ mutex_atomic + + let _ = std::sync::atomic::AtomicUsize::new(5usize); + //~^ mutex_atomic + + let _ = std::sync::atomic::AtomicIsize::new(9isize); + //~^ mutex_atomic + + let mut x = 4u32; + // `AtomicPtr` only accepts `*mut T`, so this should not lint + let _ = Mutex::new(&x as *const u32); + + let _ = std::sync::atomic::AtomicPtr::new(&mut x as *mut u32); + //~^ mutex_atomic + + let _ = std::sync::atomic::AtomicU32::new(0u32); + //~^ mutex_integer + + let _ = std::sync::atomic::AtomicI32::new(0i32); + //~^ mutex_integer + + let _ = Mutex::new(0f32); // there are no float atomics, so this should not lint + let _ = std::sync::atomic::AtomicU8::new(0u8); + //~^ mutex_integer + + let _ = std::sync::atomic::AtomicI16::new(0i16); + //~^ mutex_integer + + let _x = std::sync::atomic::AtomicI8::new(0); + //~^ mutex_integer + + const X: i64 = 0; + let _ = std::sync::atomic::AtomicI64::new(X); + //~^ mutex_integer + + // there are no 128 atomics, so these two should not lint + { + let _ = Mutex::new(0u128); + let _x: Mutex = Mutex::new(0); + } +} + +// don't lint on _use_, only declaration +fn issue13378() { + static MTX: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + //~^ mutex_integer + + let mtx = std::sync::atomic::AtomicI32::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 + + // don't eat the `)` when removing the type ascription -- see + // https://github.com/rust-lang/rust-clippy/issues/15377 + let (funky_mtx) = std::sync::atomic::AtomicU64::new(0); + //~^ mutex_integer +} diff --git a/tests/ui/mutex_atomic.rs b/tests/ui/mutex_atomic.rs index b907c3081841..95f2b135903f 100644 --- a/tests/ui/mutex_atomic.rs +++ b/tests/ui/mutex_atomic.rs @@ -1,4 +1,3 @@ -//@no-rustfix #![warn(clippy::mutex_integer)] #![warn(clippy::mutex_atomic)] #![allow(clippy::borrow_as_ptr)] @@ -54,13 +53,15 @@ fn issue13378() { static MTX: Mutex = Mutex::new(0); //~^ mutex_integer - 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 + + // don't eat the `)` when removing the type ascription -- see + // https://github.com/rust-lang/rust-clippy/issues/15377 + let (funky_mtx): Mutex = Mutex::new(0); + //~^ mutex_integer } diff --git a/tests/ui/mutex_atomic.stderr b/tests/ui/mutex_atomic.stderr index 9d8a7ccb33bd..0afc6d541dea 100644 --- a/tests/ui/mutex_atomic.stderr +++ b/tests/ui/mutex_atomic.stderr @@ -1,5 +1,5 @@ error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:9:13 + --> tests/ui/mutex_atomic.rs:8:13 | LL | let _ = Mutex::new(true); | ^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicBool::new(true)` @@ -9,7 +9,7 @@ LL | let _ = 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:12:13 + --> tests/ui/mutex_atomic.rs:11:13 | LL | let _ = Mutex::new(5usize); | ^^^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicUsize::new(5usize)` @@ -17,7 +17,7 @@ LL | let _ = Mutex::new(5usize); = 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:15:13 + --> tests/ui/mutex_atomic.rs:14:13 | LL | let _ = Mutex::new(9isize); | ^^^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicIsize::new(9isize)` @@ -25,7 +25,7 @@ LL | let _ = Mutex::new(9isize); = 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:22:13 + --> tests/ui/mutex_atomic.rs:21:13 | LL | let _ = Mutex::new(&mut x as *mut u32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicPtr::new(&mut x as *mut u32)` @@ -33,7 +33,7 @@ LL | let _ = Mutex::new(&mut x as *mut u32); = 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:25:13 + --> tests/ui/mutex_atomic.rs:24:13 | LL | let _ = Mutex::new(0u32); | ^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicU32::new(0u32)` @@ -43,7 +43,7 @@ LL | let _ = 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:28:13 + --> tests/ui/mutex_atomic.rs:27:13 | LL | let _ = Mutex::new(0i32); | ^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI32::new(0i32)` @@ -51,7 +51,7 @@ LL | let _ = Mutex::new(0i32); = 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:32:13 + --> tests/ui/mutex_atomic.rs:31:13 | LL | let _ = Mutex::new(0u8); | ^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicU8::new(0u8)` @@ -59,7 +59,7 @@ LL | let _ = Mutex::new(0u8); = 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:35:13 + --> tests/ui/mutex_atomic.rs:34:13 | LL | let _ = Mutex::new(0i16); | ^^^^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI16::new(0i16)` @@ -67,15 +67,20 @@ LL | let _ = Mutex::new(0i16); = 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:38:25 + --> tests/ui/mutex_atomic.rs:37:25 | LL | let _x: Mutex = Mutex::new(0); - | ^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI8::new(0)` + | ^^^^^^^^^^^^^ | = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` +help: try + | +LL - let _x: Mutex = Mutex::new(0); +LL + let _x = std::sync::atomic::AtomicI8::new(0); + | error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:42:13 + --> tests/ui/mutex_atomic.rs:41:13 | LL | let _ = Mutex::new(X); | ^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI64::new(X)` @@ -83,15 +88,20 @@ LL | let _ = Mutex::new(X); = 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:54:30 + --> tests/ui/mutex_atomic.rs:53:30 | LL | static MTX: Mutex = Mutex::new(0); - | ^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicU32::new(0)` + | ^^^^^^^^^^^^^ | = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` +help: try + | +LL - static MTX: Mutex = Mutex::new(0); +LL + static MTX: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + | error: using a `Mutex` where an atomic would do - --> tests/ui/mutex_atomic.rs:60:15 + --> tests/ui/mutex_atomic.rs:56:15 | LL | let mtx = Mutex::new(0); | ^^^^^^^^^^^^^ help: try: `std::sync::atomic::AtomicI32::new(0)` @@ -99,7 +109,7 @@ LL | let mtx = 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:64:22 + --> tests/ui/mutex_atomic.rs:60:22 | LL | let reassigned = mtx; | ^^^ @@ -107,5 +117,18 @@ 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 13 previous errors +error: using a `Mutex` where an atomic would do + --> tests/ui/mutex_atomic.rs:65:35 + | +LL | let (funky_mtx): Mutex = Mutex::new(0); + | ^^^^^^^^^^^^^ + | + = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` +help: try + | +LL - let (funky_mtx): Mutex = Mutex::new(0); +LL + let (funky_mtx) = std::sync::atomic::AtomicU64::new(0); + | + +error: aborting due to 14 previous errors diff --git a/tests/ui/mutex_atomic_unfixable.rs b/tests/ui/mutex_atomic_unfixable.rs new file mode 100644 index 000000000000..0c04f48cf8a9 --- /dev/null +++ b/tests/ui/mutex_atomic_unfixable.rs @@ -0,0 +1,13 @@ +//@no-rustfix +#![warn(clippy::mutex_atomic, clippy::mutex_integer)] + +use std::sync::Mutex; + +fn issue13378() { + static MTX: Mutex = Mutex::new(0); + //~^ mutex_integer + + // unfixable because we don't fix this `lock` + let mut guard = MTX.lock().unwrap(); + *guard += 1; +} diff --git a/tests/ui/mutex_atomic_unfixable.stderr b/tests/ui/mutex_atomic_unfixable.stderr new file mode 100644 index 000000000000..27ffb1304c69 --- /dev/null +++ b/tests/ui/mutex_atomic_unfixable.stderr @@ -0,0 +1,17 @@ +error: using a `Mutex` where an atomic would do + --> tests/ui/mutex_atomic_unfixable.rs:7:30 + | +LL | static MTX: Mutex = Mutex::new(0); + | ^^^^^^^^^^^^^ + | + = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>` + = note: `-D clippy::mutex-integer` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::mutex_integer)]` +help: try + | +LL - static MTX: Mutex = Mutex::new(0); +LL + static MTX: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + | + +error: aborting due to 1 previous error +