extract method to read scrutinee conditionally

rust-lang · bors · Jul 13, 2022 · Jul 7, 2022 · Jul 7, 2022 · Jul 7, 2022
commit 8e4a9710841a21f77856ae1d8207015c220a1873
diff --git a/compiler/rustc_ast_lowering/src/block.rs b/compiler/rustc_ast_lowering/src/block.rs
@@ -36,20 +36,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             match s.kind {
                 StmtKind::Local(ref local) => {
                     let hir_id = self.lower_node_id(s.id);
-                    let els = if let LocalKind::InitElse(_, els) = &local.kind {
-                        if !self.tcx.features().let_else {
-                            feature_err(
-                                &self.tcx.sess.parse_sess,
-                                sym::let_else,
-                                s.span,
-                                "`let...else` statements are unstable",
-                            )
-                            .emit();
-                        }
-                        Some(self.lower_block(els, false))
-                    } else {
-                        None
-                    };
                     let local = self.lower_local(local);
                     self.alias_attrs(hir_id, local.hir_id);
                     let kind = hir::StmtKind::Local(local);
@@ -106,9 +92,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let hir_id = self.lower_node_id(l.id);
         let pat = self.lower_pat(&l.pat);
         let els = if let LocalKind::InitElse(_, els) = &l.kind {
-            if !self.sess.features_untracked().let_else {
+            if !self.tcx.features().let_else {
                 feature_err(
-                    &self.sess.parse_sess,
+                    &self.tcx.sess.parse_sess,
                     sym::let_else,
                     l.span,
                     "`let...else` statements are unstable",

diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs
@@ -2,7 +2,10 @@
 //! normal visitor, which just walks the entire body in one shot, the
 //! `ExprUseVisitor` determines how expressions are being used.
 
+use std::slice::from_ref;
+
 use hir::def::DefKind;
+use hir::Expr;
 // Export these here so that Clippy can use them.
 pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection};
 
@@ -257,91 +260,11 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
 
             hir::ExprKind::Match(ref discr, arms, _) => {
                 let discr_place = return_if_err!(self.mc.cat_expr(discr));
-
-                // Matching should not always be considered a use of the place, hence
-                // discr does not necessarily need to be borrowed.
-                // We only want to borrow discr if the pattern contain something other
-                // than wildcards.
-                let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self;
-                let mut needs_to_be_read = false;
-                for arm in arms.iter() {
-                    return_if_err!(mc.cat_pattern(discr_place.clone(), arm.pat, |place, pat| {
-                        match &pat.kind {
-                            PatKind::Binding(.., opt_sub_pat) => {
-                                // If the opt_sub_pat is None, than the binding does not count as
-                                // a wildcard for the purpose of borrowing discr.
-                                if opt_sub_pat.is_none() {
-                                    needs_to_be_read = true;
-                                }
-                            }
-                            PatKind::Path(qpath) => {
-                                // A `Path` pattern is just a name like `Foo`. This is either a
-                                // named constant or else it refers to an ADT variant
-
-                                let res = self.mc.typeck_results.qpath_res(qpath, pat.hir_id);
-                                match res {
-                                    Res::Def(DefKind::Const, _)
-                                    | Res::Def(DefKind::AssocConst, _) => {
-                                        // Named constants have to be equated with the value
-                                        // being matched, so that's a read of the value being matched.
-                                        //
-                                        // FIXME: We don't actually  reads for ZSTs.
-                                        needs_to_be_read = true;
-                                    }
-                                    _ => {
-                                        // Otherwise, this is a struct/enum variant, and so it's
-                                        // only a read if we need to read the discriminant.
-                                        needs_to_be_read |= is_multivariant_adt(place.place.ty());
-                                    }
-                                }
-                            }
-                            PatKind::TupleStruct(..) | PatKind::Struct(..) | PatKind::Tuple(..) => {
-                                // For `Foo(..)`, `Foo { ... }` and `(...)` patterns, check if we are matching
-                                // against a multivariant enum or struct. In that case, we have to read
-                                // the discriminant. Otherwise this kind of pattern doesn't actually
-                                // read anything (we'll get invoked for the `...`, which may indeed
-                                // perform some reads).
-
-                                let place_ty = place.place.ty();
-                                needs_to_be_read |= is_multivariant_adt(place_ty);
-                            }
-                            PatKind::Lit(_) | PatKind::Range(..) => {
-                                // If the PatKind is a Lit or a Range then we want
-                                // to borrow discr.
-                                needs_to_be_read = true;
-                            }
-                            PatKind::Or(_)
-                            | PatKind::Box(_)
-                            | PatKind::Slice(..)
-                            | PatKind::Ref(..)
-                            | PatKind::Wild => {
-                                // If the PatKind is Or, Box, Slice or Ref, the decision is made later
-                                // as these patterns contains subpatterns
-                                // If the PatKind is Wild, the decision is made based on the other patterns being
-                                // examined
-                            }
-                        }
-                    }));
-                }
-
-                if needs_to_be_read {
-                    self.borrow_expr(discr, ty::ImmBorrow);
-                } else {
-                    let closure_def_id = match discr_place.place.base {
-                        PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
-                        _ => None,
-                    };
-
-                    self.delegate.fake_read(
-                        &discr_place,
-                        FakeReadCause::ForMatchedPlace(closure_def_id),
-                        discr_place.hir_id,
-                    );
-
-                    // We always want to walk the discriminant. We want to make sure, for instance,
-                    // that the discriminant has been initialized.
-                    self.walk_expr(discr);
-                }
+                self.maybe_read_scrutinee(
+                    discr,
+                    discr_place.clone(),
+                    arms.iter().map(|arm| arm.pat),
+                );
 
                 // treatment of the discriminant is handled while walking the arms.
                 for arm in arms {
@@ -470,6 +393,97 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
         }
     }
 
+    fn maybe_read_scrutinee<'t>(
+        &mut self,
+        discr: &Expr<'_>,
+        discr_place: PlaceWithHirId<'tcx>,
+        pats: impl Iterator<Item = &'t hir::Pat<'t>>,
+    ) {
+        // Matching should not always be considered a use of the place, hence
+        // discr does not necessarily need to be borrowed.
+        // We only want to borrow discr if the pattern contain something other
+        // than wildcard
D7AE
s.
+        let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self;
+        let mut needs_to_be_read = false;
+        for pat in pats {
+            return_if_err!(mc.cat_pattern(discr_place.clone(), pat, |place, pat| {
+                match &pat.kind {
+                    PatKind::Binding(.., opt_sub_pat) => {
+                        // If the opt_sub_pat is None, than the binding does not count as
+                        // a wildcard for the purpose of borrowing discr.
+                        if opt_sub_pat.is_none() {
+                            needs_to_be_read = true;
+                        }
+                    }
+                    PatKind::Path(qpath) => {
+                        // A `Path` pattern is just a name like `Foo`. This is either a
+                        // named constant or else it refers to an ADT variant
+
+                        let res = self.mc.typeck_results.qpath_res(qpath, pat.hir_id);
+                        match res {
+                            Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) => {
+                                // Named constants have to be equated with the value
+                                // being matched, so that's a read of the value being matched.
+                                //
+                                // FIXME: We don't actually  reads for ZSTs.
+                                needs_to_be_read = true;
+                            }
+                            _ => {
+                                // Otherwise, this is a struct/enum variant, and so it's
+                                // only a read if we need to read the discriminant.
+                                needs_to_be_read |= is_multivariant_adt(place.place.ty());
+                            }
+                        }
+                    }
+                    PatKind::TupleStruct(..) | PatKind::Struct(..) | PatKind::Tuple(..) => {
+                        // For `Foo(..)`, `Foo { ... }` and `(...)` patterns, check if we are matching
+                        // against a multivariant enum or struct. In that case, we have to read
+                        // the discriminant. Otherwise this kind of pattern doesn't actually
+                        // read anything (we'll get invoked for the `...`, which may indeed
+                        // perform some reads).
+
+                        let place_ty = place.place.ty();
+                        needs_to_be_read |= is_multivariant_adt(place_ty);
+                    }
+                    PatKind::Lit(_) | PatKind::Range(..) => {
+                        // If the PatKind is a Lit or a Range then we want
+                        // to borrow discr.
+                        needs_to_be_read = true;
+                    }
+                    PatKind::Or(_)
+                    | PatKind::Box(_)
+                    | PatKind::Slice(..)
+                    | PatKind::Ref(..)
+                    | PatKind::Wild => {
+                        // If the PatKind is Or, Box, Slice or Ref, the decision is made later
+                        // as these patterns contains subpatterns
+                        // If the PatKind is Wild, the decision is made based on the other patterns being
+                        // examined
+                    }
+                }
+            }));
+        }
+
+        if needs_to_be_read {
+            self.borrow_expr(discr, ty::ImmBorrow);
+        } else {
+            let closure_def_id = match discr_place.place.base {
+                PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
+                _ => None,
+            };
+
+            self.delegate.fake_read(
+                &discr_place,
+                FakeReadCause::ForMatchedPlace(closure_def_id),
+                discr_place.hir_id,
+            );
+
+            // We always want to walk the discriminant. We want to make sure, for instance,
+            // that the discriminant has been initialized.
+            self.walk_expr(discr);
+        }
+    }
+
     fn walk_local<F>(
         &mut self,
         expr: &hir::Expr<'_>,
@@ -484,10 +498,12 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
         f(self);
         if let Some(els) = els {
             // borrowing because we need to test the descriminant
-            self.borrow_expr(expr, ImmBorrow);
+            // self.borrow_expr(expr, ImmBorrow);
+            self.maybe_read_scrutinee(expr, expr_place, from_ref(pat).iter());
             self.walk_block(els)
+        } else {
+            self.walk_irrefutable_pat(&expr_place, &pat);
         }
-        self.walk_irrefutable_pat(&expr_place, &pat);
     }
 
     /// Indicates that the value of `blk` will be consumed, meaning either copied or moved