From 2476100c857147c3402b1b7c05ed65a84ceb228e Mon Sep 17 00:00:00 2001 From: Preston From Date: Fri, 10 Jun 2022 01:14:28 -0600 Subject: [PATCH 1/4] Add details about significant drop in match scrutinees causing deadlocks Adds more details about how a significant drop in a match scrutinee can cause a deadlock and include link to documentation. Emits messages indicating temporaries with significant drops in arms of matches and message about possible deadlocks/unexpected behavior. changelog: Add more details to significant drop lint to explicitly show how temporaries in match scrutinees can cause deadlocks/unexpected behavior. --- clippy_lints/src/matches/mod.rs | 2 +- .../matches/significant_drop_in_scrutinee.rs | 160 +++++++--- tests/ui/significant_drop_in_scrutinee.rs | 16 + tests/ui/significant_drop_in_scrutinee.stderr | 277 +++++++++++++++--- 4 files changed, 363 insertions(+), 92 deletions(-) diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 3e765173fb9f..d43ad1b8c9c6 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -963,7 +963,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { return; } if matches!(source, MatchSource::Normal | MatchSource::ForLoopDesugar) { - significant_drop_in_scrutinee::check(cx, expr, ex, source); + significant_drop_in_scrutinee::check(cx, expr, ex, arms, source); } collapsible_match::check_match(cx, arms); diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs index dcaf6f865de3..59ca33cbcc70 100644 --- a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs +++ b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs @@ -4,7 +4,7 @@ use clippy_utils::get_attr; use clippy_utils::source::{indent_of, snippet}; use rustc_errors::{Applicability, Diagnostic}; use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{Expr, ExprKind, MatchSource}; +use rustc_hir::{Arm, Expr, ExprKind, MatchSource}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::{Ty, TypeAndMut}; @@ -16,12 +16,21 @@ pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, scrutinee: &'tcx Expr<'_>, + arms: &'tcx [Arm<'_>], source: MatchSource, ) { if let Some((suggestions, message)) = has_significant_drop_in_scrutinee(cx, scrutinee, source) { for found in suggestions { span_lint_and_then(cx, SIGNIFICANT_DROP_IN_SCRUTINEE, found.found_span, message, |diag| { set_diagnostic(diag, cx, expr, found); + let s = Span::new(expr.span.hi(), expr.span.hi(), expr.span.ctxt(), None); + diag.span_label(s, "original temporary lives until here"); + if let Some(spans) = has_significant_drop_in_arms(cx, arms) { + for span in spans { + diag.span_label(span, "significant drop in arm here"); + } + diag.note("this might lead to deadlocks or other unexpected behavior"); + } }); } } @@ -80,22 +89,77 @@ fn has_significant_drop_in_scrutinee<'tcx, 'a>( let mut helper = SigDropHelper::new(cx); helper.find_sig_drop(scrutinee).map(|drops| { let message = if source == MatchSource::Normal { - "temporary with significant drop in match scrutinee" + "temporary with drop impl with side effects in match scrutinee lives to end of block" } else { - "temporary with significant drop in for loop" + "temporary with drop impl with side effects in for loop condition lives to end of block" }; (drops, message) }) } +struct SigDropChecker<'a, 'tcx> { + seen_types: FxHashSet>, + cx: &'a LateContext<'tcx>, +} + +impl<'a, 'tcx> SigDropChecker<'a, 'tcx> { + fn new(cx: &'a LateContext<'tcx>) -> SigDropChecker<'a, 'tcx> { + SigDropChecker { + seen_types: FxHashSet::default(), + cx, + } + } + + fn get_type(&self, ex: &'tcx Expr<'_>) -> Ty<'tcx> { + self.cx.typeck_results().expr_ty(ex) + } + + fn has_seen_type(&mut self, ty: Ty<'tcx>) -> bool { + !self.seen_types.insert(ty) + } + + fn has_sig_drop_attr(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + if let Some(adt) = ty.ty_adt_def() { + if get_attr(cx.sess(), cx.tcx.get_attrs_unchecked(adt.did()), "has_significant_drop").count() > 0 { + return true; + } + } + + match ty.kind() { + rustc_middle::ty::Adt(a, b) => { + for f in a.all_fields() { + let ty = f.ty(cx.tcx, b); + if !self.has_seen_type(ty) && self.has_sig_drop_attr(cx, ty) { + return true; + } + } + + for generic_arg in b.iter() { + if let GenericArgKind::Type(ty) = generic_arg.unpack() { + if self.has_sig_drop_attr(cx, ty) { + return true; + } + } + } + false + }, + rustc_middle::ty::Array(ty, _) + | rustc_middle::ty::RawPtr(TypeAndMut { ty, .. }) + | rustc_middle::ty::Ref(_, ty, _) + | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr(cx, *ty), + _ => false, + } + } +} + struct SigDropHelper<'a, 'tcx> { cx: &'a LateContext<'tcx>, is_chain_end: bool, - seen_types: FxHashSet>, has_significant_drop: bool, current_sig_drop: Option, sig_drop_spans: Option>, special_handling_for_binary_op: bool, + sig_drop_checker: SigDropChecker<'a, 'tcx>, } #[expect(clippy::enum_variant_names)] @@ -118,11 +182,11 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> { SigDropHelper { cx, is_chain_end: true, - seen_types: FxHashSet::default(), has_significant_drop: false, current_sig_drop: None, sig_drop_spans: None, special_handling_for_binary_op: false, + sig_drop_checker: SigDropChecker::new(cx), } } @@ -163,7 +227,7 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> { if self.current_sig_drop.is_some() { return; } - let ty = self.get_type(expr); + let ty = self.sig_drop_checker.get_type(expr); if ty.is_ref() { // We checked that the type was ref, so builtin_deref will return Some TypeAndMut, // but let's avoid any chance of an ICE @@ -187,14 +251,6 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> { } } - fn get_type(&self, ex: &'tcx Expr<'_>) -> Ty<'tcx> { - self.cx.typeck_results().expr_ty(ex) - } - - fn has_seen_type(&mut self, ty: Ty<'tcx>) -> bool { - !self.seen_types.insert(ty) - } - fn visit_exprs_for_binary_ops( &mut self, left: &'tcx Expr<'_>, @@ -214,44 +270,15 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> { self.special_handling_for_binary_op = false; } - - fn has_sig_drop_attr(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { - if let Some(adt) = ty.ty_adt_def() { - if get_attr(cx.sess(), cx.tcx.get_attrs_unchecked(adt.did()), "has_significant_drop").count() > 0 { - return true; - } - } - - match ty.kind() { - rustc_middle::ty::Adt(a, b) => { - for f in a.all_fields() { - let ty = f.ty(cx.tcx, b); - if !self.has_seen_type(ty) && self.has_sig_drop_attr(cx, ty) { - return true; - } - } - - for generic_arg in b.iter() { - if let GenericArgKind::Type(ty) = generic_arg.unpack() { - if self.has_sig_drop_attr(cx, ty) { - return true; - } - } - } - false - }, - rustc_middle::ty::Array(ty, _) - | rustc_middle::ty::RawPtr(TypeAndMut { ty, .. }) - | rustc_middle::ty::Ref(_, ty, _) - | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr(cx, *ty), - _ => false, - } - } } impl<'a, 'tcx> Visitor<'tcx> for SigDropHelper<'a, 'tcx> { fn visit_expr(&mut self, ex: &'tcx Expr<'_>) { - if !self.is_chain_end && self.has_sig_drop_attr(self.cx, self.get_type(ex)) { + if !self.is_chain_end + && self + .sig_drop_checker + .has_sig_drop_attr(self.cx, self.sig_drop_checker.get_type(ex)) + { self.has_significant_drop = true; return; } @@ -330,3 +357,40 @@ impl<'a, 'tcx> Visitor<'tcx> for SigDropHelper<'a, 'tcx> { } } } + +struct ArmSigDropHelper<'a, 'tcx> { + sig_drop_checker: SigDropChecker<'a, 'tcx>, + found_sig_drop_spans: Option>, +} + +impl<'a, 'tcx> ArmSigDropHelper<'a, 'tcx> { + fn new(cx: &'a LateContext<'tcx>) -> ArmSigDropHelper<'a, 'tcx> { + ArmSigDropHelper { + sig_drop_checker: SigDropChecker::new(cx), + found_sig_drop_spans: None, + } + } +} + +fn has_significant_drop_in_arms<'tcx, 'a>(cx: &'a LateContext<'tcx>, arms: &'tcx [Arm<'_>]) -> Option> { + let mut helper = ArmSigDropHelper::new(cx); + for arm in arms { + helper.visit_expr(arm.body); + } + helper.found_sig_drop_spans +} + +impl<'a, 'tcx> Visitor<'tcx> for ArmSigDropHelper<'a, 'tcx> { + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { + if self + .sig_drop_checker + .has_sig_drop_attr(self.sig_drop_checker.cx, self.sig_drop_checker.get_type(ex)) + { + self.found_sig_drop_spans + .get_or_insert_with(FxHashSet::default) + .insert(ex.span); + return; + } + walk_expr(self, ex); + } +} diff --git a/tests/ui/significant_drop_in_scrutinee.rs b/tests/ui/significant_drop_in_scrutinee.rs index 4347610f393f..dd64f240c4ac 100644 --- a/tests/ui/significant_drop_in_scrutinee.rs +++ b/tests/ui/significant_drop_in_scrutinee.rs @@ -591,4 +591,20 @@ fn should_trigger_lint_for_read_write_lock_for_loop() { } } +fn do_bar(mutex: &Mutex) { + mutex.lock().unwrap().bar(); +} + +fn should_trigger_lint_without_significant_drop_in_arm() { + let mutex = Mutex::new(State {}); + + // Should trigger lint because the lifetime of the temporary MutexGuard is surprising because it + // is preserved until the end of the match, but there is no clear indication that this is the + // case. + match mutex.lock().unwrap().foo() { + true => do_bar(&mutex), + false => {}, + }; +} + fn main() {} diff --git a/tests/ui/significant_drop_in_scrutinee.stderr b/tests/ui/significant_drop_in_scrutinee.stderr index 303f3c1df033..472f998ebf26 100644 --- a/tests/ui/significant_drop_in_scrutinee.stderr +++ b/tests/ui/significant_drop_in_scrutinee.stderr @@ -1,45 +1,69 @@ -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:59:11 | LL | match mutex.lock().unwrap().foo() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | true => { +LL | mutex.lock().unwrap().bar(); + | --------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here | = note: `-D clippy::significant-drop-in-scrutinee` implied by `-D warnings` + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex.lock().unwrap().foo(); LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:132:11 | LL | match s.lock_m().get_the_value() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +... +LL | println!("{}", s.lock_m().get_the_value()); + | ---------- significant drop in arm here +... +LL | } + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = s.lock_m().get_the_value(); LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:153:11 | LL | match s.lock_m_m().get_the_value() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +... +LL | println!("{}", s.lock_m().get_the_value()); + | ---------- significant drop in arm here +... +LL | } + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = s.lock_m_m().get_the_value(); LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:201:11 | LL | match counter.temp_increment().len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | }; + | - original temporary lives until here | help: try moving the temporary above the match | @@ -47,127 +71,218 @@ LL ~ let value = counter.temp_increment().len(); LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:224:16 | LL | match (mutex1.lock().unwrap().s.len(), true) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +... +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex1.lock().unwrap().s.len(); LL ~ match (value, true) { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:233:22 | LL | match (true, mutex1.lock().unwrap().s.len(), true) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +... +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex1.lock().unwrap().s.len(); LL ~ match (true, value, true) { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:243:16 | LL | match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +... +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +LL | mutex2.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex1.lock().unwrap().s.len(); LL ~ match (value, true, mutex2.lock().unwrap().s.len()) { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:243:54 | LL | match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +... +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +LL | mutex2.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex2.lock().unwrap().s.len(); LL ~ match (mutex1.lock().unwrap().s.len(), true, value) { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:254:15 | LL | match mutex3.lock().unwrap().s.as_str() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +LL | mutex2.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:264:22 | LL | match (true, mutex3.lock().unwrap().s.as_str()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +LL | mutex2.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:283:11 | LL | match mutex.lock().unwrap().s.len() > 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +LL | true => { +LL | mutex.lock().unwrap().s.len(); + | --------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex.lock().unwrap().s.len() > 1; LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:290:11 | LL | match 1 < mutex.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +LL | true => { +LL | mutex.lock().unwrap().s.len(); + | --------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = 1 < mutex.lock().unwrap().s.len(); LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:308:11 | LL | match mutex1.lock().unwrap().s.len() < mutex2.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +... +LL | mutex1.lock().unwrap().s.len(), + | ---------------------- significant drop in arm here +LL | mutex2.lock().unwrap().s.len() + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex1.lock().unwrap().s.len() < mutex2.lock().unwrap().s.len(); LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:319:11 | LL | match mutex1.lock().unwrap().s.len() >= mutex2.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +... +LL | mutex1.lock().unwrap().s.len(), + | ---------------------- significant drop in arm here +LL | mutex2.lock().unwrap().s.len() + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex1.lock().unwrap().s.len() >= mutex2.lock().unwrap().s.len(); LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:354:11 | LL | match get_mutex_guard().s.len() > 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +LL | true => { +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = get_mutex_guard().s.len() > 1; LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:371:11 | LL | match match i { @@ -179,7 +294,14 @@ LL | | .s LL | | .len() LL | | > 1 | |___________^ - | +... +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = match i { @@ -190,7 +312,7 @@ LL + .s LL + .len() ... -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:397:11 | LL | match if i > 1 { @@ -202,7 +324,14 @@ LL | | mutex2.lock().unwrap() LL | | .len() LL | | > 1 | |___________^ - | +... +LL | mutex1.lock().unwrap().s.len(); + | ---------------------- significant drop in arm here +... +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = if i > 1 { @@ -213,83 +342,145 @@ LL + } LL + .s ... -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:451:11 | LL | match s.lock().deref().deref() { | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | 0 | 1 => println!("Value was less than 2"), +LL | _ => println!("Value is {}", s.lock().deref()), + | ---------------- significant drop in arm here +LL | }; + | - original temporary lives until here | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match and create a copy | LL ~ let value = *s.lock().deref().deref(); LL ~ match value { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:479:11 | LL | match s.lock().deref().deref() { | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | matcher => println!("Value is {}", s.lock().deref()), + | ---------------- significant drop in arm here +LL | _ => println!("Value was not a match"), +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:498:11 | LL | match mutex.lock().unwrap().i = i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +LL | _ => { +LL | println!("{}", mutex.lock().unwrap().i); + | --------------------- significant drop in arm here +LL | }, +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ mutex.lock().unwrap().i = i; LL ~ match () { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:504:11 | LL | match i = mutex.lock().unwrap().i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +LL | _ => { +LL | println!("{}", mutex.lock().unwrap().i); + | --------------------- significant drop in arm here +LL | }, +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ i = mutex.lock().unwrap().i; LL ~ match () { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:510:11 | LL | match mutex.lock().unwrap().i += 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +LL | _ => { +LL | println!("{}", mutex.lock().unwrap().i); + | --------------------- significant drop in arm here +LL | }, +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ mutex.lock().unwrap().i += 1; LL ~ match () { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:516:11 | LL | match i += mutex.lock().unwrap().i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | +LL | _ => { +LL | println!("{}", mutex.lock().unwrap().i); + | --------------------- significant drop in arm here +LL | }, +LL | }; + | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ i += mutex.lock().unwrap().i; LL ~ match () { | -error: temporary with significant drop in match scrutinee +error: temporary with drop impl with side effects in match scrutinee lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:579:11 | LL | match rwlock.read().unwrap().to_number() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | }; + | - original temporary lives until here -error: temporary with significant drop in for loop +error: temporary with drop impl with side effects in for loop condition lives to end of block --> $DIR/significant_drop_in_scrutinee.rs:589:14 | LL | for s in rwlock.read().unwrap().iter() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | println!("{}", s); +LL | } + | - original temporary lives until here + +error: temporary with drop impl with side effects in match scrutinee lives to end of block + --> $DIR/significant_drop_in_scrutinee.rs:604:11 + | +LL | match mutex.lock().unwrap().foo() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | }; + | - original temporary lives until here + | +help: try moving the temporary above the match + | +LL ~ let value = mutex.lock().unwrap().foo(); +LL ~ match value { + | -error: aborting due to 25 previous errors +error: aborting due to 26 previous errors From 1f707db0bf928454b699af1a2da745dfdbdeb680 Mon Sep 17 00:00:00 2001 From: Preston From Date: Wed, 22 Jun 2022 22:34:49 -0600 Subject: [PATCH 2/4] Update messages for clarity when linting --- .../matches/significant_drop_in_scrutinee.rs | 8 +- tests/ui/significant_drop_in_scrutinee.stderr | 114 +++++++++--------- 2 files changed, 64 insertions(+), 58 deletions(-) diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs index 59ca33cbcc70..98884ac7d737 100644 --- a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs +++ b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs @@ -27,10 +27,10 @@ pub(super) fn check<'tcx>( diag.span_label(s, "original temporary lives until here"); if let Some(spans) = has_significant_drop_in_arms(cx, arms) { for span in spans { - diag.span_label(span, "significant drop in arm here"); + diag.span_label(span, "another temporary with significant `Drop` created here"); } - diag.note("this might lead to deadlocks or other unexpected behavior"); } + diag.note("this might lead to deadlocks or other unexpected behavior"); }); } } @@ -89,9 +89,9 @@ fn has_significant_drop_in_scrutinee<'tcx, 'a>( let mut helper = SigDropHelper::new(cx); helper.find_sig_drop(scrutinee).map(|drops| { let message = if source == MatchSource::Normal { - "temporary with drop impl with side effects in match scrutinee lives to end of block" + "temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression" } else { - "temporary with drop impl with side effects in for loop condition lives to end of block" + "temporary with significant `Drop` in `for` loop condition will live until the end of the `for` expression" }; (drops, message) }) diff --git a/tests/ui/significant_drop_in_scrutinee.stderr b/tests/ui/significant_drop_in_scrutinee.stderr index 472f998ebf26..f97b606c4747 100644 --- a/tests/ui/significant_drop_in_scrutinee.stderr +++ b/tests/ui/significant_drop_in_scrutinee.stderr @@ -1,11 +1,11 @@ -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:59:11 | LL | match mutex.lock().unwrap().foo() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | true => { LL | mutex.lock().unwrap().bar(); - | --------------------- significant drop in arm here + | --------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -18,14 +18,14 @@ LL ~ let value = mutex.lock().unwrap().foo(); LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:132:11 | LL | match s.lock_m().get_the_value() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | println!("{}", s.lock_m().get_the_value()); - | ---------- significant drop in arm here + | ---------- another temporary with significant `Drop` created here ... LL | } | - original temporary lives until here @@ -37,14 +37,14 @@ LL ~ let value = s.lock_m().get_the_value(); LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:153:11 | LL | match s.lock_m_m().get_the_value() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | println!("{}", s.lock_m().get_the_value()); - | ---------- significant drop in arm here + | ---------- another temporary with significant `Drop` created here ... LL | } | - original temporary lives until here @@ -56,7 +56,7 @@ LL ~ let value = s.lock_m_m().get_the_value(); LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:201:11 | LL | match counter.temp_increment().len() { @@ -65,20 +65,21 @@ LL | match counter.temp_increment().len() { LL | }; | - original temporary lives until here | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = counter.temp_increment().len(); LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:224:16 | LL | match (mutex1.lock().unwrap().s.len(), true) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -90,14 +91,14 @@ LL ~ let value = mutex1.lock().unwrap().s.len(); LL ~ match (value, true) { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:233:22 | LL | match (true, mutex1.lock().unwrap().s.len(), true) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -109,16 +110,16 @@ LL ~ let value = mutex1.lock().unwrap().s.len(); LL ~ match (true, value, true) { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:243:16 | LL | match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here LL | mutex2.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -130,16 +131,16 @@ LL ~ let value = mutex1.lock().unwrap().s.len(); LL ~ match (value, true, mutex2.lock().unwrap().s.len()) { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:243:54 | LL | match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here LL | mutex2.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -151,46 +152,46 @@ LL ~ let value = mutex2.lock().unwrap().s.len(); LL ~ match (mutex1.lock().unwrap().s.len(), true, value) { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:254:15 | LL | match mutex3.lock().unwrap().s.as_str() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here LL | mutex2.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:264:22 | LL | match (true, mutex3.lock().unwrap().s.as_str()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here LL | mutex2.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:283:11 | LL | match mutex.lock().unwrap().s.len() > 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | true => { LL | mutex.lock().unwrap().s.len(); - | --------------------- significant drop in arm here + | --------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -202,14 +203,14 @@ LL ~ let value = mutex.lock().unwrap().s.len() > 1; LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:290:11 | LL | match 1 < mutex.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | true => { LL | mutex.lock().unwrap().s.len(); - | --------------------- significant drop in arm here + | --------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -221,16 +222,16 @@ LL ~ let value = 1 < mutex.lock().unwrap().s.len(); LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:308:11 | LL | match mutex1.lock().unwrap().s.len() < mutex2.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(), - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here LL | mutex2.lock().unwrap().s.len() - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -242,16 +243,16 @@ LL ~ let value = mutex1.lock().unwrap().s.len() < mutex2.lock().unwrap().s.l LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:319:11 | LL | match mutex1.lock().unwrap().s.len() >= mutex2.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(), - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here LL | mutex2.lock().unwrap().s.len() - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -263,14 +264,14 @@ LL ~ let value = mutex1.lock().unwrap().s.len() >= mutex2.lock().unwrap().s. LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:354:11 | LL | match get_mutex_guard().s.len() > 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | true => { LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -282,7 +283,7 @@ LL ~ let value = get_mutex_guard().s.len() > 1; LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:371:11 | LL | match match i { @@ -296,7 +297,7 @@ LL | | > 1 | |___________^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -312,7 +313,7 @@ LL + .s LL + .len() ... -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:397:11 | LL | match if i > 1 { @@ -326,7 +327,7 @@ LL | | > 1 | |___________^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- significant drop in arm here + | ---------------------- another temporary with significant `Drop` created here ... LL | }; | - original temporary lives until here @@ -342,14 +343,14 @@ LL + } LL + .s ... -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:451:11 | LL | match s.lock().deref().deref() { | ^^^^^^^^^^^^^^^^^^^^^^^^ LL | 0 | 1 => println!("Value was less than 2"), LL | _ => println!("Value is {}", s.lock().deref()), - | ---------------- significant drop in arm here + | ---------------- another temporary with significant `Drop` created here LL | }; | - original temporary lives until here | @@ -360,27 +361,27 @@ LL ~ let value = *s.lock().deref().deref(); LL ~ match value { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:479:11 | LL | match s.lock().deref().deref() { | ^^^^^^^^^^^^^^^^^^^^^^^^ LL | matcher => println!("Value is {}", s.lock().deref()), - | ---------------- significant drop in arm here + | ---------------- another temporary with significant `Drop` created here LL | _ => println!("Value was not a match"), LL | }; | - original temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:498:11 | LL | match mutex.lock().unwrap().i = i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | _ => { LL | println!("{}", mutex.lock().unwrap().i); - | --------------------- significant drop in arm here + | --------------------- another temporary with significant `Drop` created here LL | }, LL | }; | - original temporary lives until here @@ -392,14 +393,14 @@ LL ~ mutex.lock().unwrap().i = i; LL ~ match () { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:504:11 | LL | match i = mutex.lock().unwrap().i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | _ => { LL | println!("{}", mutex.lock().unwrap().i); - | --------------------- significant drop in arm here + | --------------------- another temporary with significant `Drop` created here LL | }, LL | }; | - original temporary lives until here @@ -411,14 +412,14 @@ LL ~ i = mutex.lock().unwrap().i; LL ~ match () { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:510:11 | LL | match mutex.lock().unwrap().i += 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | _ => { LL | println!("{}", mutex.lock().unwrap().i); - | --------------------- significant drop in arm here + | --------------------- another temporary with significant `Drop` created here LL | }, LL | }; | - original temporary lives until here @@ -430,14 +431,14 @@ LL ~ mutex.lock().unwrap().i += 1; LL ~ match () { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:516:11 | LL | match i += mutex.lock().unwrap().i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | _ => { LL | println!("{}", mutex.lock().unwrap().i); - | --------------------- significant drop in arm here + | --------------------- another temporary with significant `Drop` created here LL | }, LL | }; | - original temporary lives until here @@ -449,7 +450,7 @@ LL ~ i += mutex.lock().unwrap().i; LL ~ match () { | -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:579:11 | LL | match rwlock.read().unwrap().to_number() { @@ -457,8 +458,10 @@ LL | match rwlock.read().unwrap().to_number() { ... LL | }; | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior -error: temporary with drop impl with side effects in for loop condition lives to end of block +error: temporary with significant `Drop` in `for` loop condition will live until the end of the `for` expression --> $DIR/significant_drop_in_scrutinee.rs:589:14 | LL | for s in rwlock.read().unwrap().iter() { @@ -466,8 +469,10 @@ LL | for s in rwlock.read().unwrap().iter() { LL | println!("{}", s); LL | } | - original temporary lives until here + | + = note: this might lead to deadlocks or other unexpected behavior -error: temporary with drop impl with side effects in match scrutinee lives to end of block +error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression --> $DIR/significant_drop_in_scrutinee.rs:604:11 | LL | match mutex.lock().unwrap().foo() { @@ -476,6 +481,7 @@ LL | match mutex.lock().unwrap().foo() { LL | }; | - original temporary lives until here | + = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match | LL ~ let value = mutex.lock().unwrap().foo(); From 3cfd1e52335517160923ae6ba9adcc14c2e7f9f9 Mon Sep 17 00:00:00 2001 From: Preston From Date: Mon, 27 Jun 2022 22:52:22 -0600 Subject: [PATCH 3/4] Make messages more accurate, check lint enabled --- .../matches/significant_drop_in_scrutinee.rs | 24 +-- tests/ui/significant_drop_in_scrutinee.rs | 13 ++ tests/ui/significant_drop_in_scrutinee.stderr | 158 +++++++++--------- 3 files changed, 104 insertions(+), 91 deletions(-) diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs index 98884ac7d737..7efa81226e0e 100644 --- a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs +++ b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs @@ -1,6 +1,6 @@ use crate::FxHashSet; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::get_attr; +use clippy_utils::{get_attr, is_lint_allowed}; use clippy_utils::source::{indent_of, snippet}; use rustc_errors::{Applicability, Diagnostic}; use rustc_hir::intravisit::{walk_expr, Visitor}; @@ -19,16 +19,18 @@ pub(super) fn check<'tcx>( arms: &'tcx [Arm<'_>], source: MatchSource, ) { + if is_lint_allowed(cx, SIGNIFICANT_DROP_IN_SCRUTINEE, expr.hir_id) { + return; + } + if let Some((suggestions, message)) = has_significant_drop_in_scrutinee(cx, scrutinee, source) { for found in suggestions { span_lint_and_then(cx, SIGNIFICANT_DROP_IN_SCRUTINEE, found.found_span, message, |diag| { set_diagnostic(diag, cx, expr, found); let s = Span::new(expr.span.hi(), expr.span.hi(), expr.span.ctxt(), None); - diag.span_label(s, "original temporary lives until here"); - if let Some(spans) = has_significant_drop_in_arms(cx, arms) { - for span in spans { - diag.span_label(span, "another temporary with significant `Drop` created here"); - } + diag.span_label(s, "temporary lives until here"); + for span in has_significant_drop_in_arms(cx, arms) { + diag.span_label(span, "another value with significant `Drop` created here"); } diag.note("this might lead to deadlocks or other unexpected behavior"); }); @@ -360,19 +362,19 @@ impl<'a, 'tcx> Visitor<'tcx> for SigDropHelper<'a, 'tcx> { struct ArmSigDropHelper<'a, 'tcx> { sig_drop_checker: SigDropChecker<'a, 'tcx>, - found_sig_drop_spans: Option>, + found_sig_drop_spans: FxHashSet, } impl<'a, 'tcx> ArmSigDropHelper<'a, 'tcx> { fn new(cx: &'a LateContext<'tcx>) -> ArmSigDropHelper<'a, 'tcx> { ArmSigDropHelper { sig_drop_checker: SigDropChecker::new(cx), - found_sig_drop_spans: None, + found_sig_drop_spans: FxHashSet::::default(), } } } -fn has_significant_drop_in_arms<'tcx, 'a>(cx: &'a LateContext<'tcx>, arms: &'tcx [Arm<'_>]) -> Option> { +fn has_significant_drop_in_arms<'tcx, 'a>(cx: &'a LateContext<'tcx>, arms: &'tcx [Arm<'_>]) -> FxHashSet { let mut helper = ArmSigDropHelper::new(cx); for arm in arms { helper.visit_expr(arm.body); @@ -386,9 +388,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ArmSigDropHelper<'a, 'tcx> { .sig_drop_checker .has_sig_drop_attr(self.sig_drop_checker.cx, self.sig_drop_checker.get_type(ex)) { - self.found_sig_drop_spans - .get_or_insert_with(FxHashSet::default) - .insert(ex.span); + self.found_sig_drop_spans.insert(ex.span); return; } walk_expr(self, ex); diff --git a/tests/ui/significant_drop_in_scrutinee.rs b/tests/ui/significant_drop_in_scrutinee.rs index dd64f240c4ac..185e5009b60f 100644 --- a/tests/ui/significant_drop_in_scrutinee.rs +++ b/tests/ui/significant_drop_in_scrutinee.rs @@ -64,6 +64,19 @@ fn should_trigger_lint_with_mutex_guard_in_match_scrutinee() { }; } +fn should_not_trigger_lint_with_mutex_guard_in_match_scrutinee_when_lint_allowed() { + let mutex = Mutex::new(State {}); + + // Lint should not be triggered because it is "allowed" below. + #[allow(clippy::significant_drop_in_scrutinee)] + match mutex.lock().unwrap().foo() { + true => { + mutex.lock().unwrap().bar(); + }, + false => {}, + }; +} + fn should_not_trigger_lint_for_insignificant_drop() { // Should not trigger lint because there are no temporaries whose drops have a significant // side effect. diff --git a/tests/ui/significant_drop_in_scrutinee.stderr b/tests/ui/significant_drop_in_scrutinee.stderr index f97b606c4747..dbb41837e790 100644 --- a/tests/ui/significant_drop_in_scrutinee.stderr +++ b/tests/ui/significant_drop_in_scrutinee.stderr @@ -5,10 +5,10 @@ LL | match mutex.lock().unwrap().foo() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | true => { LL | mutex.lock().unwrap().bar(); - | --------------------- another temporary with significant `Drop` created here + | --------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: `-D clippy::significant-drop-in-scrutinee` implied by `-D warnings` = note: this might lead to deadlocks or other unexpected behavior @@ -19,16 +19,16 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:132:11 + --> $DIR/significant_drop_in_scrutinee.rs:145:11 | LL | match s.lock_m().get_the_value() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | println!("{}", s.lock_m().get_the_value()); - | ---------- another temporary with significant `Drop` created here + | ---------- another value with significant `Drop` created here ... LL | } - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -38,16 +38,16 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:153:11 + --> $DIR/significant_drop_in_scrutinee.rs:166:11 | LL | match s.lock_m_m().get_the_value() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | println!("{}", s.lock_m().get_the_value()); - | ---------- another temporary with significant `Drop` created here + | ---------- another value with significant `Drop` created here ... LL | } - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -57,13 +57,13 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:201:11 + --> $DIR/significant_drop_in_scrutinee.rs:214:11 | LL | match counter.temp_increment().len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -73,16 +73,16 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:224:16 + --> $DIR/significant_drop_in_scrutinee.rs:237:16 | LL | match (mutex1.lock().unwrap().s.len(), true) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -92,16 +92,16 @@ LL ~ match (value, true) { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:233:22 + --> $DIR/significant_drop_in_scrutinee.rs:246:22 | LL | match (true, mutex1.lock().unwrap().s.len(), true) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -111,18 +111,18 @@ LL ~ match (true, value, true) { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:243:16 + --> $DIR/significant_drop_in_scrutinee.rs:256:16 | LL | match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here LL | mutex2.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -132,18 +132,18 @@ LL ~ match (value, true, mutex2.lock().unwrap().s.len()) { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:243:54 + --> $DIR/significant_drop_in_scrutinee.rs:256:54 | LL | match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here LL | mutex2.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -153,48 +153,48 @@ LL ~ match (mutex1.lock().unwrap().s.len(), true, value) { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:254:15 + --> $DIR/significant_drop_in_scrutinee.rs:267:15 | LL | match mutex3.lock().unwrap().s.as_str() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here LL | mutex2.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:264:22 + --> $DIR/significant_drop_in_scrutinee.rs:277:22 | LL | match (true, mutex3.lock().unwrap().s.as_str()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here LL | mutex2.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:283:11 + --> $DIR/significant_drop_in_scrutinee.rs:296:11 | LL | match mutex.lock().unwrap().s.len() > 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | true => { LL | mutex.lock().unwrap().s.len(); - | --------------------- another temporary with significant `Drop` created here + | --------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -204,16 +204,16 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:290:11 + --> $DIR/significant_drop_in_scrutinee.rs:303:11 | LL | match 1 < mutex.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | true => { LL | mutex.lock().unwrap().s.len(); - | --------------------- another temporary with significant `Drop` created here + | --------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -223,18 +223,18 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:308:11 + --> $DIR/significant_drop_in_scrutinee.rs:321:11 | LL | match mutex1.lock().unwrap().s.len() < mutex2.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(), - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here LL | mutex2.lock().unwrap().s.len() - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -244,18 +244,18 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:319:11 + --> $DIR/significant_drop_in_scrutinee.rs:332:11 | LL | match mutex1.lock().unwrap().s.len() >= mutex2.lock().unwrap().s.len() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | mutex1.lock().unwrap().s.len(), - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here LL | mutex2.lock().unwrap().s.len() - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -265,16 +265,16 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:354:11 + --> $DIR/significant_drop_in_scrutinee.rs:367:11 | LL | match get_mutex_guard().s.len() > 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | true => { LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -284,7 +284,7 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:371:11 + --> $DIR/significant_drop_in_scrutinee.rs:384:11 | LL | match match i { | ___________^ @@ -297,10 +297,10 @@ LL | | > 1 | |___________^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -314,7 +314,7 @@ LL + .len() ... error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:397:11 + --> $DIR/significant_drop_in_scrutinee.rs:410:11 | LL | match if i > 1 { | ___________^ @@ -327,10 +327,10 @@ LL | | > 1 | |___________^ ... LL | mutex1.lock().unwrap().s.len(); - | ---------------------- another temporary with significant `Drop` created here + | ---------------------- another value with significant `Drop` created here ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -344,15 +344,15 @@ LL + .s ... error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:451:11 + --> $DIR/significant_drop_in_scrutinee.rs:464:11 | LL | match s.lock().deref().deref() { | ^^^^^^^^^^^^^^^^^^^^^^^^ LL | 0 | 1 => println!("Value was less than 2"), LL | _ => println!("Value is {}", s.lock().deref()), - | ---------------- another temporary with significant `Drop` created here + | ---------------- another value with significant `Drop` created here LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match and create a copy @@ -362,29 +362,29 @@ LL ~ match value { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:479:11 + --> $DIR/significant_drop_in_scrutinee.rs:492:11 | LL | match s.lock().deref().deref() { | ^^^^^^^^^^^^^^^^^^^^^^^^ LL | matcher => println!("Value is {}", s.lock().deref()), - | ---------------- another temporary with significant `Drop` created here + | ---------------- another value with significant `Drop` created here LL | _ => println!("Value was not a match"), LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:498:11 + --> $DIR/significant_drop_in_scrutinee.rs:511:11 | LL | match mutex.lock().unwrap().i = i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | _ => { LL | println!("{}", mutex.lock().unwrap().i); - | --------------------- another temporary with significant `Drop` created here + | --------------------- another value with significant `Drop` created here LL | }, LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -394,16 +394,16 @@ LL ~ match () { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:504:11 + --> $DIR/significant_drop_in_scrutinee.rs:517:11 | LL | match i = mutex.lock().unwrap().i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | _ => { LL | println!("{}", mutex.lock().unwrap().i); - | --------------------- another temporary with significant `Drop` created here + | --------------------- another value with significant `Drop` created here LL | }, LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -413,16 +413,16 @@ LL ~ match () { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:510:11 + --> $DIR/significant_drop_in_scrutinee.rs:523:11 | LL | match mutex.lock().unwrap().i += 1 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | _ => { LL | println!("{}", mutex.lock().unwrap().i); - | --------------------- another temporary with significant `Drop` created here + | --------------------- another value with significant `Drop` created here LL | }, LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -432,16 +432,16 @@ LL ~ match () { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:516:11 + --> $DIR/significant_drop_in_scrutinee.rs:529:11 | LL | match i += mutex.lock().unwrap().i { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | _ => { LL | println!("{}", mutex.lock().unwrap().i); - | --------------------- another temporary with significant `Drop` created here + | --------------------- another value with significant `Drop` created here LL | }, LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match @@ -451,35 +451,35 @@ LL ~ match () { | error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:579:11 + --> $DIR/significant_drop_in_scrutinee.rs:592:11 | LL | match rwlock.read().unwrap().to_number() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior error: temporary with significant `Drop` in `for` loop condition will live until the end of the `for` expression - --> $DIR/significant_drop_in_scrutinee.rs:589:14 + --> $DIR/significant_drop_in_scrutinee.rs:602:14 | LL | for s in rwlock.read().unwrap().iter() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LL | println!("{}", s); LL | } - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression - --> $DIR/significant_drop_in_scrutinee.rs:604:11 + --> $DIR/significant_drop_in_scrutinee.rs:617:11 | LL | match mutex.lock().unwrap().foo() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | }; - | - original temporary lives until here + | - temporary lives until here | = note: this might lead to deadlocks or other unexpected behavior help: try moving the temporary above the match From 7bc40967a44f08e359c760ca6587b0d7d62e57a7 Mon Sep 17 00:00:00 2001 From: Preston From Date: Mon, 27 Jun 2022 23:01:47 -0600 Subject: [PATCH 4/4] Run cargo dev fmt --- clippy_lints/src/matches/significant_drop_in_scrutinee.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs index 7efa81226e0e..0704a5af5259 100644 --- a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs +++ b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs @@ -1,7 +1,7 @@ use crate::FxHashSet; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::{get_attr, is_lint_allowed}; use clippy_utils::source::{indent_of, snippet}; +use clippy_utils::{get_attr, is_lint_allowed}; use rustc_errors::{Applicability, Diagnostic}; use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::{Arm, Expr, ExprKind, MatchSource};