significant_drop_in_scrutinee: Fix false positives due to false drops of place expressions

clippy_lints/src/matches/significant_drop_in_scrutinee.rs

@@ -115,45 +115,60 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
         }
     }
 
-    fn get_type(&self, ex: &'tcx Expr<'_>) -> Ty<'tcx> {
-        self.cx.typeck_results().expr_ty(ex)
+    fn is_sig_drop_expr(&mut self, ex: &'tcx Expr<'_>) -> bool {
+        !ex.is_syntactic_place_expr() && self.has_sig_drop_attr(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, ty: Ty<'tcx>) -> bool {
+        self.seen_types.clear();
+        self.has_sig_drop_attr_impl(ty)
     }
 
-    fn has_sig_drop_attr(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
+    fn has_sig_drop_attr_impl(&mut self, 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 {
+            if get_attr(
+                self.cx.sess(),
+                self.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;
-                    }
-                }
+        if !self.seen_types.insert(ty) {
+            return false;
+        }
 
-                for generic_arg in *b {
-                    if let GenericArgKind::Type(ty) = generic_arg.unpack() {
-                        if self.has_sig_drop_attr(cx, ty) {
-                            return true;
-                        }
-                    }
-                }
-                false
+        let result = match ty.kind() {
+            rustc_middle::ty::Adt(adt, args) => {
+                // if some field has significant drop,
+                adt.all_fields()
+                    .map(|field| field.ty(self.cx.tcx, args))
+                    .any(|ty| self.has_sig_drop_attr_impl(ty))
+                    // or if there is no generic lifetime and..
+                    // (to avoid false positive on `Ref<'a, MutexGuard<Foo>>`)
+                    || (args
+                        .iter()
+                        .all(|arg| !matches!(arg.unpack(), GenericArgKind::Lifetime(_)))
+                        // some generic parameter has significant drop
+                        // (to avoid false negative on `Box<MutexGuard<Foo>>`)
+                        && args
+                            .iter()
+                            .filter_map(|arg| match arg.unpack() {
+                                GenericArgKind::Type(ty) => Some(ty),
+                                _ => None,
+                            })
+                            .any(|ty| self.has_sig_drop_attr_impl(ty)))
             },
-            rustc_middle::ty::Array(ty, _)
-            | rustc_middle::ty::RawPtr(ty, _)
-            | rustc_middle::ty::Ref(_, ty, _)
-            | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr(cx, *ty),
+            rustc_middle::ty::Tuple(tys) => tys.iter().any(|ty| self.has_sig_drop_attr_impl(ty)),
+            rustc_middle::ty::Array(ty, _) | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr_impl(*ty),
             _ => false,
-        }
+        };
+
+        result
     }
 }
 
@@ -232,7 +247,7 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> {
         if self.current_sig_drop.is_some() {
             return;
         }
-        let ty = self.sig_drop_checker.get_type(expr);
+        let ty = self.cx.typeck_results().expr_ty(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
@@ -279,11 +294,7 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> {
 
 impl<'a, 'tcx> Visitor<'tcx> for SigDropHelper<'a, 'tcx> {
     fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
-        if !self.is_chain_end
-            && self
-                .sig_drop_checker
-                .has_sig_drop_attr(self.cx, self.sig_drop_checker.get_type(ex))
-        {
+        if !self.is_chain_end && self.sig_drop_checker.is_sig_drop_expr(ex) {
             self.has_significant_drop = true;
             return;
         }
@@ -387,10 +398,7 @@ fn has_significant_drop_in_arms<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'
 
 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))
-        {
+        if self.sig_drop_checker.is_sig_drop_expr(ex) {
             self.found_sig_drop_spans.insert(ex.span);
             return;
         }

tests/ui/significant_drop_in_scrutinee.rs

@@ -675,4 +675,60 @@ fn should_not_trigger_on_significant_iterator_drop() {
     }
 }
 
+// https://github.com/rust-lang/rust-clippy/issues/9072
+fn should_not_trigger_lint_if_place_expr_has_significant_drop() {
+    let x = Mutex::new(vec![1, 2, 3]);
+    let x_guard = x.lock().unwrap();
+
+    match x_guard[0] {
+        1 => println!("1!"),
+        x => println!("{x}"),
+    }
+
+    match x_guard.len() {
+        1 => println!("1!"),
+        x => println!("{x}"),
+    }
+}
+
+struct Guard<'a, T>(MutexGuard<'a, T>);
+
+struct Ref<'a, T>(&'a T);
+
+impl<'a, T> Guard<'a, T> {
+    fn guard(&self) -> &MutexGuard<T> {
+        &self.0
+    }
+
+    fn guard_ref(&self) -> Ref<MutexGuard<T>> {
+        Ref(&self.0)
+    }
+
+    fn take(self) -> Box<MutexGuard<'a, T>> {
+        Box::new(self.0)
+    }
+}
+
+fn should_not_trigger_for_significant_drop_ref() {
+    let mutex = Mutex::new(vec![1, 2]);
+    let guard = Guard(mutex.lock().unwrap());
+
+    match guard.guard().len() {
+        0 => println!("empty"),
+        _ => println!("not empty"),
+    }
+
+    match guard.guard_ref().0.len() {
+        0 => println!("empty"),
+        _ => println!("not empty"),
+    }
+
+    match guard.take().len() {
+        //~^ ERROR: temporary with significant `Drop` in `match` scrutinee will live until the
+        //~| NOTE: this might lead to deadlocks or other unexpected behavior
+        0 => println!("empty"),
+        _ => println!("not empty"),
+    };
+}
+
 fn main() {}

tests/ui/significant_drop_in_scrutinee.stderr

@@ -28,6 +28,9 @@ LL |     match s.lock_m().get_the_value() {
 LL |             println!("{}", s.lock_m().get_the_value());
    |                            ---------- another value with significant `Drop` created here
 ...
+LL |             println!("{}", s.lock_m().get_the_value());
+   |                            ---------- another value with significant `Drop` created here
+...
 LL |     }
    |      - temporary lives until here
    |
@@ -47,6 +50,9 @@ LL |     match s.lock_m_m().get_the_value() {
 LL |             println!("{}", s.lock_m().get_the_value());
    |                            ---------- another value with significant `Drop` created here
 ...
+LL |             println!("{}", s.lock_m().get_the_value());
+   |                            ---------- another value with significant `Drop` created here
+...
 LL |     }
    |      - temporary lives until here
    |
@@ -360,7 +366,7 @@ LL |     match s.lock().deref().deref() {
    |           ^^^^^^^^^^^^^^^^^^^^^^^^
 ...
 LL |         _ => println!("Value is {}", s.lock().deref()),
-   |                                      ---------------- another value with significant `Drop` created here
+   |                                      -------- another value with significant `Drop` created here
 LL |     };
    |      - temporary lives until here
    |
@@ -378,7 +384,7 @@ LL |     match s.lock().deref().deref() {
    |           ^^^^^^^^^^^^^^^^^^^^^^^^
 ...
 LL |         matcher => println!("Value is {}", s.lock().deref()),
-   |                                            ---------------- another value with significant `Drop` created here
+   |                                            -------- another value with significant `Drop` created here
 LL |         _ => println!("Value was not a match"),
 LL |     };
    |      - temporary lives until here
@@ -499,5 +505,21 @@ LL ~     let value = mutex.lock().unwrap().foo();
 LL ~     match value {
    |
 
-error: aborting due to 26 previous errors
+error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression
+  --> tests/ui/significant_drop_in_scrutinee.rs:726:11
+   |
+LL |     match guard.take().len() {
+   |           ^^^^^^^^^^^^^^^^^^
+...
+LL |     };
+   |      - 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 = guard.take().len();
+LL ~     match value {
+   |
+
+error: aborting due to 27 previous errors