Refactor away some functions from hir::pat_util

Author: petrochenkov

src/librustc/cfg/construct.rs

@@ -456,8 +456,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
                     // Visit the guard expression
                     let guard_exit = self.expr(&guard, guard_start);
 
-                    let this_has_bindings = pat_util::pat_contains_bindings_or_wild(
-                        &self.tcx.def_map.borrow(), &pat);
+                    let this_has_bindings = pat_util::pat_contains_bindings_or_wild(&pat);
 
                     // If both this pattern and the previous pattern
                     // were free of bindings, they must consist only

src/librustc/hir/mod.rs

@@ -524,14 +524,7 @@ pub enum PatKind {
     /// Represents a wildcard pattern (`_`)
     Wild,
 
-    /// A `PatKind::Ident` may either be a new bound variable,
-    /// or a unit struct/variant pattern, or a const pattern (in the last two cases
-    /// the third field must be `None`).
-    ///
-    /// In the unit or const pattern case, the parser can't determine
-    /// which it is. The resolver determines this, and
-    /// records this pattern's `NodeId` in an auxiliary
-    /// set (of "PatIdents that refer to unit patterns or constants").
+    /// A fresh binding `ref mut binding @ OPT_SUBPATTERN`.
     Binding(BindingMode, Spanned<Name>, Option<P<Pat>>),
 
     /// A struct or struct variant pattern, e.g. `Variant {x, y, ..}`.
@@ -547,10 +540,8 @@ pub enum PatKind {
     /// Such pattern can be resolved to a unit struct/variant or a constant.
     Path(Path),
 
-    /// An associated const named using the qualified path `<T>::CONST` or
-    /// `<T as Trait>::CONST`. Associated consts from inherent impls can be
-    /// referred to as simply `T::CONST`, in which case they will end up as
-    /// PatKind::Path, and the resolver will have to sort that out.
+    /// A path pattern written in qualified form, i.e. `<T as Trait>::CONST` or `<T>::CONST`.
+    /// Such patterns can only refer to associated constants at the moment.
     QPath(QSelf, Path),
 
     /// A tuple pattern `(a, b)`.

src/librustc/hir/pat_util.rs

@@ -10,14 +10,12 @@
 
 use hir::def::*;
 use hir::def_id::DefId;
+use hir::{self, PatKind};
 use ty::TyCtxt;
 use util::nodemap::FnvHashMap;
-
 use syntax::ast;
-use hir::{self, PatKind};
-use syntax::codemap::{respan, Span, Spanned, DUMMY_SP};
+use syntax::codemap::{Span, Spanned, DUMMY_SP};
 
-use std::cell::RefCell;
 use std::iter::{Enumerate, ExactSizeIterator};
 
 pub type PatIdMap = FnvHashMap<ast::Name, ast::NodeId>;
@@ -57,9 +55,9 @@ impl<T: ExactSizeIterator> EnumerateAndAdjustIterator for T {
 
 // This is used because same-named variables in alternative patterns need to
 // use the NodeId of their namesake in the first pattern.
-pub fn pat_id_map(dm: &RefCell<DefMap>, pat: &hir::Pat) -> PatIdMap {
+pub fn pat_id_map(pat: &hir::Pat) -> PatIdMap {
     let mut map = FnvHashMap();
-    pat_bindings(dm, pat, |_bm, p_id, _s, path1| {
+    pat_bindings(pat, |_bm, p_id, _s, path1| {
         map.insert(path1.node, p_id);
     });
     map
@@ -123,42 +121,25 @@ pub fn pat_is_resolved_const(dm: &DefMap, pat: &hir::Pat) -> bool {
     }
 }
 
-pub fn pat_is_binding(_: &DefMap, pat: &hir::Pat) -> bool {
-    match pat.node {
-        PatKind::Binding(..) => true,
-        _ => false
-    }
-}
-
-pub fn pat_is_binding_or_wild(_: &DefMap, pat: &hir::Pat) -> bool {
-    match pat.node {
-        PatKind::Binding(..) | PatKind::Wild => true,
-        _ => false
-    }
-}
-
-/// Call `it` on every "binding" in a pattern, e.g., on `a` in
+/// Call `f` on every "binding" in a pattern, e.g., on `a` in
 /// `match foo() { Some(a) => (), None => () }`
-pub fn pat_bindings<I>(_: &RefCell<DefMap>, pat: &hir::Pat, mut it: I) where
-    I: FnMut(hir::BindingMode, ast::NodeId, Span, &Spanned<ast::Name>),
+pub fn pat_bindings<F>(pat: &hir::Pat, mut f: F)
+    where F: FnMut(hir::BindingMode, ast::NodeId, Span, &Spanned<ast::Name>),
 {
     pat.walk(|p| {
-        match p.node {
-          PatKind::Binding(binding_mode, ref pth, _) => {
-            it(binding_mode, p.id, p.span, &respan(pth.span, pth.node));
-          }
-          _ => {}
+        if let PatKind::Binding(binding_mode, ref pth, _) = p.node {
+            f(binding_mode, p.id, p.span, pth);
         }
         true
     });
 }
 
 /// Checks if the pattern contains any patterns that bind something to
 /// an ident, e.g. `foo`, or `Foo(foo)` or `foo @ Bar(..)`.
-pub fn pat_contains_bindings(dm: &DefMap, pat: &hir::Pat) -> bool {
+pub fn pat_contains_bindings(pat: &hir::Pat) -> bool {
     let mut contains_bindings = false;
     pat.walk(|p| {
-        if pat_is_binding(dm, p) {
+        if let PatKind::Binding(..) = p.node {
             contains_bindings = true;
             false // there's at least one binding, can short circuit now.
         } else {
@@ -170,28 +151,25 @@ pub fn pat_contains_bindings(dm: &DefMap, pat: &hir::Pat) -> bool {
 
 /// Checks if the pattern contains any `ref` or `ref mut` bindings,
 /// and if yes whether its containing mutable ones or just immutables ones.
-pub fn pat_contains_ref_binding(dm: &RefCell<DefMap>, pat: &hir::Pat) -> Option<hir::Mutability> {
+pub fn pat_contains_ref_binding(pat: &hir::Pat) -> Option<hir::Mutability> {
     let mut result = None;
-    pat_bindings(dm, pat, |mode, _, _, _| {
-        match mode {
-            hir::BindingMode::BindByRef(m) => {
-                // Pick Mutable as maximum
-                match result {
-                    None | Some(hir::MutImmutable) => result = Some(m),
-                    _ => (),
-                }
+    pat_bindings(pat, |mode, _, _, _| {
+        if let hir::BindingMode::BindByRef(m) = mode {
+            // Pick Mutable as maximum
+            match result {
+                None | Some(hir::MutImmutable) => result = Some(m),
+                _ => (),
             }
-            hir::BindingMode::BindByValue(_) => { }
         }
     });
     result
 }
 
 /// Checks if the patterns for this arm contain any `ref` or `ref mut`
 /// bindings, and if yes whether its containing mutable ones or just immutables ones.
-pub fn arm_contains_ref_binding(dm: &RefCell<DefMap>, arm: &hir::Arm) -> Option<hir::Mutability> {
+pub fn arm_contains_ref_binding(arm: &hir::Arm) -> Option<hir::Mutability> {
     arm.pats.iter()
-            .filter_map(|pat| pat_contains_ref_binding(dm, pat))
+            .filter_map(|pat| pat_contains_ref_binding(pat))
             .max_by_key(|m| match *m {
                 hir::MutMutable => 1,
                 hir::MutImmutable => 0,
@@ -200,14 +178,15 @@ pub fn arm_contains_ref_binding(dm: &RefCell<DefMap>, arm: &hir::Arm) -> Option<
 
 /// Checks if the pattern contains any patterns that bind something to
 /// an ident or wildcard, e.g. `foo`, or `Foo(_)`, `foo @ Bar(..)`,
-pub fn pat_contains_bindings_or_wild(dm: &DefMap, pat: &hir::Pat) -> bool {
+pub fn pat_contains_bindings_or_wild(pat: &hir::Pat) -> bool {
     let mut contains_bindings = false;
     pat.walk(|p| {
-        if pat_is_binding_or_wild(dm, p) {
-            contains_bindings = true;
-            false // there's at least one binding/wildcard, can short circuit now.
-        } else {
-            true
+        match p.node {
+            PatKind::Binding(..) | PatKind::Wild => {
+                contains_bindings = true;
+                false // there's at least one binding/wildcard, can short circuit now.
+            }
+            _ => true
         }
     });
     contains_bindings

src/librustc/middle/expr_use_visitor.rs

@@ -612,8 +612,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
         match local.init {
             None => {
                 let delegate = &mut self.delegate;
-                pat_util::pat_bindings(&self.mc.infcx.tcx.def_map, &local.pat,
-                                       |_, id, span, _| {
+                pat_util::pat_bindings(&local.pat, |_, id, span, _| {
                     delegate.decl_without_init(id, span);
                 })
             }
@@ -932,23 +931,16 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
         debug!("determine_pat_move_mode cmt_discr={:?} pat={:?}", cmt_discr,
                pat);
         return_if_err!(self.mc.cat_pattern(cmt_discr, pat, |_mc, cmt_pat, pat| {
-            let def_map = &self.tcx().def_map;
-            if pat_util::pat_is_binding(&def_map.borrow(), pat) {
-                match pat.node {
-                    PatKind::Binding(hir::BindByRef(_), _, _) =>
-                        mode.lub(BorrowingMatch),
-                    PatKind::Binding(hir::BindByValue(_), _, _) => {
-                        match copy_or_move(self.mc.infcx, &cmt_pat, PatBindingMove) {
-                            Copy => mode.lub(CopyingMatch),
-                            Move(_) => mode.lub(MovingMatch),
-                        }
-                    }
-                    _ => {
-                        span_bug!(
-                            pat.span,
-                            "binding pattern not an identifier");
+            match pat.node {
+                PatKind::Binding(hir::BindByRef(..), _, _) =>
+                    mode.lub(BorrowingMatch),
+                PatKind::Binding(hir::BindByValue(..), _, _) => {
+                    match copy_or_move(self.mc.infcx, &cmt_pat, PatBindingMove) {
+                        Copy => mode.lub(CopyingMatch),
+                        Move(..) => mode.lub(MovingMatch),
                     }
                 }
+                _ => {}
             }
         }));
     }
@@ -968,83 +960,74 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
         let def_map = &self.tcx().def_map;
         let delegate = &mut self.delegate;
         return_if_err!(mc.cat_pattern(cmt_discr.clone(), pat, |mc, cmt_pat, pat| {
-            if pat_util::pat_is_binding(&def_map.borrow(), pat) {
-                debug!("binding cmt_pat={:?} pat={:?} match_mode={:?}",
-                       cmt_pat,
-                       pat,
-                       match_mode);
-
-                // pat_ty: the type of the binding being produced.
-                let pat_ty = return_if_err!(infcx.node_ty(pat.id));
-
-                // Each match binding is effectively an assignment to the
-                // binding being produced.
-                let def = def_map.borrow().get(&pat.id).unwrap().full_def();
-                match mc.cat_def(pat.id, pat.span, pat_ty, def) {
-                    Ok(binding_cmt) => {
+            match pat.node {
+                PatKind::Binding(bmode, _, _) => {
+                    debug!("binding cmt_pat={:?} pat={:?} match_mode={:?}",
+                           cmt_pat,
+                           pat,
+                           match_mode);
+
+                    // pat_ty: the type of the binding being produced.
+                    let pat_ty = return_if_err!(infcx.node_ty(pat.id));
+
+                    // Each match binding is effectively an assignment to the
+                    // binding being produced.
+                    let def = def_map.borrow().get(&pat.id).unwrap().full_def();
+                    if let Ok(binding_cmt) = mc.cat_def(pat.id, pat.span, pat_ty, def) {
                         delegate.mutate(pat.id, pat.span, binding_cmt, MutateMode::Init);
                     }
-                    Err(_) => { }
-                }
 
-                // It is also a borrow or copy/move of the value being matched.
-                match pat.node {
-                    PatKind::Binding(hir::BindByRef(m), _, _) => {
-                        if let ty::TyRef(&r, _) = pat_ty.sty {
-                            let bk = ty::BorrowKind::from_mutbl(m);
-                            delegate.borrow(pat.id, pat.span, cmt_pat,
-                                            r, bk, RefBinding);
+                    // It is also a borrow or copy/move of the value being matched.
+                    match bmode {
+                        hir::BindByRef(m) => {
+                            if let ty::TyRef(&r, _) = pat_ty.sty {
+                                let bk = ty::BorrowKind::from_mutbl(m);
+                                delegate.borrow(pat.id, pat.span, cmt_pat,
+                                                r, bk, RefBinding);
+                            }
+                        }
+                        hir::BindByValue(..) => {
+                            let mode = copy_or_move(infcx, &cmt_pat, PatBindingMove);
+                            debug!("walk_pat binding consuming pat");
+                            delegate.consume_pat(pat, cmt_pat, mode);
                         }
-                    }
-                    PatKind::Binding(hir::BindByValue(_), _, _) => {
-                        let mode = copy_or_move(infcx, &cmt_pat, PatBindingMove);
-                        debug!("walk_pat binding consuming pat");
-                        delegate.consume_pat(pat, cmt_pat, mode);
-                    }
-                    _ => {
-                        span_bug!(
-                            pat.span,
-                            "binding pattern not an identifier");
                     }
                 }
-            } else {
-                match pat.node {
-                    PatKind::Vec(_, Some(ref slice_pat), _) => {
-                        // The `slice_pat` here creates a slice into
-                        // the original vector.  This is effectively a
-                        // borrow of the elements of the vector being
-                        // matched.
-
-                        let (slice_cmt, slice_mutbl, slice_r) =
-                            return_if_err!(mc.cat_slice_pattern(cmt_pat, &slice_pat));
-
-                        // Note: We declare here that the borrow
-                        // occurs upon entering the `[...]`
-                        // pattern. This implies that something like
-                        // `[a; b]` where `a` is a move is illegal,
-                        // because the borrow is already in effect.
-                        // In fact such a move would be safe-ish, but
-                        // it effectively *requires* that we use the
-                        // nulling out semantics to indicate when a
-                        // value has been moved, which we are trying
-                        // to move away from.  Otherwise, how can we
-                        // indicate that the first element in the
-                        // vector has been moved?  Eventually, we
-                        // could perhaps modify this rule to permit
-                        // `[..a, b]` where `b` is a move, because in
-                        // that case we can adjust the length of the
-                        // original vec accordingly, but we'd have to
-                        // make trans do the right thing, and it would
-                        // only work for `Box<[T]>`s. It seems simpler
-                        // to just require that people call
-                        // `vec.pop()` or `vec.unshift()`.
-                        let slice_bk = ty::BorrowKind::from_mutbl(slice_mutbl);
-                        delegate.borrow(pat.id, pat.span,
-                                        slice_cmt, slice_r,
-                                        slice_bk, RefBinding);
-                    }
-                    _ => { }
+                PatKind::Vec(_, Some(ref slice_pat), _) => {
+                    // The `slice_pat` here creates a slice into
+                    // the original vector.  This is effectively a
+                    // borrow of the elements of the vector being
+                    // matched.
+
+                    let (slice_cmt, slice_mutbl, slice_r) =
+                        return_if_err!(mc.cat_slice_pattern(cmt_pat, &slice_pat));
+
+                    // Note: We declare here that the borrow
+                    // occurs upon entering the `[...]`
+                    // pattern. This implies that something like
+                    // `[a; b]` where `a` is a move is illegal,
+                    // because the borrow is already in effect.
+                    // In fact such a move would be safe-ish, but
+                    // it effectively *requires* that we use the
+                    // nulling out semantics to indicate when a
+                    // value has been moved, which we are trying
+                    // to move away from.  Otherwise, how can we
+                    // indicate that the first element in the
+                    // vector has been moved?  Eventually, we
+                    // could perhaps modify this rule to permit
+                    // `[..a, b]` where `b` is a move, because in
+                    // that case we can adjust the length of the
+                    // original vec accordingly, but we'd have to
+                    // make trans do the right thing, and it would
+                    // only work for `Box<[T]>`s. It seems simpler
+                    // to just require that people call
+                    // `vec.pop()` or `vec.unshift()`.
+                    let slice_bk = ty::BorrowKind::from_mutbl(slice_mutbl);
+                    delegate.borrow(pat.id, pat.span,
+                                    slice_cmt, slice_r,
+                                    slice_bk, RefBinding);
                 }
+                _ => {}
             }
         }));