Auto merge of #11129 - c410-f3r:lock-1, r=Manishearth

[significant_drop_tightening] Fix #11128 Fix #11128 ``` changelog: [`significant_drop_tightening`]: Consider manual alias of the `drop` function. ```
rust-lang · Jul 8, 2023 · 8fcd476 · 8fcd476
2 parents 8e261c0 + 2be695b
commit 8fcd476
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 8 deletions.
diff --git a/clippy_lints/src/significant_drop_tightening.rs b/clippy_lints/src/significant_drop_tightening.rs
@@ -1,12 +1,13 @@
 use clippy_utils::{
     diagnostics::span_lint_and_then,
-    expr_or_init, get_attr, path_to_local,
+    expr_or_init, get_attr, match_def_path, path_to_local, paths,
     source::{indent_of, snippet},
 };
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
 use rustc_errors::Applicability;
 use rustc_hir::{
     self as hir,
+    def::{DefKind, Res},
     intravisit::{walk_expr, Visitor},
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
@@ -333,7 +334,7 @@ impl<'ap, 'lc, 'others, 'stmt, 'tcx> Visitor<'tcx> for StmtsChecker<'ap, 'lc, 'o
                         }
                     },
                     hir::StmtKind::Semi(expr) => {
-                        if has_drop(expr, &apa.first_bind_ident) {
+                        if has_drop(expr, &apa.first_bind_ident, self.cx) {
                             apa.has_expensive_expr_after_last_attr = false;
                             apa.last_stmt_span = DUMMY_SP;
                             return;
@@ -430,11 +431,11 @@ fn dummy_stmt_expr<'any>(expr: &'any hir::Expr<'any>) -> hir::Stmt<'any> {
     }
 }
 
-fn has_drop(expr: &hir::Expr<'_>, first_bind_ident: &Ident) -> bool {
+fn has_drop(expr: &hir::Expr<'_>, first_bind_ident: &Ident, lcx: &LateContext<'_>) -> bool {
     if let hir::ExprKind::Call(fun, args) = expr.kind
         && let hir::ExprKind::Path(hir::QPath::Resolved(_, fun_path)) = &fun.kind
-        && let [fun_ident, ..] = fun_path.segments
-        && fun_ident.ident.name == rustc_span::sym::drop
+        && let Res::Def(DefKind::Fn, did) = fun_path.res
+        && match_def_path(lcx, did, &paths::DROP)
         && let [first_arg, ..] = args
         && let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &first_arg.kind
         && let [first_arg_ps, .. ] = arg_path.segments

diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs
@@ -57,6 +57,7 @@ pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"];
 pub const LATE_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "LateLintPass"];
 #[cfg(feature = "internal")]
 pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"];
+pub const DROP: [&str; 3] = ["core", "mem", "drop"];
 pub const MEM_SWAP: [&str; 3] = ["core", "mem", "swap"];
 #[cfg(feature = "internal")]
 pub const MSRV: [&str; 3] = ["clippy_utils", "msrvs", "Msrv"];

diff --git a/tests/ui/significant_drop_tightening.fixed b/tests/ui/significant_drop_tightening.fixed
@@ -28,6 +28,29 @@ pub fn issue_10413() {
     }
 }
 
+pub fn issue_11128() {
+    use std::mem::drop as unlock;
+
+    struct Foo {
+        droppable: Option<Vec<i32>>,
+        mutex: Mutex<Vec<i32>>,
+    }
+
+    impl Drop for Foo {
+        fn drop(&mut self) {
+            if let Some(droppable) = self.droppable.take() {
+                let lock = self.mutex.lock().unwrap();
+                let idx_opt = lock.iter().copied().find(|el| Some(el) == droppable.first());
+                if let Some(idx) = idx_opt {
+                    let local_droppable = vec![lock.first().copied().unwrap_or_default()];
+                    unlock(lock);
+                    drop(local_droppable);
+                }
+            }
+        }
+    }
+}
+
 pub fn path_return_can_be_ignored() -> i32 {
     let mutex = Mutex::new(1);
     let lock = mutex.lock().unwrap();

diff --git a/tests/ui/significant_drop_tightening.rs b/tests/ui/significant_drop_tightening.rs
@@ -27,6 +27,29 @@ pub fn issue_10413() {
     }
 }
 
+pub fn issue_11128() {
+    use std::mem::drop as unlock;
+
+    struct Foo {
+        droppable: Option<Vec<i32>>,
+        mutex: Mutex<Vec<i32>>,
+    }
+
+    impl Drop for Foo {
+        fn drop(&mut self) {
+            if let Some(droppable) = self.droppable.take() {
+                let lock = self.mutex.lock().unwrap();
+                let idx_opt = lock.iter().copied().find(|el| Some(el) == droppable.first());
+                if let Some(idx) = idx_opt {
+                    let local_droppable = vec![lock.first().copied().unwrap_or_default()];
+                    unlock(lock);
+                    drop(local_droppable);
+                }
+            }
+        }
+    }
+}
+
 pub fn path_return_can_be_ignored() -> i32 {
     let mutex = Mutex::new(1);
     let lock = mutex.lock().unwrap();

diff --git a/tests/ui/significant_drop_tightening.stderr b/tests/ui/significant_drop_tightening.stderr
@@ -23,7 +23,7 @@ LL +     drop(lock);
    |
 
 error: temporary with significant `Drop` can be early dropped
-  --> $DIR/significant_drop_tightening.rs:56:13
+  --> $DIR/significant_drop_tightening.rs:79:13
    |
 LL | /     {
 LL | |         let mutex = Mutex::new(1i32);
@@ -43,7 +43,7 @@ LL +         drop(lock);
    |
 
 error: temporary with significant `Drop` can be early dropped
-  --> $DIR/significant_drop_tightening.rs:77:13
+  --> $DIR/significant_drop_tightening.rs:100:13
    |
 LL | /     {
 LL | |         let mutex = Mutex::new(1i32);
@@ -67,7 +67,7 @@ LL +
    |
 
 error: temporary with significant `Drop` can be early dropped
-  --> $DIR/significant_drop_tightening.rs:83:17
+  --> $DIR/significant_drop_tightening.rs:106:17
    |
 LL | /     {
 LL | |         let mutex = Mutex::new(vec![1i32]);