Skip to content

Commit

Permalink
Fix mut_mutex_lock when reference not ultimately mutable
Browse files Browse the repository at this point in the history
When there is are multiple references where one of the references
isn't mutable then this results in a false-positive for
`mut_mutex_lock` as it only checks the mutability of the first
reference level.

Fix this by using `peel_mid_ty_refs_is_mutable` which correctly
determines whether the reference is ultimately mutable and thus
whether `Mutex::get_lock()` can actually be used.

Fixes #9854
  • Loading branch information
rshearman authored and blyxyas committed Oct 1, 2024
1 parent 71c7d44 commit c88cb08
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
6 changes: 3 additions & 3 deletions clippy_lints/src/methods/mut_mutex_lock.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::expr_custom_deref_adjustment;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable};
use rustc_errors::Applicability;
use rustc_hir::{Expr, Mutability};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::{Span, sym};

use super::MUT_MUTEX_LOCK;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>, recv: &'tcx Expr<'tcx>, name_span: Span) {
if matches!(expr_custom_deref_adjustment(cx, recv), None | Some(Mutability::Mut))
&& let ty::Ref(_, _, Mutability::Mut) = cx.typeck_results().expr_ty(recv).kind()
&& let (_, ref_depth, Mutability::Mut) = peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(recv))
&& ref_depth >= 1
&& let Some(method_id) = cx.typeck_results().type_dependent_def_id(ex.hir_id)
&& let Some(impl_id) = cx.tcx.impl_of_method(method_id)
&& is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id).instantiate_identity(), sym::Mutex)
Expand Down
12 changes: 12 additions & 0 deletions tests/ui/mut_mutex_lock.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ fn mut_mutex_lock() {

let mut value = value_mutex.get_mut().unwrap();
*value += 1;

let mut value_mutex = Mutex::new(42_u8);
let mut_ref_mut_ref_mutex = &mut &mut value_mutex;
let mut value = mut_ref_mut_ref_mutex.get_mut().unwrap();
*value += 1;
}

fn no_owned_mutex_lock() {
Expand All @@ -24,4 +29,11 @@ fn issue9415() {
*guard += 1;
}

fn mut_ref_ref_mutex_lock() {
let mutex = Mutex::new(42_u8);
let mut_ref_ref_mutex = &mut &mutex;
let mut guard = mut_ref_ref_mutex.lock().unwrap();
*guard += 1;
}

fn main() {}
12 changes: 12 additions & 0 deletions tests/ui/mut_mutex_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ fn mut_mutex_lock() {

let mut value = value_mutex.lock().unwrap();
*value += 1;

let mut value_mutex = Mutex::new(42_u8);
let mut_ref_mut_ref_mutex = &mut &mut value_mutex;
let mut value = mut_ref_mut_ref_mutex.lock().unwrap();
*value += 1;
}

fn no_owned_mutex_lock() {
Expand All @@ -24,4 +29,11 @@ fn issue9415() {
*guard += 1;
}

fn mut_ref_ref_mutex_lock() {
let mutex = Mutex::new(42_u8);
let mut_ref_ref_mutex = &mut &mutex;
let mut guard = mut_ref_ref_mutex.lock().unwrap();
*guard += 1;
}

fn main() {}
8 changes: 7 additions & 1 deletion tests/ui/mut_mutex_lock.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,11 @@ LL | let mut value = value_mutex.lock().unwrap();
= note: `-D clippy::mut-mutex-lock` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::mut_mutex_lock)]`

error: aborting due to 1 previous error
error: calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference
--> tests/ui/mut_mutex_lock.rs:15:43
|
LL | let mut value = mut_ref_mut_ref_mutex.lock().unwrap();
| ^^^^ help: change this to: `get_mut`

error: aborting due to 2 previous errors

0 comments on commit c88cb08

Please sign in to comment.