diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs index b38cc7b36a1..1c52514a330 100644 --- a/clippy_lints/src/if_let_mutex.rs +++ b/clippy_lints/src/if_let_mutex.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::visitors::for_each_expr_without_closures; -use clippy_utils::{higher, SpanlessEq}; +use clippy_utils::{eq_expr_value, higher}; use core::ops::ControlFlow; use rustc_errors::Diag; use rustc_hir::{Expr, ExprKind}; @@ -51,53 +51,45 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if_else: Some(if_else), .. }) = higher::IfLet::hir(cx, expr) + && let Some(op_mutex) = for_each_expr_without_closures(let_expr, |e| mutex_lock_call(cx, e, None)) + && let Some(arm_mutex) = + for_each_expr_without_closures((if_then, if_else), |e| mutex_lock_call(cx, e, Some(op_mutex))) { - let is_mutex_lock = |e: &'tcx Expr<'tcx>| { - if let Some(mutex) = is_mutex_lock_call(cx, e) { - ControlFlow::Break(mutex) - } else { - ControlFlow::Continue(()) - } + let diag = |diag: &mut Diag<'_, ()>| { + diag.span_label( + op_mutex.span, + "this Mutex will remain locked for the entire `if let`-block...", + ); + diag.span_label( + arm_mutex.span, + "... and is tried to lock again here, which will always deadlock.", + ); + diag.help("move the lock call outside of the `if let ...` expression"); }; - - let op_mutex = for_each_expr_without_closures(let_expr, is_mutex_lock); - if let Some(op_mutex) = op_mutex { - let arm_mutex = for_each_expr_without_closures((if_then, if_else), is_mutex_lock); - if let Some(arm_mutex) = arm_mutex - && SpanlessEq::new(cx).eq_expr(op_mutex, arm_mutex) - { - let diag = |diag: &mut Diag<'_, ()>| { - diag.span_label( - op_mutex.span, - "this Mutex will remain locked for the entire `if let`-block...", - ); - diag.span_label( - arm_mutex.span, - "... and is tried to lock again here, which will always deadlock.", - ); - diag.help("move the lock call outside of the `if let ...` expression"); - }; - span_lint_and_then( - cx, - IF_LET_MUTEX, - expr.span, - "calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock", - diag, - ); - } - } + span_lint_and_then( + cx, + IF_LET_MUTEX, + expr.span, + "calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock", + diag, + ); } } } -fn is_mutex_lock_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { +fn mutex_lock_call<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + op_mutex: Option<&'tcx Expr<'_>>, +) -> ControlFlow<&'tcx Expr<'tcx>> { if let ExprKind::MethodCall(path, self_arg, ..) = &expr.kind && path.ident.as_str() == "lock" && let ty = cx.typeck_results().expr_ty(self_arg).peel_refs() && is_type_diagnostic_item(cx, ty, sym::Mutex) + && op_mutex.map_or(true, |op| eq_expr_value(cx, self_arg, op)) { - Some(self_arg) + ControlFlow::Break(self_arg) } else { - None + ControlFlow::Continue(()) } } diff --git a/tests/ui/if_let_mutex.rs b/tests/ui/if_let_mutex.rs index cb6915e0eba..bb0eadfca1c 100644 --- a/tests/ui/if_let_mutex.rs +++ b/tests/ui/if_let_mutex.rs @@ -1,4 +1,5 @@ #![warn(clippy::if_let_mutex)] +#![allow(clippy::redundant_pattern_matching)] use std::ops::Deref; use std::sync::Mutex; @@ -50,4 +51,12 @@ fn mutex_ref(mutex: &Mutex) { }; } +fn multiple_mutexes(m1: &Mutex<()>, m2: &Mutex<()>) { + if let Ok(_) = m1.lock() { + m2.lock(); + } else { + m1.lock(); + } +} + fn main() {} diff --git a/tests/ui/if_let_mutex.stderr b/tests/ui/if_let_mutex.stderr index 6e0115c23af..45df4ac4d67 100644 --- a/tests/ui/if_let_mutex.stderr +++ b/tests/ui/if_let_mutex.stderr @@ -1,5 +1,5 @@ error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock - --> tests/ui/if_let_mutex.rs:10:5 + --> tests/ui/if_let_mutex.rs:11:5 | LL | if let Err(locked) = m.lock() { | ^ - this Mutex will remain locked for the entire `if let`-block... @@ -19,7 +19,7 @@ LL | | }; = help: to override `-D warnings` add `#[allow(clippy::if_let_mutex)]` error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock - --> tests/ui/if_let_mutex.rs:23:5 + --> tests/ui/if_let_mutex.rs:24:5 | LL | if let Some(locked) = m.lock().unwrap().deref() { | ^ - this Mutex will remain locked for the entire `if let`-block... @@ -37,7 +37,7 @@ LL | | }; = help: move the lock call outside of the `if let ...` expression error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock - --> tests/ui/if_let_mutex.rs:45:5 + --> tests/ui/if_let_mutex.rs:46:5 | LL | if let Ok(i) = mutex.lock() { | ^ ----- this Mutex will remain locked for the entire `if let`-block... @@ -53,5 +53,21 @@ LL | | }; | = help: move the lock call outside of the `if let ...` expression -error: aborting due to 3 previous errors +error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock + --> tests/ui/if_let_mutex.rs:55:5 + | +LL | if let Ok(_) = m1.lock() { + | ^ -- this Mutex will remain locked for the entire `if let`-block... + | _____| + | | +LL | | m2.lock(); +LL | | } else { +LL | | m1.lock(); + | | -- ... and is tried to lock again here, which will always deadlock. +LL | | } + | |_____^ + | + = help: move the lock call outside of the `if let ...` expression + +error: aborting due to 4 previous errors