diff --git a/compiler/rustc_lint/src/let_underscore.rs b/compiler/rustc_lint/src/let_underscore.rs index 44242173c008..81906a24d902 100644 --- a/compiler/rustc_lint/src/let_underscore.rs +++ b/compiler/rustc_lint/src/let_underscore.rs @@ -1,5 +1,7 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_hir as hir; +use rustc_middle::ty::{self, subst::GenericArgKind}; +use rustc_span::Symbol; declare_lint! { /// The `let_underscore_drop` lint checks for statements which don't bind @@ -43,7 +45,53 @@ declare_lint! { "non-binding let on a type that implements `Drop`" } -declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP]); +declare_lint! { + /// The `let_underscore_lock` lint checks for statements which don't bind + /// a mutex to anything, causing the lock to be released immediately instead + /// of at end of scope, which is typically incorrect. + /// + /// ### Example + /// ```rust + /// use std::sync::{Arc, Mutex}; + /// use std::thread; + /// let data = Arc::new(Mutex::new(0)); + /// + /// thread::spawn(move || { + /// // The lock is immediately released instead of at the end of the + /// // scope, which is probably not intended. + /// let _ = data.lock().unwrap(); + /// println!("doing some work"); + /// let mut lock = data.lock().unwrap(); + /// *lock += 1; + /// }); + /// ``` + /// ### Explanation + /// + /// Statements which assign an expression to an underscore causes the + /// expression to immediately drop instead of extending the expression's + /// lifetime to the end of the scope. This is usually unintended, + /// especially for types like `MutexGuard`, which are typically used to + /// lock a mutex for the duration of an entire scope. + /// + /// If you want to extend the expression's lifetime to the end of the scope, + /// assign an underscore-prefixed name (such as `_foo`) to the expression. + /// If you do actually want to drop the expression immediately, then + /// calling `std::mem::drop` on the expression is clearer and helps convey + /// intent. + pub LET_UNDERSCORE_LOCK, + Warn, + "non-binding let on a synchronization lock" +} + +declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK]); + +const SYNC_GUARD_PATHS: [&[&str]; 5] = [ + &["std", "sync", "mutex", "MutexGuard"], + &["std", "sync", "rwlock", "RwLockReadGuard"], + &["std", "sync", "rwlock", "RwLockWriteGuard"], + &["parking_lot", "raw_mutex", "RawMutex"], + &["parking_lot", "raw_rwlock", "RawRwLock"], +]; impl<'tcx> LateLintPass<'tcx> for LetUnderscore { fn check_local(&mut self, cx: &LateContext<'_>, local: &hir::Local<'_>) { @@ -53,7 +101,27 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore { if let Some(init) = local.init { let init_ty = cx.typeck_results().expr_ty(init); let needs_drop = init_ty.needs_drop(cx.tcx, cx.param_env); - if needs_drop { + let is_sync_lock = init_ty.walk().any(|inner| match inner.unpack() { + GenericArgKind::Type(inner_ty) => { + SYNC_GUARD_PATHS.iter().any(|guard_path| match inner_ty.kind() { + ty::Adt(adt, _) => { + let ty_path = cx.get_def_path(adt.did()); + guard_path.iter().map(|x| Symbol::intern(x)).eq(ty_path.iter().copied()) + } + _ => false, + }) + } + + GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false, + }); + if is_sync_lock { + cx.struct_span_lint(LET_UNDERSCORE_LOCK, local.span, |lint| { + lint.build("non-binding let on a synchronization lock") + .help("consider binding to an unused variable") + .help("consider explicitly droping with `std::mem::drop`") + .emit(); + }) + } else if needs_drop { cx.struct_span_lint(LET_UNDERSCORE_DROP, local.span, |lint| { lint.build("non-binding let on a type that implements `Drop`") .help("consider binding to an unused variable") diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 55396b36dbc2..79661c0fefe8 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -317,7 +317,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { REDUNDANT_SEMICOLONS ); - add_lint_group!("let_underscore", LET_UNDERSCORE_DROP); + add_lint_group!("let_underscore", LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK); add_lint_group!( "rust_2018_idioms", diff --git a/src/test/ui/let_underscore_lock.rs b/src/test/ui/let_underscore_lock.rs new file mode 100644 index 000000000000..774b610db2fb --- /dev/null +++ b/src/test/ui/let_underscore_lock.rs @@ -0,0 +1,8 @@ +// run-pass + +use std::sync::{Arc, Mutex}; + +fn main() { + let data = Arc::new(Mutex::new(0)); + let _ = data.lock().unwrap(); //~WARNING non-binding let on a synchronization lock +} diff --git a/src/test/ui/let_underscore_lock.stderr b/src/test/ui/let_underscore_lock.stderr new file mode 100644 index 000000000000..77379d8c3db2 --- /dev/null +++ b/src/test/ui/let_underscore_lock.stderr @@ -0,0 +1,12 @@ +warning: non-binding let on a synchronization lock + --> $DIR/let_underscore_lock.rs:7:5 + | +LL | let _ = data.lock().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(let_underscore_lock)]` on by default + = help: consider binding to an unused variable + = help: consider explicitly droping with `std::mem::drop` + +warning: 1 warning emitted +