Auto merge of #12764 - lrh2000:ignore-place, r=blyxyas

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

Place expressions do not really create temporaries, so they will not create significant drops. For example, the following code snippet is quite good (#8963):
```rust
fn main() {
    let x = std::sync::Mutex::new(vec![1, 2, 3]);
    let x_guard = x.lock().unwrap();
    match x_guard[0] {
        1 => println!("1!"),
        x => println!("{x}"),
    }
    drop(x_guard); // Some "usage"
}
```

Also, the previous logic thinks that references like `&MutexGuard<_>`/`Ref<'_, MutexGuard<'_, _>>` have significant drops, which is simply not true, so it is fixed together in this PR.

Fixes #8963
Fixes #9072

changelog: [`significant_drop_in_scrutinee`]: Fix false positives due to false drops of place expressions.

r? `@blyxyas`

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