diff --git a/clippy_lints/src/significant_drop_tightening.rs b/clippy_lints/src/significant_drop_tightening.rs index 367466b7d82..e2d90edec5a 100644 --- a/clippy_lints/src/significant_drop_tightening.rs +++ b/clippy_lints/src/significant_drop_tightening.rs @@ -61,10 +61,26 @@ pub struct SignificantDropTightening<'tcx> { } impl<'tcx> SignificantDropTightening<'tcx> { + /// Unifies the statements of a block with its return expression. + fn all_block_stmts<'ret, 'rslt, 'stmts>( + block_stmts: &'stmts [hir::Stmt<'tcx>], + dummy_ret_stmt: Option<&'ret hir::Stmt<'tcx>>, + ) -> impl Iterator> + where + 'ret: 'rslt, + 'stmts: 'rslt, + { + block_stmts.iter().chain(dummy_ret_stmt) + } + /// Searches for at least one statement that could slow down the release of a significant drop. - fn at_least_one_stmt_is_expensive(stmts: &[hir::Stmt<'_>]) -> bool { + fn at_least_one_stmt_is_expensive<'stmt>(stmts: impl Iterator>) -> bool + where + 'tcx: 'stmt, + { for stmt in stmts { match stmt.kind { + hir::StmtKind::Expr(expr) if let hir::ExprKind::Path(_) = expr.kind => {} hir::StmtKind::Local(local) if let Some(expr) = local.init && let hir::ExprKind::Path(_) = expr.kind => {}, _ => return true @@ -99,7 +115,7 @@ fn modify_sdap_if_sig_drop_exists( expr: &'tcx hir::Expr<'_>, idx: usize, sdap: &mut SigDropAuxParams, - stmt: &'tcx hir::Stmt<'_>, + stmt: &hir::Stmt<'_>, cb: impl Fn(&mut SigDropAuxParams), ) { let mut sig_drop_finder = SigDropFinder::new(cx, &mut self.seen_types); @@ -117,7 +133,7 @@ fn modify_sdap_if_sig_drop_exists( } } - /// Shows a generic overall message as well as specialized messages depending on the usage. + /// Shows generic overall messages as well as specialized messages depending on the usage. fn set_suggestions(cx: &LateContext<'tcx>, block_span: Span, diag: &mut Diagnostic, sdap: &SigDropAuxParams) { match sdap.number_of_stmts { 0 | 1 => {}, @@ -172,8 +188,13 @@ fn set_suggestions(cx: &LateContext<'tcx>, block_span: Span, diag: &mut Diagnost impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> { fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) { + let dummy_ret_stmt = block.expr.map(|expr| hir::Stmt { + hir_id: hir::HirId::INVALID, + kind: hir::StmtKind::Expr(expr), + span: DUMMY_SP, + }); let mut sdap = SigDropAuxParams::default(); - for (idx, stmt) in block.stmts.iter().enumerate() { + for (idx, stmt) in Self::all_block_stmts(block.stmts, dummy_ret_stmt.as_ref()).enumerate() { match stmt.kind { hir::StmtKind::Expr(expr) => self.modify_sdap_if_sig_drop_exists( cx, @@ -213,11 +234,9 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) { _ => {} }; } - let stmts_after_last_use = sdap - .last_use_stmt_idx - .checked_add(1) - .and_then(|idx| block.stmts.get(idx..)) - .unwrap_or_default(); + + let idx = sdap.last_use_stmt_idx.wrapping_add(1); + let stmts_after_last_use = Self::all_block_stmts(block.stmts, dummy_ret_stmt.as_ref()).skip(idx); if sdap.number_of_stmts > 1 && Self::at_least_one_stmt_is_expensive(stmts_after_last_use) { span_lint_and_then( cx, diff --git a/tests/ui/significant_drop_tightening.fixed b/tests/ui/significant_drop_tightening.fixed index 5d176d2a927..da998c610bd 100644 --- a/tests/ui/significant_drop_tightening.fixed +++ b/tests/ui/significant_drop_tightening.fixed @@ -4,6 +4,26 @@ use std::sync::Mutex; +pub fn complex_return_triggers_the_lint() -> i32 { + fn foo() -> i32 { + 1 + } + let mutex = Mutex::new(1); + let lock = mutex.lock().unwrap(); + let _ = *lock; + let _ = *lock; + drop(lock); + foo() +} + +pub fn path_return_can_be_ignored() -> i32 { + let mutex = Mutex::new(1); + let lock = mutex.lock().unwrap(); + let rslt = *lock; + let _ = *lock; + rslt +} + pub fn post_bindings_can_be_ignored() { 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 index be9c0774275..83823f95f68 100644 --- a/tests/ui/significant_drop_tightening.rs +++ b/tests/ui/significant_drop_tightening.rs @@ -4,6 +4,25 @@ use std::sync::Mutex; +pub fn complex_return_triggers_the_lint() -> i32 { + fn foo() -> i32 { + 1 + } + let mutex = Mutex::new(1); + let lock = mutex.lock().unwrap(); + let _ = *lock; + let _ = *lock; + foo() +} + +pub fn path_return_can_be_ignored() -> i32 { + let mutex = Mutex::new(1); + let lock = mutex.lock().unwrap(); + let rslt = *lock; + let _ = *lock; + rslt +} + pub fn post_bindings_can_be_ignored() { 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 index 0ec9b1acb59..ab8ce356ec7 100644 --- a/tests/ui/significant_drop_tightening.stderr +++ b/tests/ui/significant_drop_tightening.stderr @@ -1,5 +1,29 @@ error: temporary with significant `Drop` can be early dropped - --> $DIR/significant_drop_tightening.rs:25:13 + --> $DIR/significant_drop_tightening.rs:12:9 + | +LL | pub fn complex_return_triggers_the_lint() -> i32 { + | __________________________________________________- +LL | | fn foo() -> i32 { +LL | | 1 +LL | | } +LL | | let mutex = Mutex::new(1); +LL | | let lock = mutex.lock().unwrap(); + | | ^^^^ +... | +LL | | foo() +LL | | } + | |_- temporary `lock` is currently being dropped at the end of its contained scope + | + = note: this might lead to unnecessary resource contention + = note: `-D clippy::significant-drop-tightening` implied by `-D warnings` +help: drop the temporary after the end of its last usage + | +LL ~ let _ = *lock; +LL + drop(lock); + | + +error: temporary with significant `Drop` can be early dropped + --> $DIR/significant_drop_tightening.rs:44:13 | LL | / { LL | | let mutex = Mutex::new(1i32); @@ -12,7 +36,6 @@ LL | | } | |_____- temporary `lock` is currently being dropped at the end of its contained scope | = note: this might lead to unnecessary resource contention - = note: `-D clippy::significant-drop-tightening` implied by `-D warnings` help: drop the temporary after the end of its last usage | LL ~ let rslt1 = lock.is_positive(); @@ -20,7 +43,7 @@ LL + drop(lock); | error: temporary with significant `Drop` can be early dropped - --> $DIR/significant_drop_tightening.rs:46:13 + --> $DIR/significant_drop_tightening.rs:65:13 | LL | / { LL | | let mutex = Mutex::new(1i32); @@ -44,7 +67,7 @@ LL + | error: temporary with significant `Drop` can be early dropped - --> $DIR/significant_drop_tightening.rs:52:17 + --> $DIR/significant_drop_tightening.rs:71:17 | LL | / { LL | | let mutex = Mutex::new(vec![1i32]); @@ -67,5 +90,5 @@ LL - lock.clear(); LL + | -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors