From a248b2c1263bc3895f749a23a00cba99a1280445 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 14 Nov 2024 22:02:12 +0500 Subject: [PATCH] Deprecated if_let_mutex due to stabilization in 2024 edition --- clippy_lints/src/declared_lints.rs | 1 - clippy_lints/src/deprecated_lints.rs | 2 + clippy_lints/src/if_let_mutex.rs | 95 ---------------------------- clippy_lints/src/lib.rs | 2 - tests/ui/deprecated.rs | 1 + tests/ui/if_let_mutex.rs | 62 ------------------ tests/ui/if_let_mutex.stderr | 73 --------------------- 7 files changed, 3 insertions(+), 233 deletions(-) delete mode 100644 clippy_lints/src/if_let_mutex.rs delete mode 100644 tests/ui/if_let_mutex.rs delete mode 100644 tests/ui/if_let_mutex.stderr diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index dff60f76b746..615ab12dc8c1 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -212,7 +212,6 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::functions::TOO_MANY_ARGUMENTS_INFO, crate::functions::TOO_MANY_LINES_INFO, crate::future_not_send::FUTURE_NOT_SEND_INFO, - crate::if_let_mutex::IF_LET_MUTEX_INFO, crate::if_not_else::IF_NOT_ELSE_INFO, crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO, crate::ignored_unit_patterns::IGNORED_UNIT_PATTERNS_INFO, diff --git a/clippy_lints/src/deprecated_lints.rs b/clippy_lints/src/deprecated_lints.rs index 77dbe9b78a18..78739ae32364 100644 --- a/clippy_lints/src/deprecated_lints.rs +++ b/clippy_lints/src/deprecated_lints.rs @@ -40,6 +40,8 @@ declare_with_version! { DEPRECATED(DEPRECATED_VERSION): &[(&str, &str)] = &[ ("clippy::pub_enum_variant_names", "`clippy::enum_variant_names` now covers this case via the `avoid-breaking-exported-api` config"), #[clippy::version = "1.54.0"] ("clippy::wrong_pub_self_convention", "`clippy::wrong_self_convention` now covers this case via the `avoid-breaking-exported-api` config"), + #[clippy::version = "1.83.0"] + ("clippy::if_let_mutex", "`if_let_rescope` is now stable [#131154](https://github.com/rust-lang/rust/issues/131154). The Mutex lock does not held for the whole `if let ... else` block. There are no deadlocks on this reason."), // end deprecated lints. used by `cargo dev deprecate_lint` ]} diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs deleted file mode 100644 index ba80c099a015..000000000000 --- a/clippy_lints/src/if_let_mutex.rs +++ /dev/null @@ -1,95 +0,0 @@ -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::{eq_expr_value, higher}; -use core::ops::ControlFlow; -use rustc_errors::Diag; -use rustc_hir::{Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::declare_lint_pass; -use rustc_span::sym; - -declare_clippy_lint! { - /// ### What it does - /// Checks for `Mutex::lock` calls in `if let` expression - /// with lock calls in any of the else blocks. - /// - /// ### Why is this bad? - /// The Mutex lock remains held for the whole - /// `if let ... else` block and deadlocks. - /// - /// ### Example - /// ```rust,ignore - /// if let Ok(thing) = mutex.lock() { - /// do_thing(); - /// } else { - /// mutex.lock(); - /// } - /// ``` - /// Should be written - /// ```rust,ignore - /// let locked = mutex.lock(); - /// if let Ok(thing) = locked { - /// do_thing(thing); - /// } else { - /// use_locked(locked); - /// } - /// ``` - #[clippy::version = "1.45.0"] - pub IF_LET_MUTEX, - correctness, - "locking a `Mutex` in an `if let` block can cause deadlocks" -} - -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>) { - if let Some(higher::IfLet { - let_expr, - if_then, - 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 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, - ); - } - } -} - -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)) - { - ControlFlow::Break(self_arg) - } else { - ControlFlow::Continue(()) - } -} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c9064df25ac8..d07c9083d611 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -152,7 +152,6 @@ mod from_raw_with_void_ptr; mod from_str_radix_10; mod functions; mod future_not_send; -mod if_let_mutex; mod if_not_else; mod if_then_some_else_none; mod ignored_unit_patterns; @@ -791,7 +790,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(option_if_let_else::OptionIfLetElse)); store.register_late_pass(|_| Box::new(future_not_send::FutureNotSend)); store.register_late_pass(move |_| Box::new(large_futures::LargeFuture::new(conf))); - store.register_late_pass(|_| Box::new(if_let_mutex::IfLetMutex)); store.register_late_pass(|_| Box::new(if_not_else::IfNotElse)); store.register_late_pass(|_| Box::new(equatable_if_let::PatternEquality)); store.register_late_pass(|_| Box::new(manual_async_fn::ManualAsyncFn)); diff --git a/tests/ui/deprecated.rs b/tests/ui/deprecated.rs index 5617db90a470..218e8663a74c 100644 --- a/tests/ui/deprecated.rs +++ b/tests/ui/deprecated.rs @@ -15,5 +15,6 @@ #![warn(clippy::regex_macro)] //~ ERROR: lint `clippy::regex_macro` #![warn(clippy::pub_enum_variant_names)] //~ ERROR: lint `clippy::pub_enum_variant_names` #![warn(clippy::wrong_pub_self_convention)] //~ ERROR: lint `clippy::wrong_pub_self_convention` +#![warn(clippy::if_let_mutex)] //~ ERROR: lint `clippy::if_let_mutex` fn main() {} diff --git a/tests/ui/if_let_mutex.rs b/tests/ui/if_let_mutex.rs deleted file mode 100644 index bb0eadfca1c7..000000000000 --- a/tests/ui/if_let_mutex.rs +++ /dev/null @@ -1,62 +0,0 @@ -#![warn(clippy::if_let_mutex)] -#![allow(clippy::redundant_pattern_matching)] - -use std::ops::Deref; -use std::sync::Mutex; - -fn do_stuff(_: T) {} - -fn if_let() { - let m = Mutex::new(1_u8); - if let Err(locked) = m.lock() { - //~^ ERROR: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a d - do_stuff(locked); - } else { - let lock = m.lock().unwrap(); - do_stuff(lock); - }; -} - -// This is the most common case as the above case is pretty -// contrived. -fn if_let_option() { - let m = Mutex::new(Some(0_u8)); - if let Some(locked) = m.lock().unwrap().deref() { - //~^ ERROR: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a d - do_stuff(locked); - } else { - let lock = m.lock().unwrap(); - do_stuff(lock); - }; -} - -// When mutexes are different don't warn -fn if_let_different_mutex() { - let m = Mutex::new(Some(0_u8)); - let other = Mutex::new(None::); - if let Some(locked) = m.lock().unwrap().deref() { - do_stuff(locked); - } else { - let lock = other.lock().unwrap(); - do_stuff(lock); - }; -} - -fn mutex_ref(mutex: &Mutex) { - if let Ok(i) = mutex.lock() { - //~^ ERROR: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a d - do_stuff(i); - } else { - let _x = mutex.lock(); - }; -} - -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 deleted file mode 100644 index 45df4ac4d679..000000000000 --- a/tests/ui/if_let_mutex.stderr +++ /dev/null @@ -1,73 +0,0 @@ -error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock - --> 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... - | _____| - | | -LL | | -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 | | }; - | |_____^ - | - = help: move the lock call outside of the `if let ...` expression - = note: `-D clippy::if-let-mutex` implied by `-D warnings` - = 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:24:5 - | -LL | if let Some(locked) = m.lock().unwrap().deref() { - | ^ - this Mutex will remain locked for the entire `if let`-block... - | _____| - | | -LL | | -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 | | }; - | |_____^ - | - = 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:46:5 - | -LL | if let Ok(i) = mutex.lock() { - | ^ ----- this Mutex will remain locked for the entire `if let`-block... - | _____| - | | -LL | | -LL | | do_stuff(i); -LL | | } else { -LL | | let _x = mutex.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: 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 -