From 3cfd1e52335517160923ae6ba9adcc14c2e7f9f9 Mon Sep 17 00:00:00 2001 From: Preston From Date: Mon, 27 Jun 2022 22:52:22 -0600 Subject: [PATCH] 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 98884ac7d..7efa81226 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 dd64f240c..185e5009b 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 f97b606c4..dbb41837e 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