Lint for significant drops who may have surprising lifetimes #1

author Preston From <prestonfrom@gmail.com> 1645164142 -0600
committer Preston From <prestonfrom@gmail.com> 1650005351 -0600

CHANGELOG.md

@@ -3696,6 +3696,7 @@ Released 2018-09-13
 [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
 [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
 [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
+[`significant_drop_in_scrutinee`]: https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_in_scrutinee
 [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
 [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add
 [`string_add_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add_assign

clippy_lints/src/lib.register_lints.rs

@@ -475,6 +475,7 @@
     size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT,
     slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,
     stable_sort_primitive::STABLE_SORT_PRIMITIVE,
+    significant_drop_in_scrutinee::SIGNIFICANT_DROP_IN_SCRUTINEE,
     strings::STRING_ADD,
     strings::STRING_ADD_ASSIGN,
     strings::STRING_FROM_UTF8_AS_BYTES,

clippy_lints/src/lib.register_nursery.rs

@@ -25,6 +25,7 @@
     LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
     LintId::of(redundant_pub_crate::REDUNDANT_PUB_CRATE),
     LintId::of(regex::TRIVIAL_REGEX),
+    LintId::of(significant_drop_in_scrutinee::SIGNIFICANT_DROP_IN_SCRUTINEE),
     LintId::of(strings::STRING_LIT_AS_BYTES),
     LintId::of(suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS),
     LintId::of(trailing_empty_array::TRAILING_EMPTY_ARRAY),

clippy_lints/src/lib.rs

@@ -364,6 +364,7 @@ macro_rules! declare_clippy_lint {
 mod semicolon_if_nothing_returned;
 mod serde_api;
 mod shadow;
+mod significant_drop_in_scrutinee;
 mod single_char_lifetime_names;
 mod single_component_path_imports;
 mod size_of_in_element_count;
@@ -874,6 +875,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv)));
     store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation));
     store.register_late_pass(|| Box::new(only_used_in_recursion::OnlyUsedInRecursion));
+    store.register_late_pass(|| Box::new(significant_drop_in_scrutinee::SignificantDropInScrutinee));
     store.register_late_pass(|| Box::new(dbg_macro::DbgMacro));
     let cargo_ignore_publish = conf.cargo_ignore_publish;
     store.register_late_pass(move || {

clippy_lints/src/significant_drop_in_scrutinee.rs

@@ -0,0 +1,408 @@
+use crate::FxHashSet;
+use clippy_utils::diagnostics::span_lint_and_then;
+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};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::ty::subst::GenericArgKind;
+use rustc_middle::ty::{Ty, TypeAndMut};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Check for temporaries returned from function calls in a match scrutinee that have the
+    /// `clippy::has_significant_drop` attribute.
+    ///
+    /// ### Why is this bad?
+    /// The `clippy::has_significant_drop` attribute can be added to types whose Drop impls have
+    /// an important side-effect, such as unlocking a mutex, making it important for users to be
+    /// able to accurately understand their lifetimes. When a temporary is returned in a function
+    /// call in a match scrutinee, its lifetime lasts until the end of the match block, which may
+    /// be surprising.
+    ///
+    /// For `Mutex`es this can lead to a deadlock. This happens when the match scrutinee uses a
+    /// function call that returns a `MutexGuard` and then tries to lock again in one of the match
+    /// arms. In that case the `MutexGuard` in the scrutinee will not be dropped until the end of
+    /// the match block and thus will not unlock.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # use std::sync::Mutex;
+    ///
+    /// # struct State {}
+    ///
+    /// # impl State {
+    /// #     fn foo(&self) -> bool {
+    /// #         true
+    /// #     }
+    ///
+    /// #     fn bar(&self) {}
+    /// # }
+    ///
+    ///
+    /// let mutex = Mutex::new(State {});
+    ///
+    /// match mutex.lock().unwrap().foo() {
+    ///     true => {
+    ///         mutex.lock().unwrap().bar(); // Deadlock!
+    ///     }
+    ///     false => {}
+    /// };
+    ///
+    /// println!("All done!");
+    ///
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # use std::sync::Mutex;
+    ///
+    /// # struct State {}
+    ///
+    /// # impl State {
+    /// #     fn foo(&self) -> bool {
+    /// #         true
+    /// #     }
+    ///
+    /// #     fn bar(&self) {}
+    /// # }
+    ///
+    /// let mutex = Mutex::new(State {});
+    ///
+    /// let is_foo = mutex.lock().unwrap().foo();
+    /// match is_foo {
+    ///     true => {
+    ///         mutex.lock().unwrap().bar();
+    ///     }
+    ///     false => {}
+    /// };
+    ///
+    /// println!("All done!");
+    /// ```
+    #[clippy::version = "1.60.0"]
+    pub SIGNIFICANT_DROP_IN_SCRUTINEE,
+    nursery,
+    "warns when a temporary of a type with a drop with a significant side-effect might have a surprising lifetime"
+}
+
+declare_lint_pass!(SignificantDropInScrutinee => [SIGNIFICANT_DROP_IN_SCRUTINEE]);
+
+impl<'tcx> LateLintPass<'tcx> for SignificantDropInScrutinee {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        if let Some(suggestions) = has_significant_drop_in_scrutinee(cx, expr) {
+            for found in suggestions {
+                span_lint_and_then(
+                    cx,
+                    SIGNIFICANT_DROP_IN_SCRUTINEE,
+                    found.found_span,
+                    "temporary with significant drop in match scrutinee",
+                    |diag| set_diagnostic(diag, cx, expr, found),
+                )
+            }
+        }
+    }
+}
+
+fn set_diagnostic<'tcx>(diag: &mut Diagnostic, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, found: FoundSigDrop) {
+    if found.lint_suggestion == LintSuggestion::MoveAndClone {
+        // If our suggestion is to move and clone, then we want to leave it to the user to
+        // decide how to address this lint, since it may be that cloning is inappropriate.
+        // Therefore, we won't to emit a suggestion.
+        return;
+    }
+
+    let original = snippet(cx, found.found_span, "..");
+    let trailing_indent = " ".repeat(indent_of(cx, found.found_span).unwrap_or(0));
+
+    let replacement = if found.lint_suggestion == LintSuggestion::MoveAndDerefToCopy {
+        format!("let value = *{};\n{}", original, trailing_indent)
+    } else if found.is_unit_return_val {
+        // If the return value of the expression to be moved is unit, then we don't need to
+        // capture the result in a temporary -- we can just replace it completely with `()`.
+        format!("{};\n{}", original, trailing_indent)
+    } else {
+        format!("let value = {};\n{}", original, trailing_indent)
+    };
+
+    let suggestion_message = if found.lint_suggestion == LintSuggestion::MoveOnly {
+        "try moving the temporary above the match"
+    } else {
+        "try moving the temporary above the match and create a copy"
+    };
+
+    let scrutinee_replacement = if found.is_unit_return_val {
+        "()".to_owned()
+    } else {
+        "value".to_owned()
+    };
+
+    diag.multipart_suggestion(
+        suggestion_message,
+        vec![
+            (expr.span.shrink_to_lo(), replacement),
+            (found.found_span, scrutinee_replacement),
+        ],
+        Applicability::MaybeIncorrect,
+    );
+}
+
+/// If the expression is an ExprKind::Match, check if the scrutinee has a significant drop that may
+/// have a surprising lifetime.
+fn has_significant_drop_in_scrutinee<'tcx, 'a>(
+    cx: &'a LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+) -> Option<Vec<FoundSigDrop>> {
+    let mut helper = SigDropHelper::new(cx);
+    match expr.kind {
+        ExprKind::Match(match_expr, _, _) => helper.find_sig_drop(match_expr),
+        _ => None,
+    }
+}
+
+struct SigDropHelper<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    is_chain_end: bool,
+    seen_types: FxHashSet<Ty<'tcx>>,
+    has_significant_drop: bool,
+    current_sig_drop: Option<FoundSigDrop>,
+    sig_drop_spans: Option<Vec<FoundSigDrop>>,
+    special_handling_for_binary_op: bool,
+}
+
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+enum LintSuggestion {
+    MoveOnly,
+    MoveAndDerefToCopy,
+    MoveAndClone,
+}
+
+#[derive(Clone, Copy)]
+struct FoundSigDrop {
+    found_span: Span,
+    is_unit_return_val: bool,
+    lint_suggestion: LintSuggestion,
+}
+
+impl<'a, 'tcx> SigDropHelper<'a, 'tcx> {
+    fn new(cx: &'a LateContext<'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,
+        }
+    }
+
+    fn find_sig_drop(&mut self, match_expr: &'tcx Expr<'_>) -> Option<Vec<FoundSigDrop>> {
+        self.visit_expr(match_expr);
+
+        // If sig drop spans is empty but we found a significant drop, it means that we didn't find
+        // a type that was trivially copyable as we moved up the chain after finding a significant
+        // drop, so move the entire scrutinee.
+        if self.has_significant_drop && self.sig_drop_spans.is_none() {
+            self.try_setting_current_suggestion(match_expr, true);
+            self.move_current_suggestion();
+        }
+
+        self.sig_drop_spans.take()
+    }
+
+    /// This will try to set the current suggestion (so it can be moved into the suggestions vec
+    /// later). If allow_move_and_clone is false, the suggestion *won't* be set -- this gives us
+    /// an opportunity to look for another type in the chain that will be trivially copyable.
+    /// However, if we are at the the end of the chain, we want to accept whatever is there. (The
+    /// suggestion won't actually be output, but the diagnostic message will be output, so the user
+    /// can determine the best way to handle the lint.)
+    fn try_setting_current_suggestion(&mut self, expr: &'tcx Expr<'_>, allow_move_and_clone: bool) {
+        if self.current_sig_drop.is_some() {
+            return;
+        }
+        let ty = self.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
+            if let Some(TypeAndMut { ty, .. }) = ty.builtin_deref(true) {
+                if ty.is_trivially_pure_clone_copy() {
+                    self.current_sig_drop.replace(FoundSigDrop {
+                        found_span: expr.span,
+                        is_unit_return_val: false,
+                        lint_suggestion: LintSuggestion::MoveAndDerefToCopy,
+                    });
+                } else if allow_move_and_clone {
+                    self.current_sig_drop.replace(FoundSigDrop {
+                        found_span: expr.span,
+                        is_unit_return_val: false,
+                        lint_suggestion: LintSuggestion::MoveAndClone,
+                    });
+                }
+            }
+        } else if ty.is_trivially_pure_clone_copy() {
+            self.current_sig_drop.replace(FoundSigDrop {
+                found_span: expr.span,
+                is_unit_return_val: false,
+                lint_suggestion: LintSuggestion::MoveOnly,
+            });
+        }
+    }
+
+    fn move_current_suggestion(&mut self) {
+        if let Some(current) = self.current_sig_drop.take() {
+            self.sig_drop_spans.get_or_insert_with(Vec::new).push(current);
+        }
+    }
+
+    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<'_>,
+        right: &'tcx Expr<'_>,
+        is_unit_return_val: bool,
+        span: Span,
+    ) {
+        self.special_handling_for_binary_op = true;
+        self.visit_expr(left);
+        self.visit_expr(right);
+
+        // If either side had a significant drop, suggest moving the entire scrutinee to avoid
+        // unnecessary copies and to simplify cases where both sides have significant drops.
+        if self.has_significant_drop {
+            self.current_sig_drop.replace(FoundSigDrop {
+                found_span: span,
+                is_unit_return_val,
+                lint_suggestion: LintSuggestion::MoveOnly,
+            });
+        }
+
+        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(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, _) => self.has_sig_drop_attr(cx, *ty),
+            rustc_middle::ty::RawPtr(TypeAndMut { ty, .. }) => self.has_sig_drop_attr(cx, *ty),
+            rustc_middle::ty::Ref(_, ty, _) => self.has_sig_drop_attr(cx, *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)) {
+            self.has_significant_drop = true;
+            return;
+        }
+        self.is_chain_end = false;
+
+        match ex.kind {
+            ExprKind::MethodCall(_, [ref expr, ..], _) => {
+                self.visit_expr(expr)
+            }
+            ExprKind::Binary(_, left, right) => {
+                self.visit_exprs_for_binary_ops(left, right, false, ex.span);
+            }
+            ExprKind::Assign(left, right, _) => {
+                self.visit_exprs_for_binary_ops(left, right, true, ex.span);
+            }
+            ExprKind::AssignOp(_, left, right) => {
+                self.visit_exprs_for_binary_ops(left, right, true, ex.span);
+            }
+            ExprKind::Tup(exprs) => {
+                for expr in exprs {
+                    self.visit_expr(expr);
+                    if self.has_significant_drop {
+                        // We may have not have set current_sig_drop if all the suggestions were
+                        // MoveAndClone, so add this tuple item's full expression in that case.
+                        if self.current_sig_drop.is_none() {
+                            self.try_setting_current_suggestion(expr, true);
+                        }
+
+                        // Now we are guaranteed to have something, so add it to the final vec.
+                        self.move_current_suggestion();
+                    }
+                    // Reset `has_significant_drop` after each tuple expression so we can look for
+                    // additional cases.
+                    self.has_significant_drop = false;
+                }
+                if self.sig_drop_spans.is_some() {
+                    self.has_significant_drop = true;
+                }
+            }
+            ExprKind::Box(..) |
+                ExprKind::Array(..) |
+                ExprKind::Call(..) |
+                ExprKind::Unary(..) |
+                ExprKind::If(..) |
+                ExprKind::Match(..) |
+                ExprKind::Field(..) |
+                ExprKind::Index(..) |
+                ExprKind::Ret(..) |
+                ExprKind::Repeat(..) |
+                ExprKind::Yield(..) |
+                ExprKind::MethodCall(..) => walk_expr(self, ex),
+            ExprKind::AddrOf(_, _, _) |
+                ExprKind::Block(_, _) |
+                ExprKind::Break(_, _) |
+                ExprKind::Cast(_, _) |
+                // Don't want to check the closure itself, only invocation, which is covered by MethodCall
+                ExprKind::Closure(_, _, _, _, _) |
+                ExprKind::ConstBlock(_) |
+                ExprKind::Continue(_) |
+                ExprKind::DropTemps(_) |
+                ExprKind::Err |
+                ExprKind::InlineAsm(_) |
+                ExprKind::Let(_) |
+                ExprKind::Lit(_) |
+                ExprKind::Loop(_, _, _, _) |
+                ExprKind::Path(_) |
+                ExprKind::Struct(_, _, _) |
+                ExprKind::Type(_, _) => {
+                return;
+            }
+        }
+
+        // Once a significant temporary has been found, we need to go back up at least 1 level to
+        // find the span to extract for replacement, so the temporary gets dropped. However, for
+        // binary ops, we want to move the whole scrutinee so we avoid unnecessary copies and to
+        // simplify cases where both sides have significant drops.
+        if self.has_significant_drop && !self.special_handling_for_binary_op {
+            self.try_setting_current_suggestion(ex, false);
+        }
+    }
+}

clippy_utils/src/attrs.rs

@@ -22,6 +22,7 @@ pub enum DeprecationStatus {
     ("cyclomatic_complexity", DeprecationStatus::Replaced("cognitive_complexity")),
     ("dump",                  DeprecationStatus::None),
     ("msrv",                  DeprecationStatus::None),
+    ("has_significant_drop",  DeprecationStatus::None),
 ];
 
 pub struct LimitStack {

tests/ui/significant_drop_in_scrutinee.rs

@@ -0,0 +1,526 @@
+// FIXME: Ideally these suggestions would be fixed via rustfix. Blocked by rust-lang/rust#53934
+// // run-rustfix
+
+#![warn(clippy::significant_drop_in_scrutinee)]
+#![allow(clippy::single_match)]
+#![allow(clippy::match_single_binding)]
+#![allow(unused_assignments)]
+#![allow(dead_code)]
+
+use std::ops::Deref;
+use std::sync::atomic::{AtomicU64, Ordering};
+use std::sync::{Mutex, MutexGuard};
+
+struct State {}
+
+impl State {
+    fn foo(&self) -> bool {
+        true
+    }
+
+    fn bar(&self) {}
+}
+
+fn should_not_trigger_lint_with_mutex_guard_outside_match() {
+    let mutex = Mutex::new(State {});
+
+    // Should not trigger lint because the temporary should drop at the `;` on line before the match
+    let is_foo = mutex.lock().unwrap().foo();
+    match is_foo {
+        true => {
+            mutex.lock().unwrap().bar();
+        }
+        false => {}
+    };
+}
+
+fn should_not_trigger_lint_with_mutex_guard_when_taking_ownership_in_match() {
+    let mutex = Mutex::new(State {});
+
+    // Should not trigger lint because the scrutinee is explicitly returning the MutexGuard,
+    // so its lifetime should not be surprising.
+    match mutex.lock() {
+        Ok(guard) => {
+            guard.foo();
+            mutex.lock().unwrap().bar();
+        }
+        _ => {}
+    };
+}
+
+fn should_trigger_lint_with_mutex_guard_in_match_scrutinee() {
+    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 => {
+            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.
+    match 1u64.to_string().is_empty() {
+        true => {
+            println!("It was empty")
+        }
+        false => {
+            println!("It was not empty")
+        }
+    }
+}
+
+struct StateWithMutex {
+    m: Mutex<u64>,
+}
+
+struct MutexGuardWrapper<'a> {
+    mg: MutexGuard<'a, u64>,
+}
+
+impl<'a> MutexGuardWrapper<'a> {
+    fn get_the_value(&self) -> u64 {
+        *self.mg.deref()
+    }
+}
+
+struct MutexGuardWrapperWrapper<'a> {
+    mg: MutexGuardWrapper<'a>,
+}
+
+impl<'a> MutexGuardWrapperWrapper<'a> {
+    fn get_the_value(&self) -> u64 {
+        *self.mg.mg.deref()
+    }
+}
+
+impl StateWithMutex {
+    fn lock_m(&self) -> MutexGuardWrapper<'_> {
+        MutexGuardWrapper {
+            mg: self.m.lock().unwrap(),
+        }
+    }
+
+    fn lock_m_m(&self) -> MutexGuardWrapperWrapper<'_> {
+        MutexGuardWrapperWrapper {
+            mg: MutexGuardWrapper {
+                mg: self.m.lock().unwrap(),
+            },
+        }
+    }
+
+    fn foo(&self) -> bool {
+        true
+    }
+
+    fn bar(&self) {}
+}
+
+fn should_trigger_lint_with_wrapped_mutex() {
+    let s = StateWithMutex { m: Mutex::new(1) };
+
+    // Should trigger lint because a temporary contains a type with a significant drop and its
+    // lifetime is not obvious. Additionally, it is not obvious from looking at the scrutinee that
+    // the temporary contains such a type, making it potentially even more surprising.
+    match s.lock_m().get_the_value() {
+        1 => {
+            println!("Got 1. Is it still 1?");
+            println!("{}", s.lock_m().get_the_value());
+        }
+        2 => {
+            println!("Got 2. Is it still 2?");
+            println!("{}", s.lock_m().get_the_value());
+        }
+        _ => {}
+    }
+    println!("All done!");
+}
+
+fn should_trigger_lint_with_double_wrapped_mutex() {
+    let s = StateWithMutex { m: Mutex::new(1) };
+
+    // Should trigger lint because a temporary contains a type which further contains a type with a
+    // significant drop and its lifetime is not obvious. Additionally, it is not obvious from
+    // looking at the scrutinee that the temporary contains such a type, making it potentially even
+    // more surprising.
+    match s.lock_m_m().get_the_value() {
+        1 => {
+            println!("Got 1. Is it still 1?");
+            println!("{}", s.lock_m().get_the_value());
+        }
+        2 => {
+            println!("Got 2. Is it still 2?");
+            println!("{}", s.lock_m().get_the_value());
+        }
+        _ => {}
+    }
+    println!("All done!");
+}
+
+struct Counter {
+    i: AtomicU64,
+}
+
+#[clippy::has_significant_drop]
+struct CounterWrapper<'a> {
+    counter: &'a Counter,
+}
+
+impl<'a> CounterWrapper<'a> {
+    fn new(counter: &Counter) -> CounterWrapper {
+        counter.i.fetch_add(1, Ordering::Relaxed);
+        CounterWrapper { counter }
+    }
+}
+
+impl<'a> Drop for CounterWrapper<'a> {
+    fn drop(&mut self) {
+        self.counter.i.fetch_sub(1, Ordering::Relaxed);
+    }
+}
+
+impl Counter {
+    fn temp_increment(&self) -> Vec<CounterWrapper> {
+        vec![CounterWrapper::new(self), CounterWrapper::new(self)]
+    }
+}
+
+fn should_trigger_lint_for_vec() {
+    let counter = Counter { i: AtomicU64::new(0) };
+
+    // Should trigger lint because the temporary in the scrutinee returns a collection of types
+    // which have significant drops. The types with significant drops are also non-obvious when
+    // reading the expression in the scrutinee.
+    match counter.temp_increment().len() {
+        2 => {
+            let current_count = counter.i.load(Ordering::Relaxed);
+            println!("Current count {}", current_count);
+            assert_eq!(current_count, 0);
+        }
+        1 => {}
+        3 => {}
+        _ => {}
+    };
+}
+
+struct StateWithField {
+    s: String,
+}
+
+// Should trigger lint only on the type in the tuple which is created using a temporary
+// with a significant drop. Additionally, this test ensures that the format of the tuple
+// is preserved correctly in the suggestion.
+fn should_trigger_lint_for_tuple_in_scrutinee() {
+    let mutex1 = Mutex::new(StateWithField { s: "one".to_owned() });
+
+    {
+        match (mutex1.lock().unwrap().s.len(), true) {
+            (3, _) => {
+                println!("started");
+                mutex1.lock().unwrap().s.len();
+                println!("done");
+            }
+            (_, _) => {}
+        };
+
+        match (true, mutex1.lock().unwrap().s.len(), true) {
+            (_, 3, _) => {
+                println!("started");
+                mutex1.lock().unwrap().s.len();
+                println!("done");
+            }
+            (_, _, _) => {}
+        };
+
+        let mutex2 = Mutex::new(StateWithField { s: "two".to_owned() });
+        match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) {
+            (3, _, 3) => {
+                println!("started");
+                mutex1.lock().unwrap().s.len();
+                mutex2.lock().unwrap().s.len();
+                println!("done");
+            }
+            (_, _, _) => {}
+        };
+
+        let mutex3 = Mutex::new(StateWithField { s: "three".to_owned() });
+        match mutex3.lock().unwrap().s.as_str() {
+            "three" => {
+                println!("started");
+                mutex1.lock().unwrap().s.len();
+                mutex2.lock().unwrap().s.len();
+                println!("done");
+            }
+            _ => {}
+        };
+
+
+        match (true, mutex3.lock().unwrap().s.as_str()) {
+            (_, "three") => {
+                println!("started");
+                mutex1.lock().unwrap().s.len();
+                mutex2.lock().unwrap().s.len();
+                println!("done");
+            }
+            (_, _) => {}
+        };
+    }
+}
+
+// Should trigger lint when either side of a binary operation creates a temporary with a
+// significant drop.
+// To avoid potential unnecessary copies or creating references that would trigger the significant
+// drop problem, the lint recommends moving the entire binary operation.
+fn should_trigger_lint_for_accessing_field_in_mutex_in_one_side_of_binary_op() {
+    let mutex = Mutex::new(StateWithField { s: "state".to_owned() });
+
+    match mutex.lock().unwrap().s.len() > 1 {
+        true => {
+            mutex.lock().unwrap().s.len();
+        }
+        false => {}
+    };
+
+    match 1 < mutex.lock().unwrap().s.len() {
+        true => {
+            mutex.lock().unwrap().s.len();
+        }
+        false => {}
+    };
+}
+
+// Should trigger lint when both sides of a binary operation creates a temporary with a
+// significant drop.
+// To avoid potential unnecessary copies or creating references that would trigger the significant
+// drop problem, the lint recommends moving the entire binary operation.
+fn should_trigger_lint_for_accessing_fields_in_mutex_in_both_sides_of_binary_op() {
+    let mutex1 = Mutex::new(StateWithField { s: "state".to_owned() });
+    let mutex2 = Mutex::new(StateWithField { s: "statewithfield".to_owned() });
+
+    match mutex1.lock().unwrap().s.len() < mutex2.lock().unwrap().s.len() {
+        true => {
+            println!("{} < {}", mutex1.lock().unwrap().s.len(), mutex2.lock().unwrap().s.len());
+        }
+        false => {}
+    };
+
+    match mutex1.lock().unwrap().s.len() >= mutex2.lock().unwrap().s.len() {
+        true => {
+            println!("{} >= {}", mutex1.lock().unwrap().s.len(), mutex2.lock().unwrap().s.len());
+        }
+        false => {}
+    };
+}
+
+fn should_not_trigger_lint_for_closure_in_scrutinee() {
+    let mutex1 = Mutex::new(StateWithField { s: "one".to_owned() });
+
+    let get_mutex_guard = || mutex1.lock().unwrap().s.len();
+
+    // Should not trigger lint because the temporary with a significant drop will be dropped
+    // at the end of the closure, so the MutexGuard will be unlocked and not have a potentially
+    // surprising lifetime.
+    match get_mutex_guard() > 1 {
+        true => {
+            mutex1.lock().unwrap().s.len();
+        }
+        false => {}
+    };
+}
+
+fn should_trigger_lint_for_return_from_closure_in_scrutinee() {
+    let mutex1 = Mutex::new(StateWithField { s: "one".to_owned() });
+
+    let get_mutex_guard = || mutex1.lock().unwrap();
+
+    // Should trigger lint because the temporary with a significant drop is returned from the
+    // closure but not used directly in any match arms, so it has a potentially surprising lifetime.
+    match get_mutex_guard().s.len() > 1 {
+        true => {
+            mutex1.lock().unwrap().s.len();
+        }
+        false => {}
+    };
+}
+
+fn should_trigger_lint_for_return_from_match_in_scrutinee() {
+    let mutex1 = Mutex::new(StateWithField { s: "one".to_owned() });
+    let mutex2 = Mutex::new(StateWithField { s: "two".to_owned() });
+
+    let i = 100;
+
+    // Should trigger lint because the nested match within the scrutinee returns a temporary with a
+    // significant drop is but not used directly in any match arms, so it has a potentially
+    // surprising lifetime.
+    match match i { 100 => mutex1.lock().unwrap(), _ => mutex2.lock().unwrap() }.s.len() > 1 {
+        true => {
+            mutex1.lock().unwrap().s.len();
+        }
+        false => {
+            println!("nothing to do here");
+        }
+    };
+}
+
+fn should_trigger_lint_for_return_from_if_in_scrutinee() {
+    let mutex1 = Mutex::new(StateWithField { s: "one".to_owned() });
+    let mutex2 = Mutex::new(StateWithField { s: "two".to_owned() });
+
+    let i = 100;
+
+    // Should trigger lint because the nested if-expression within the scrutinee returns a temporary
+    // with a significant drop is but not used directly in any match arms, so it has a potentially
+    // surprising lifetime.
+    match if i > 1 { mutex1.lock().unwrap() } else { mutex2.lock().unwrap() }.s.len() > 1 {
+        true => {
+            mutex1.lock().unwrap().s.len();
+        }
+        false => {}
+    };
+}
+
+fn should_not_trigger_lint_for_if_in_scrutinee() {
+    let mutex = Mutex::new(StateWithField { s: "state".to_owned() });
+
+    let i = 100;
+
+    // Should not trigger the lint because the temporary with a significant drop *is* dropped within
+    // the body of the if-expression nested within the match scrutinee, and therefore does not have
+    // a potentially surprising lifetime.
+    match if i > 1 { mutex.lock().unwrap().s.len() > 1 } else { false } {
+        true => {
+            mutex.lock().unwrap().s.len();
+        }
+        false => {}
+    };
+}
+
+struct StateWithBoxedMutexGuard {
+    u: Mutex<u64>,
+}
+
+impl StateWithBoxedMutexGuard {
+    fn new() -> StateWithBoxedMutexGuard {
+        StateWithBoxedMutexGuard { u: Mutex::new(42) }
+    }
+    fn lock(&self) -> Box<MutexGuard<u64>> {
+        Box::new(self.u.lock().unwrap())
+    }
+}
+
+fn should_trigger_lint_for_boxed_mutex_guard() {
+    let s = StateWithBoxedMutexGuard::new();
+
+    // Should trigger lint because a temporary Box holding a type with a significant drop in a match
+    // scrutinee may have a potentially surprising lifetime.
+    match s.lock().deref().deref() {
+        0 | 1 => println!("Value was less than 2"),
+        _ => println!("Value is {}", s.lock().deref()),
+    };
+}
+
+struct StateStringWithBoxedMutexGuard {
+    s: Mutex<String>,
+}
+
+impl StateStringWithBoxedMutexGuard {
+    fn new() -> StateStringWithBoxedMutexGuard {
+        StateStringWithBoxedMutexGuard { s: Mutex::new("A String".to_owned()) }
+    }
+    fn lock(&self) -> Box<MutexGuard<String>> {
+        Box::new(self.s.lock().unwrap())
+    }
+}
+
+fn should_trigger_lint_for_boxed_mutex_guard_holding_string() {
+    let s = StateStringWithBoxedMutexGuard::new();
+
+    let matcher = String::from("A String");
+
+    // Should trigger lint because a temporary Box holding a type with a significant drop in a match
+    // scrutinee may have a potentially surprising lifetime.
+    match s.lock().deref().deref() {
+        matcher => println!("Value is {}", s.lock().deref()),
+        _ => println!("Value was not a match"),
+    };
+}
+
+
+struct StateWithIntField {
+    i: u64,
+}
+
+// Should trigger lint when either side of an assign expression contains a temporary with a
+// significant drop, because the temporary's lifetime will be extended to the end of the match.
+// To avoid potential unnecessary copies or creating references that would trigger the significant
+// drop problem, the lint recommends moving the entire binary operation.
+fn should_trigger_lint_in_assign_expr() {
+    let mutex = Mutex::new(StateWithIntField { i: 10 });
+
+    let mut i = 100;
+
+    match mutex.lock().unwrap().i = i {
+        _ => {
+            println!("{}", mutex.lock().unwrap().i);
+        }
+    };
+
+    match i = mutex.lock().unwrap().i {
+        _ => {
+            println!("{}", mutex.lock().unwrap().i);
+        }
+    };
+
+    match mutex.lock().unwrap().i += 1 {
+        _ => {
+            println!("{}", mutex.lock().unwrap().i);
+        }
+    };
+
+    match i += mutex.lock().unwrap().i {
+        _ => {
+            println!("{}", mutex.lock().unwrap().i);
+        }
+    };
+}
+
+#[derive(Debug)]
+enum RecursiveEnum {
+    Foo(Option<Box<RecursiveEnum>>)
+}
+
+#[derive(Debug)]
+enum GenericRecursiveEnum<T> {
+    Foo(T, Option<Box<GenericRecursiveEnum<T>>>)
+}
+
+fn should_not_cause_stack_overflow() {
+    // Test that when a type recursively contains itself, a stack overflow does not occur when
+    // checking sub-types for significant drops.
+    let f = RecursiveEnum::Foo(Some(Box::new(RecursiveEnum::Foo(None))));
+    match f {
+        RecursiveEnum::Foo(Some(f)) => {
+            println!("{:?}", f)
+        }
+        RecursiveEnum::Foo(f) => {
+            println!("{:?}", f)
+        }
+    }
+
+    let f = GenericRecursiveEnum::Foo(1u64, Some(Box::new(GenericRecursiveEnum::Foo(2u64, None))));
+    match f {
+        GenericRecursiveEnum::Foo(i, Some(f)) => {
+            println!("{} {:?}", i, f)
+        }
+        GenericRecursiveEnum::Foo(i, f) => {
+            println!("{} {:?}", i, f)
+        }
+    }
+}
+
+fn main() {}

tests/ui/significant_drop_in_scrutinee.stderr

@@ -0,0 +1,261 @@
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:57:11
+   |
+LL |     match mutex.lock().unwrap().foo() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::significant-drop-in-scrutinee` implied by `-D warnings`
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:130:11
+   |
+LL |     match s.lock_m().get_the_value() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:151:11
+   |
+LL |     match s.lock_m_m().get_the_value() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:199:11
+   |
+LL |     match counter.temp_increment().len() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try moving the temporary above the match
+   |
+LL ~     let value = counter.temp_increment().len();
+LL ~     match value {
+   |
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:222:16
+   |
+LL |         match (mutex1.lock().unwrap().s.len(), true) {
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:231:22
+   |
+LL |         match (true, mutex1.lock().unwrap().s.len(), true) {
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:241:16
+   |
+LL |         match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) {
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:241:54
+   |
+LL |         match (mutex1.lock().unwrap().s.len(), true, mutex2.lock().unwrap().s.len()) {
+   |                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:252:15
+   |
+LL |         match mutex3.lock().unwrap().s.as_str() {
+   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:263:22
+   |
+LL |         match (true, mutex3.lock().unwrap().s.as_str()) {
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:282:11
+   |
+LL |     match mutex.lock().unwrap().s.len() > 1 {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:289:11
+   |
+LL |     match 1 < mutex.lock().unwrap().s.len() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:305:11
+   |
+LL |     match mutex1.lock().unwrap().s.len() < mutex2.lock().unwrap().s.len() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:312:11
+   |
+LL |     match mutex1.lock().unwrap().s.len() >= mutex2.lock().unwrap().s.len() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:343:11
+   |
+LL |     match get_mutex_guard().s.len() > 1 {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:360:11
+   |
+LL |     match match i { 100 => mutex1.lock().unwrap(), _ => mutex2.lock().unwrap() }.s.len() > 1 {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try moving the temporary above the match
+   |
+LL ~     let value = match i { 100 => mutex1.lock().unwrap(), _ => mutex2.lock().unwrap() }.s.len() > 1;
+LL ~     match value {
+   |
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:379:11
+   |
+LL |     match if i > 1 { mutex1.lock().unwrap() } else { mutex2.lock().unwrap() }.s.len() > 1 {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try moving the temporary above the match
+   |
+LL ~     let value = if i > 1 { mutex1.lock().unwrap() } else { mutex2.lock().unwrap() }.s.len() > 1;
+LL ~     match value {
+   |
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:421:11
+   |
+LL |     match s.lock().deref().deref() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+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
+  --> $DIR/significant_drop_in_scrutinee.rs:447:11
+   |
+LL |     match s.lock().deref().deref() {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:467:11
+   |
+LL |     match mutex.lock().unwrap().i = i {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try moving the temporary above the match
+   |
+LL ~     mutex.lock().unwrap().i = i;
+LL ~     match () {
+   |
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:473:11
+   |
+LL |     match i = mutex.lock().unwrap().i {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try moving the temporary above the match
+   |
+LL ~     i = mutex.lock().unwrap().i;
+LL ~     match () {
+   |
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:479:11
+   |
+LL |     match mutex.lock().unwrap().i += 1 {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try moving the temporary above the match
+   |
+LL ~     mutex.lock().unwrap().i += 1;
+LL ~     match () {
+   |
+
+error: temporary with significant drop in match scrutinee
+  --> $DIR/significant_drop_in_scrutinee.rs:485:11
+   |
+LL |     match i += mutex.lock().unwrap().i {
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try moving the temporary above the match
+   |
+LL ~     i += mutex.lock().unwrap().i;
+LL ~     match () {
+   |
+
+error: aborting due to 23 previous errors
+