Improve univariant match case

Author: compiler-errors

compiler/rustc_hir/src/hir.rs

@@ -1497,6 +1497,14 @@ impl<'hir> Pat<'hir> {
         });
         is_never_pattern
     }
+
+    pub fn peel_borrows(&self) -> &Self {
+        let mut pat = self;
+        while let PatKind::Ref(inner, _) = &pat.kind {
+            pat = inner;
+        }
+        pat
+    }
 }
 
 /// A single field in a struct pattern.

compiler/rustc_lint/src/if_let_rescope.rs

@@ -6,10 +6,12 @@ use rustc_ast::Recovered;
 use rustc_errors::{
     Applicability, Diag, EmissionGuarantee, SubdiagMessageOp, Subdiagnostic, SuggestionStyle,
 };
+use rustc_hir::def::Res;
+use rustc_hir::def_id::DefId;
 use rustc_hir::{self as hir, HirIdSet};
 use rustc_macros::LintDiagnostic;
-use rustc_middle::ty::TyCtxt;
 use rustc_middle::ty::adjustment::Adjust;
+use rustc_middle::ty::{self, TyCtxt};
 use rustc_session::lint::{FutureIncompatibilityReason, LintId};
 use rustc_session::{declare_lint, impl_lint_pass};
 use rustc_span::Span;
@@ -161,7 +163,7 @@ impl IfLetRescope {
                 let lifetime_end = source_map.end_point(conseq.span);
 
                 if let ControlFlow::Break(significant_dropper) =
-                    (FindSignificantDropper { cx }).check_if_let_scrutinee(init)
+                    (FindSignificantDropper { cx }).check_if_let_scrutinee(init, pat)
                 {
                     first_if_to_lint = first_if_to_lint.or_else(|| Some((span, expr.hir_id)));
                     significant_droppers.push(significant_dropper);
@@ -374,7 +376,52 @@ impl<'tcx> FindSignificantDropper<'_, 'tcx> {
     /// of the scrutinee itself, and also recurses into the expression to find any ref
     /// exprs (or autoref) which would promote temporaries that would be scoped to the
     /// end of this `if`.
-    fn check_if_let_scrutinee(&mut self, init: &'tcx hir::Expr<'tcx>) -> ControlFlow<Span> {
+    fn check_if_let_scrutinee(
+        &mut self,
+        init: &'tcx hir::Expr<'tcx>,
+        scrut_pat: &'tcx hir::Pat<'tcx>,
+    ) -> ControlFlow<Span> {
+        // Try first to match a variant constructor. If that variant constructor's
+        // subpatterns are infallible (which for now is just `x` and `_`), then
+        // check the fields of the ADT for drop *excluding* that variant, and walk
+        // that pattern excluding any of the peeled refs up to that point.
+        match &scrut_pat.peel_borrows().kind {
+            rustc_hir::PatKind::Struct(qpath, pat_fields, _) => {
+                if let Res::Def(_, def_id) =
+                    self.cx.typeck_results().qpath_res(qpath, scrut_pat.hir_id)
+                    && pat_fields.iter().all(|pat| is_infallible(pat.pat))
+                {
+                    // Peel the borrows of the init expr because we want to drill down to
+                    // the (possibly) temporary value. For example, given something like:
+                    // `let Enum::Variant(_) = &&&temp()`, we want to drill down to the
+                    // type of `temp()` so we can check it excluding the variant.
+                    self.check_promoted_temp_with_drop_excluding_variant(
+                        init.peel_borrows(),
+                        def_id,
+                    )?;
+                    // Once we've already checked the adt, just visit the expression after
+                    // all the refs. Otherwise we'll re-encounter the expression and count
+                    // it as a bad temporary.
+                    return self.visit_expr(init.peel_borrows());
+                }
+            }
+            rustc_hir::PatKind::TupleStruct(qpath, pats, _) => {
+                if let Res::Def(_, def_id) =
+                    self.cx.typeck_results().qpath_res(qpath, scrut_pat.hir_id)
+                    && let def_id = self.cx.tcx.parent(def_id)
+                    && pats.iter().all(|pat| is_infallible(pat))
+                {
+                    // See above for the justification here.
+                    self.check_promoted_temp_with_drop_excluding_variant(
+                        init.peel_borrows(),
+                        def_id,
+                    )?;
+                    return self.visit_expr(init.peel_borrows());
+                }
+            }
+            _ => {}
+        }
+
         self.check_promoted_temp_with_drop(init)?;
         self.visit_expr(init)
     }
@@ -386,23 +433,55 @@ impl<'tcx> FindSignificantDropper<'_, 'tcx> {
     /// or is the scrutinee of the `if let`, *and* the expression is not a place
     /// expr, and it has a significant drop.
     fn check_promoted_temp_with_drop(&self, expr: &'tcx hir::Expr<'tcx>) -> ControlFlow<Span> {
-        if !expr.is_place_expr(|base| {
-            self.cx
+        if !self.expr_is_place(expr)
+            && self
+                .cx
                 .typeck_results()
-                .adjustments()
-                .get(base.hir_id)
-                .is_some_and(|x| x.iter().any(|adj| matches!(adj.kind, Adjust::Deref(_))))
-        }) && self
-            .cx
-            .typeck_results()
-            .expr_ty(expr)
-            .has_significant_drop(self.cx.tcx, self.cx.typing_env())
+                .expr_ty(expr)
+                .has_significant_drop(self.cx.tcx, self.cx.typing_env())
+        {
+            ControlFlow::Break(expr.span)
+        } else {
+            ControlFlow::Continue(())
+        }
+    }
+
+    /// Same as above, but ignoring the excluded variant def-id.
+    fn check_promoted_temp_with_drop_excluding_variant(
+        &self,
+        expr: &'tcx hir::Expr<'tcx>,
+        excluded_variant: DefId,
+    ) -> ControlFlow<Span> {
+        if !self.expr_is_place(expr)
+            && let ty::Adt(def, args) = self.cx.typeck_results().expr_ty(expr).kind()
+            && def.variants().iter().any(|variant| {
+                variant.def_id != excluded_variant
+                    && variant.fields.iter().any(|f| {
+                        f.ty(self.cx.tcx, args)
+                            .has_significant_drop(self.cx.tcx, self.cx.typing_env())
+                    })
+            })
         {
             ControlFlow::Break(expr.span)
         } else {
             ControlFlow::Continue(())
         }
     }
+
+    fn expr_is_place(&self, expr: &rustc_hir::Expr<'tcx>) -> bool {
+        expr.is_place_expr(|base| {
+            self.cx
+                .typeck_results()
+                .adjustments()
+                .get(base.hir_id)
+                .is_some_and(|x| x.iter().any(|adj| matches!(adj.kind, Adjust::Deref(_))))
+        })
+    }
+}
+
+// FIXME: This is pretty rudimentary.
+fn is_infallible<'tcx>(pat: &'tcx hir::Pat<'tcx>) -> bool {
+    matches!(pat.kind, hir::PatKind::Wild | hir::PatKind::Binding(_, _, _, None))
 }
 
 impl<'tcx> Visitor<'tcx> for FindSignificantDropper<'_, 'tcx> {

tests/ui/drop/lint-if-let-rescope-false-positives.rs

@@ -18,6 +18,11 @@ impl Drop {
 
 fn consume(_: impl Sized) -> Option<i32> { Some(1) }
 
+enum MyOption<T> {
+    Some { f: T },
+    None,
+}
+
 fn main() {
     let drop = Drop;
 
@@ -26,4 +31,9 @@ fn main() {
 
     // Make sure we don't lint if we consume the droppy value.
     if let None = consume(Drop) {} else {}
+
+    // Make sure we don't lint if the droppy value is unconditionally matched.
+    if let Some(_) = Some(Drop) {} else {}
+    if let Some(_) = &Some(Drop) {} else {}
+    if let MyOption::Some { f: _ } = (MyOption::Some{ f: Drop }) {} else {}
 }