Skip to content

Commit 2be695b

Browse files
committed
[significant_drop_tightening] Fix #11128
1 parent c7bf05c commit 2be695b

File tree

5 files changed

+56
-8
lines changed

5 files changed

+56
-8
lines changed

clippy_lints/src/significant_drop_tightening.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
use clippy_utils::{
22
diagnostics::span_lint_and_then,
3-
expr_or_init, get_attr, path_to_local,
3+
expr_or_init, get_attr, match_def_path, path_to_local, paths,
44
source::{indent_of, snippet},
55
};
66
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
77
use rustc_errors::Applicability;
88
use rustc_hir::{
99
self as hir,
10+
def::{DefKind, Res},
1011
intravisit::{walk_expr, Visitor},
1112
};
1213
use rustc_lint::{LateContext, LateLintPass, LintContext};
@@ -333,7 +334,7 @@ impl<'ap, 'lc, 'others, 'stmt, 'tcx> Visitor<'tcx> for StmtsChecker<'ap, 'lc, 'o
333334
}
334335
},
335336
hir::StmtKind::Semi(expr) => {
336-
if has_drop(expr, &apa.first_bind_ident) {
337+
if has_drop(expr, &apa.first_bind_ident, self.cx) {
337338
apa.has_expensive_expr_after_last_attr = false;
338339
apa.last_stmt_span = DUMMY_SP;
339340
return;
@@ -430,11 +431,11 @@ fn dummy_stmt_expr<'any>(expr: &'any hir::Expr<'any>) -> hir::Stmt<'any> {
430431
}
431432
}
432433

433-
fn has_drop(expr: &hir::Expr<'_>, first_bind_ident: &Ident) -> bool {
434+
fn has_drop(expr: &hir::Expr<'_>, first_bind_ident: &Ident, lcx: &LateContext<'_>) -> bool {
434435
if let hir::ExprKind::Call(fun, args) = expr.kind
435436
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, fun_path)) = &fun.kind
436-
&& let [fun_ident, ..] = fun_path.segments
437-
&& fun_ident.ident.name == rustc_span::sym::drop
437+
&& let Res::Def(DefKind::Fn, did) = fun_path.res
438+
&& match_def_path(lcx, did, &paths::DROP)
438439
&& let [first_arg, ..] = args
439440
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &first_arg.kind
440441
&& let [first_arg_ps, .. ] = arg_path.segments

clippy_utils/src/paths.rs

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"];
5757
pub const LATE_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "LateLintPass"];
5858
#[cfg(feature = "internal")]
5959
pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"];
60+
pub const DROP: [&str; 3] = ["core", "mem", "drop"];
6061
pub const MEM_SWAP: [&str; 3] = ["core", "mem", "swap"];
6162
#[cfg(feature = "internal")]
6263
pub const MSRV: [&str; 3] = ["clippy_utils", "msrvs", "Msrv"];

tests/ui/significant_drop_tightening.fixed

+23
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,29 @@ pub fn issue_10413() {
2828
}
2929
}
3030

31+
pub fn issue_11128() {
32+
use std::mem::drop as unlock;
33+
34+
struct Foo {
35+
droppable: Option<Vec<i32>>,
36+
mutex: Mutex<Vec<i32>>,
37+
}
38+
39+
impl Drop for Foo {
40+
fn drop(&mut self) {
41+
if let Some(droppable) = self.droppable.take() {
42+
let lock = self.mutex.lock().unwrap();
43+
let idx_opt = lock.iter().copied().find(|el| Some(el) == droppable.first());
44+
if let Some(idx) = idx_opt {
45+
let local_droppable = vec![lock.first().copied().unwrap_or_default()];
46+
unlock(lock);
47+
drop(local_droppable);
48+
}
49+
}
50+
}
51+
}
52+
}
53+
3154
pub fn path_return_can_be_ignored() -> i32 {
3255
let mutex = Mutex::new(1);
3356
let lock = mutex.lock().unwrap();

tests/ui/significant_drop_tightening.rs

+23
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,29 @@ pub fn issue_10413() {
2727
}
2828
}
2929

30+
pub fn issue_11128() {
31+
use std::mem::drop as unlock;
32+
33+
struct Foo {
34+
droppable: Option<Vec<i32>>,
35+
mutex: Mutex<Vec<i32>>,
36+
}
37+
38+
impl Drop for Foo {
39+
fn drop(&mut self) {
40+
if let Some(droppable) = self.droppable.take() {
41+
let lock = self.mutex.lock().unwrap();
42+
let idx_opt = lock.iter().copied().find(|el| Some(el) == droppable.first());
43+
if let Some(idx) = idx_opt {
44+
let local_droppable = vec![lock.first().copied().unwrap_or_default()];
45+
unlock(lock);
46+
drop(local_droppable);
47+
}
48+
}
49+
}
50+
}
51+
}
52+
3053
pub fn path_return_can_be_ignored() -> i32 {
3154
let mutex = Mutex::new(1);
3255
let lock = mutex.lock().unwrap();

tests/ui/significant_drop_tightening.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ LL + drop(lock);
2323
|
2424

2525
error: temporary with significant `Drop` can be early dropped
26-
--> $DIR/significant_drop_tightening.rs:56:13
26+
--> $DIR/significant_drop_tightening.rs:79:13
2727
|
2828
LL | / {
2929
LL | | let mutex = Mutex::new(1i32);
@@ -43,7 +43,7 @@ LL + drop(lock);
4343
|
4444

4545
error: temporary with significant `Drop` can be early dropped
46-
--> $DIR/significant_drop_tightening.rs:77:13
46+
--> $DIR/significant_drop_tightening.rs:100:13
4747
|
4848
LL | / {
4949
LL | | let mutex = Mutex::new(1i32);
@@ -67,7 +67,7 @@ LL +
6767
|
6868

6969
error: temporary with significant `Drop` can be early dropped
70-
--> $DIR/significant_drop_tightening.rs:83:17
70+
--> $DIR/significant_drop_tightening.rs:106:17
7171
|
7272
LL | / {
7373
LL | | let mutex = Mutex::new(vec![1i32]);

0 commit comments

Comments
 (0)