Add labels to if_let_mutex

This commit is contained in:
Lukas Lueg 2022-08-10 20:56:44 +02:00
parent 6a73a45418
commit 0428f0d234
2 changed files with 40 additions and 26 deletions

View File

@ -1,8 +1,9 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::SpanlessEq;
use if_chain::if_chain;
use rustc_errors::Diagnostic;
use rustc_hir::intravisit::{self as visit, Visitor};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
@ -45,16 +46,8 @@ declare_lint_pass!(IfLetMutex => [IF_LET_MUTEX]);
impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
let mut arm_visit = ArmVisitor {
mutex_lock_called: false,
found_mutex: None,
cx,
};
let mut op_visit = OppVisitor {
mutex_lock_called: false,
found_mutex: None,
cx,
};
let mut arm_visit = ArmVisitor { found_mutex: None, cx };
let mut op_visit = OppVisitor { found_mutex: None, cx };
if let Some(higher::IfLet {
let_expr,
if_then,
@ -63,18 +56,28 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
}) = higher::IfLet::hir(cx, expr)
{
op_visit.visit_expr(let_expr);
if op_visit.mutex_lock_called {
if let Some(op_mutex) = op_visit.found_mutex {
arm_visit.visit_expr(if_then);
arm_visit.visit_expr(if_else);
if arm_visit.mutex_lock_called && arm_visit.same_mutex(cx, op_visit.found_mutex.unwrap()) {
span_lint_and_help(
if let Some(arm_mutex) = arm_visit.found_mutex_if_same_as(op_mutex) {
let diag = |diag: &mut Diagnostic| {
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",
None,
"move the lock call outside of the `if let ...` expression",
diag,
);
}
}
@ -84,7 +87,6 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
/// Checks if `Mutex::lock` is called in the `if let` expr.
pub struct OppVisitor<'a, 'tcx> {
mutex_lock_called: bool,
found_mutex: Option<&'tcx Expr<'tcx>>,
cx: &'a LateContext<'tcx>,
}
@ -93,7 +95,6 @@ impl<'tcx> Visitor<'tcx> for OppVisitor<'_, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if let Some(mutex) = is_mutex_lock_call(self.cx, expr) {
self.found_mutex = Some(mutex);
self.mutex_lock_called = true;
return;
}
visit::walk_expr(self, expr);
@ -102,7 +103,6 @@ impl<'tcx> Visitor<'tcx> for OppVisitor<'_, 'tcx> {
/// Checks if `Mutex::lock` is called in any of the branches.
pub struct ArmVisitor<'a, 'tcx> {
mutex_lock_called: bool,
found_mutex: Option<&'tcx Expr<'tcx>>,
cx: &'a LateContext<'tcx>,
}
@ -111,7 +111,6 @@ impl<'tcx> Visitor<'tcx> for ArmVisitor<'_, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if let Some(mutex) = is_mutex_lock_call(self.cx, expr) {
self.found_mutex = Some(mutex);
self.mutex_lock_called = true;
return;
}
visit::walk_expr(self, expr);
@ -119,9 +118,12 @@ impl<'tcx> Visitor<'tcx> for ArmVisitor<'_, 'tcx> {
}
impl<'tcx, 'l> ArmVisitor<'tcx, 'l> {
fn same_mutex(&self, cx: &LateContext<'_>, op_mutex: &Expr<'_>) -> bool {
self.found_mutex
.map_or(false, |arm_mutex| SpanlessEq::new(cx).eq_expr(op_mutex, arm_mutex))
fn found_mutex_if_same_as(&self, op_mutex: &Expr<'_>) -> Option<&Expr<'_>> {
self.found_mutex.and_then(|arm_mutex| {
SpanlessEq::new(self.cx)
.eq_expr(op_mutex, arm_mutex)
.then_some(arm_mutex)
})
}
}

View File

@ -1,10 +1,14 @@
error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock
--> $DIR/if_let_mutex.rs:10:5
|
LL | / if let Err(locked) = m.lock() {
LL | if let Err(locked) = m.lock() {
| ^ - This Mutex will remain locked for the entire `if let`-block...
| _____|
| |
LL | | do_stuff(locked);
LL | | } else {
LL | | let lock = m.lock().unwrap();
| | - ... and is tried to lock again here, which will always deadlock.
LL | | do_stuff(lock);
LL | | };
| |_____^
@ -15,10 +19,14 @@ LL | | };
error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock
--> $DIR/if_let_mutex.rs:22:5
|
LL | / if let Some(locked) = m.lock().unwrap().deref() {
LL | if let Some(locked) = m.lock().unwrap().deref() {
| ^ - This Mutex will remain locked for the entire `if let`-block...
| _____|
| |
LL | | do_stuff(locked);
LL | | } else {
LL | | let lock = m.lock().unwrap();
| | - ... and is tried to lock again here, which will always deadlock.
LL | | do_stuff(lock);
LL | | };
| |_____^
@ -28,10 +36,14 @@ LL | | };
error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock
--> $DIR/if_let_mutex.rs:43:5
|
LL | / if let Ok(i) = mutex.lock() {
LL | if let Ok(i) = mutex.lock() {
| ^ ----- This Mutex will remain locked for the entire `if let`-block...
| _____|
| |
LL | | do_stuff(i);
LL | | } else {
LL | | let _x = mutex.lock();
| | ----- ... and is tried to lock again here, which will always deadlock.
LL | | };
| |_____^
|