or-patterns: Push PatKind/PatternKind::Or at top level to HIR & HAIR

src/librustc/hir/intravisit.rs

@@ -1103,7 +1103,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
 
 pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm) {
     visitor.visit_id(arm.hir_id);
-    walk_list!(visitor, visit_pat, &arm.pats);
+    visitor.visit_pat(&arm.pat);
     if let Some(ref g) = arm.guard {
         match g {
             Guard::If(ref e) => visitor.visit_expr(e),

src/librustc/hir/lowering.rs

@@ -434,35 +434,6 @@ impl<'a> LoweringContext<'a> {
                 visit::walk_pat(self, p)
             }
 
-            // HACK(or_patterns; Centril | dlrobertson): Avoid creating
-            // HIR  nodes for `PatKind::Or` for the top level of a `ast::Arm`.
-            // This is a temporary hack that should go away once we push down
-            // `arm.pats: HirVec<P<Pat>>` -> `arm.pat: P<Pat>` to HIR. // Centril
-            fn visit_arm(&mut self, arm: &'tcx Arm) {
-                match &arm.pat.node {
-                    PatKind::Or(pats) => pats.iter().for_each(|p| self.visit_pat(p)),
-                    _ => self.visit_pat(&arm.pat),
-                }
-                walk_list!(self, visit_expr, &arm.guard);
-                self.visit_expr(&arm.body);
-                walk_list!(self, visit_attribute, &arm.attrs);
-            }
-
-            // HACK(or_patterns; Centril | dlrobertson): Same as above. // Centril
-            fn visit_expr(&mut self, e: &'tcx Expr) {
-                if let ExprKind::Let(pat, scrutinee) = &e.node {
-                    walk_list!(self, visit_attribute, e.attrs.iter());
-                    match &pat.node {
-                        PatKind::Or(pats) => pats.iter().for_each(|p| self.visit_pat(p)),
-                        _ => self.visit_pat(&pat),
-                    }
-                    self.visit_expr(scrutinee);
-                    self.visit_expr_post(e);
-                    return;
-                }
-                visit::walk_expr(self, e)
-            }
-
             fn visit_item(&mut self, item: &'tcx Item) {
                 let hir_id = self.lctx.allocate_hir_id_counter(item.id);
 

src/librustc/hir/lowering/expr.rs

@@ -250,14 +250,14 @@ impl LoweringContext<'_> {
         // 4. The return type of the block is `bool` which seems like what the user wanted.
         let scrutinee = self.lower_expr(scrutinee);
         let then_arm = {
-            let pat = self.lower_pat_top_hack(pat);
+            let pat = self.lower_pat(pat);
             let expr = self.expr_bool(span, true);
             self.arm(pat, P(expr))
         };
         let else_arm = {
             let pat = self.pat_wild(span);
             let expr = self.expr_bool(span, false);
-            self.arm(hir_vec![pat], P(expr))
+            self.arm(pat, P(expr))
         };
         hir::ExprKind::Match(
             P(scrutinee),
@@ -281,7 +281,7 @@ impl LoweringContext<'_> {
             None => (self.expr_block_empty(span), false),
             Some(els) => (self.lower_expr(els), true),
         };
-        let else_arm = self.arm(hir_vec![else_pat], P(else_expr));
+        let else_arm = self.arm(else_pat, P(else_expr));
 
         // Handle then + scrutinee:
         let then_blk = self.lower_block(then, false);
@@ -290,7 +290,7 @@ impl LoweringContext<'_> {
             // `<pat> => <then>`:
             ExprKind::Let(ref pat, ref scrutinee) => {
                 let scrutinee = self.lower_expr(scrutinee);
-                let pat = self.lower_pat_top_hack(pat);
+                let pat = self.lower_pat(pat);
                 (pat, scrutinee, hir::MatchSource::IfLetDesugar { contains_else_clause })
             }
             // `true => <then>`:
@@ -307,7 +307,7 @@ impl LoweringContext<'_> {
                 // let temporaries live outside of `cond`.
                 let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new());
                 let pat = self.pat_bool(span, true);
-                (hir_vec![pat], cond, hir::MatchSource::IfDesugar { contains_else_clause })
+                (pat, cond, hir::MatchSource::IfDesugar { contains_else_clause })
             }
         };
         let then_arm = self.arm(then_pat, P(then_expr));
@@ -331,7 +331,7 @@ impl LoweringContext<'_> {
         let else_arm = {
             let else_pat = self.pat_wild(span);
             let else_expr = self.expr_break(span, ThinVec::new());
-            self.arm(hir_vec![else_pat], else_expr)
+            self.arm(else_pat, else_expr)
         };
 
         // Handle then + scrutinee:
@@ -348,7 +348,7 @@ impl LoweringContext<'_> {
                 //     }
                 //   }
                 let scrutinee = self.with_loop_condition_scope(|t| t.lower_expr(scrutinee));
-                let pat = self.lower_pat_top_hack(pat);
+                let pat = self.lower_pat(pat);
                 (pat, scrutinee, hir::MatchSource::WhileLetDesugar, hir::LoopSource::WhileLet)
             }
             _ => {
@@ -376,7 +376,7 @@ impl LoweringContext<'_> {
                 let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new());
                 // `true => <then>`:
                 let pat = self.pat_bool(span, true);
-                (hir_vec![pat], cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While)
+                (pat, cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While)
             }
         };
         let then_arm = self.arm(then_pat, P(then_expr));
@@ -429,7 +429,7 @@ impl LoweringContext<'_> {
         hir::Arm {
             hir_id: self.next_id(),
             attrs: self.lower_attrs(&arm.attrs),
-            pats: self.lower_pat_top_hack(&arm.pat),
+            pat: self.lower_pat(&arm.pat),
             guard: match arm.guard {
                 Some(ref x) => Some(hir::Guard::If(P(self.lower_expr(x)))),
                 _ => None,
@@ -439,16 +439,6 @@ impl LoweringContext<'_> {
         }
     }
 
-    /// HACK(or_patterns; Centril | dlrobertson): For now we don't push down top level or-patterns
-    /// `p | q` into `hir::PatKind::Or(...)` as post-lowering bits of the compiler are not ready
-    /// to deal with it. This should by fixed by pushing it down to HIR and then HAIR.
-    fn lower_pat_top_hack(&mut self, pat: &Pat) -> HirVec<P<hir::Pat>> {
-        match pat.node {
-            PatKind::Or(ref ps) => ps.iter().map(|x| self.lower_pat(x)).collect(),
-            _ => hir_vec![self.lower_pat(pat)],
-        }
-    }
-
     pub(super) fn make_async_expr(
         &mut self,
         capture_clause: CaptureBy,
@@ -597,7 +587,7 @@ impl LoweringContext<'_> {
                 );
                 P(this.expr(await_span, expr_break, ThinVec::new()))
             });
-            self.arm(hir_vec![ready_pat], break_x)
+            self.arm(ready_pat, break_x)
         };
 
         // `::std::task::Poll::Pending => {}`
@@ -608,7 +598,7 @@ impl LoweringContext<'_> {
                 hir_vec![],
             );
             let empty_block = P(self.expr_block_empty(span));
-            self.arm(hir_vec![pending_pat], empty_block)
+            self.arm(pending_pat, empty_block)
         };
 
         let inner_match_stmt = {
@@ -650,7 +640,7 @@ impl LoweringContext<'_> {
         });
 
         // mut pinned => loop { ... }
-        let pinned_arm = self.arm(hir_vec![pinned_pat], loop_expr);
+        let pinned_arm = self.arm(pinned_pat, loop_expr);
 
         // match <expr> {
         //     mut pinned => loop { .. }
@@ -1084,15 +1074,15 @@ impl LoweringContext<'_> {
                 ThinVec::new(),
             ));
             let some_pat = self.pat_some(pat.span, val_pat);
-            self.arm(hir_vec![some_pat], assign)
+            self.arm(some_pat, assign)
         };
 
         // `::std::option::Option::None => break`
         let break_arm = {
             let break_expr =
                 self.with_loop_scope(e.id, |this| this.expr_break(e.span, ThinVec::new()));
             let pat = self.pat_none(e.span);
-            self.arm(hir_vec![pat], break_expr)
+            self.arm(pat, break_expr)
         };
 
         // `mut iter`
@@ -1163,7 +1153,7 @@ impl LoweringContext<'_> {
         });
 
         // `mut iter => { ... }`
-        let iter_arm = self.arm(hir_vec![iter_pat], loop_expr);
+        let iter_arm = self.arm(iter_pat, loop_expr);
 
         // `match ::std::iter::IntoIterator::into_iter(<head>) { ... }`
         let into_iter_expr = {
@@ -1249,7 +1239,7 @@ impl LoweringContext<'_> {
                 ThinVec::from(attrs.clone()),
             ));
             let ok_pat = self.pat_ok(span, val_pat);
-            self.arm(hir_vec![ok_pat], val_expr)
+            self.arm(ok_pat, val_expr)
         };
 
         // `Err(err) => #[allow(unreachable_code)]
@@ -1284,7 +1274,7 @@ impl LoweringContext<'_> {
             };
 
             let err_pat = self.pat_err(try_span, err_local);
-            self.arm(hir_vec![err_pat], ret_expr)
+            self.arm(err_pat, ret_expr)
         };
 
         hir::ExprKind::Match(
@@ -1479,14 +1469,11 @@ impl LoweringContext<'_> {
         }
     }
 
-    /// HACK(or_patterns; Centril | dlrobertson): For now we don't push down top level or-patterns
-    /// `p | q` into `hir::PatKind::Or(...)` as post-lowering bits of the compiler are not ready
-    /// to deal with it. This should by fixed by pushing it down to HIR and then HAIR.
-    fn arm(&mut self, pats: HirVec<P<hir::Pat>>, expr: P<hir::Expr>) -> hir::Arm {
+    fn arm(&mut self, pat: P<hir::Pat>, expr: P<hir::Expr>) -> hir::Arm {
         hir::Arm {
             hir_id: self.next_id(),
             attrs: hir_vec![],
-            pats,
+            pat,
             guard: None,
             span: expr.span,
             body: expr,

src/librustc/hir/mod.rs

@@ -882,44 +882,61 @@ impl fmt::Debug for Pat {
 
 impl Pat {
     // FIXME(#19596) this is a workaround, but there should be a better way
-    fn walk_<G>(&self, it: &mut G) -> bool
-        where G: FnMut(&Pat) -> bool
-    {
+    fn walk_short_(&self, it: &mut impl FnMut(&Pat) -> bool) -> bool {
         if !it(self) {
             return false;
         }
 
-        match self.node {
-            PatKind::Binding(.., Some(ref p)) => p.walk_(it),
-            PatKind::Struct(_, ref fields, _) => {
-                fields.iter().all(|field| field.pat.walk_(it))
-            }
-            PatKind::TupleStruct(_, ref s, _) | PatKind::Tuple(ref s, _) => {
-                s.iter().all(|p| p.walk_(it))
-            }
-            PatKind::Or(ref pats) => pats.iter().all(|p| p.walk_(it)),
-            PatKind::Box(ref s) | PatKind::Ref(ref s, _) => {
-                s.walk_(it)
-            }
-            PatKind::Slice(ref before, ref slice, ref after) => {
+        use PatKind::*;
+        match &self.node {
+            Wild | Lit(_) | Range(..) | Binding(.., None) | Path(_) => true,
+            Box(s) | Ref(s, _) | Binding(.., Some(s)) => s.walk_short_(it),
+            Struct(_, fields, _) => fields.iter().all(|field| field.pat.walk_short_(it)),
+            TupleStruct(_, s, _) | Tuple(s, _) | Or(s) => s.iter().all(|p| p.walk_short_(it)),
+            Slice(before, slice, after) => {
                 before.iter()
                       .chain(slice.iter())
                       .chain(after.iter())
-                      .all(|p| p.walk_(it))
+                      .all(|p| p.walk_short_(it))
             }
-            PatKind::Wild |
-            PatKind::Lit(_) |
-            PatKind::Range(..) |
-            PatKind::Binding(..) |
-            PatKind::Path(_) => {
-                true
+        }
+    }
+
+    /// Walk the pattern in left-to-right order,
+    /// short circuiting (with `.all(..)`) if `false` is returned.
+    ///
+    /// Note that when visiting e.g. `Tuple(ps)`,
+    /// if visiting `ps[0]` returns `false`,
+    /// then `ps[1]` will not be visited.
+    pub fn walk_short(&self, mut it: impl FnMut(&Pat) -> bool) -> bool {
+        self.walk_short_(&mut it)
+    }
+
+    // FIXME(#19596) this is a workaround, but there should be a better way
+    fn walk_(&self, it: &mut impl FnMut(&Pat) -> bool) {
+        if !it(self) {
+            return;
+        }
+
+        use PatKind::*;
+        match &self.node {
+            Wild | Lit(_) | Range(..) | Binding(.., None) | Path(_) => {},
+            Box(s) | Ref(s, _) | Binding(.., Some(s)) => s.walk_(it),
+            Struct(_, fields, _) => fields.iter().for_each(|field| field.pat.walk_(it)),
+            TupleStruct(_, s, _) | Tuple(s, _) | Or(s) => s.iter().for_each(|p| p.walk_(it)),
+            Slice(before, slice, after) => {
+                before.iter()
+                      .chain(slice.iter())
+                      .chain(after.iter())
+                      .for_each(|p| p.walk_(it))
             }
         }
     }
 
-    pub fn walk<F>(&self, mut it: F) -> bool
-        where F: FnMut(&Pat) -> bool
-    {
+    /// Walk the pattern in left-to-right order.
+    ///
+    /// If `it(pat)` returns `false`, the children are not visited.
+    pub fn walk(&self, mut it: impl FnMut(&Pat) -> bool) {
         self.walk_(&mut it)
     }
 }
@@ -1259,21 +1276,32 @@ pub struct Local {
 }
 
 /// Represents a single arm of a `match` expression, e.g.
-/// `<pats> (if <guard>) => <body>`.
+/// `<pat> (if <guard>) => <body>`.
 #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)]
 pub struct Arm {
     #[stable_hasher(ignore)]
     pub hir_id: HirId,
     pub span: Span,
     pub attrs: HirVec<Attribute>,
-    /// Multiple patterns can be combined with `|`
-    pub pats: HirVec<P<Pat>>,
+    /// If this pattern and the optional guard matches, then `body` is evaluated.
+    pub pat: P<Pat>,
     /// Optional guard clause.
     pub guard: Option<Guard>,
     /// The expression the arm evaluates to if this arm matches.
     pub body: P<Expr>,
 }
 
+impl Arm {
+    // HACK(or_patterns; Centril | dlrobertson): Remove this and
+    // correctly handle each case in which this method is used.
+    pub fn top_pats_hack(&self) -> &[P<Pat>] {
+        match &self.pat.node {
+            PatKind::Or(pats) => pats,
+            _ => std::slice::from_ref(&self.pat),
+        }
+    }
+}
+
 #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)]
 pub enum Guard {
     If(P<Expr>),