Rollup merge of rust-lang#64111 - Centril:ast-only-patkind-or, r=petrochenkov

or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve

Following up on work in rust-lang#63693 and rust-lang#61708, in this PR we:

- Uniformly use `PatKind::Or(...)` in AST:

   - Change `ast::Arm.pats: Vec<P<Pat>>` => `ast::Arm.pat: P<Pat>`

   - Change `ast::ExprKind::Let.0: Vec<P<Pat>>` => `ast::ExprKind::Let.0: P<Pat>`

- Adjust `librustc_resolve/late.rs` to correctly handle or-patterns at any level of nesting as a result.

  In particular, the already-bound check which rejects e.g. `let (a, a);` now accounts for or-patterns. The consistency checking (ensures no missing bindings and binding mode consistency) also now accounts for or-patterns. In the process, a bug was found in the current compiler which allowed:

   ```rust
   enum E<T> { A(T, T), B(T) }
   use E::*;
   fn foo() {
       match A(0, 1) {
           B(mut a) | A(mut a, mut a) => {}
       }
   }
   ```

   The new algorithms took a few iterations to get right. I tried several clever schemes but ultimately a version based on a stack of hashsets and recording product/sum contexts was chosen since it is more clearly correct.

- Clean up `librustc_resolve/late.rs` by, among other things, using a new `with_rib` function to better ensure stack dicipline.

- Do not push the change in AST to HIR for now to avoid doing too much in this PR. To cope with  this, we introduce a temporary hack in `rustc::hir::lowering` (clearly marked in the diff).

cc rust-lang#54883
cc @dlrobertson @matthewjasper
r? @petrochenkov

Author: Centril

src/librustc/hir/lowering.rs

@@ -425,19 +425,44 @@ impl<'a> LoweringContext<'a> {
 
         impl<'tcx, 'interner> Visitor<'tcx> for MiscCollector<'tcx, 'interner> {
             fn visit_pat(&mut self, p: &'tcx Pat) {
-                match p.node {
+                if let PatKind::Paren(..) | PatKind::Rest = p.node {
                     // Doesn't generate a HIR node
-                    PatKind::Paren(..) | PatKind::Rest => {},
-                    _ => {
-                        if let Some(owner) = self.hir_id_owner {
-                            self.lctx.lower_node_id_with_owner(p.id, owner);
-                        }
-                    }
-                };
+                } else if let Some(owner) = self.hir_id_owner {
+                    self.lctx.lower_node_id_with_owner(p.id, owner);
+                }
 
                 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

@@ -68,7 +68,7 @@ impl LoweringContext<'_> {
                 let ohs = P(self.lower_expr(ohs));
                 hir::ExprKind::AddrOf(m, ohs)
             }
-            ExprKind::Let(ref pats, ref scrutinee) => self.lower_expr_let(e.span, pats, scrutinee),
+            ExprKind::Let(ref pat, ref scrutinee) => self.lower_expr_let(e.span, pat, scrutinee),
             ExprKind::If(ref cond, ref then, ref else_opt) => {
                 self.lower_expr_if(e.span, cond, then, else_opt.as_deref())
             }
@@ -227,16 +227,11 @@ impl LoweringContext<'_> {
         }
     }
 
-    /// Emit an error and lower `ast::ExprKind::Let(pats, scrutinee)` into:
+    /// Emit an error and lower `ast::ExprKind::Let(pat, scrutinee)` into:
     /// ```rust
     /// match scrutinee { pats => true, _ => false }
     /// ```
-    fn lower_expr_let(
-        &mut self,
-        span: Span,
-        pats: &[AstP<Pat>],
-        scrutinee: &Expr
-    ) -> hir::ExprKind {
+    fn lower_expr_let(&mut self, span: Span, pat: &Pat, scrutinee: &Expr) -> hir::ExprKind {
         // If we got here, the `let` expression is not allowed.
         self.sess
             .struct_span_err(span, "`let` expressions are not supported here")
@@ -246,23 +241,23 @@ impl LoweringContext<'_> {
 
         // For better recovery, we emit:
         // ```
-        // match scrutinee { pats => true, _ => false }
+        // match scrutinee { pat => true, _ => false }
         // ```
         // While this doesn't fully match the user's intent, it has key advantages:
         // 1. We can avoid using `abort_if_errors`.
-        // 2. We can typeck both `pats` and `scrutinee`.
-        // 3. `pats` is allowed to be refutable.
+        // 2. We can typeck both `pat` and `scrutinee`.
+        // 3. `pat` is allowed to be refutable.
         // 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 pats = pats.iter().map(|pat| self.lower_pat(pat)).collect();
+            let pat = self.lower_pat_top_hack(pat);
             let expr = self.expr_bool(span, true);
-            self.arm(pats, P(expr))
+            self.arm(pat, P(expr))
         };
         let else_arm = {
-            let pats = hir_vec![self.pat_wild(span)];
+            let pat = self.pat_wild(span);
             let expr = self.expr_bool(span, false);
-            self.arm(pats, P(expr))
+            self.arm(hir_vec![pat], P(expr))
         };
         hir::ExprKind::Match(
             P(scrutinee),
@@ -291,13 +286,12 @@ impl LoweringContext<'_> {
         // Handle then + scrutinee:
         let then_blk = self.lower_block(then, false);
         let then_expr = self.expr_block(then_blk, ThinVec::new());
-        let (then_pats, scrutinee, desugar) = match cond.node {
+        let (then_pat, scrutinee, desugar) = match cond.node {
             // `<pat> => <then>`:
-            ExprKind::Let(ref pats, ref scrutinee) => {
+            ExprKind::Let(ref pat, ref scrutinee) => {
                 let scrutinee = self.lower_expr(scrutinee);
-                let pats = pats.iter().map(|pat| self.lower_pat(pat)).collect();
-                let desugar = hir::MatchSource::IfLetDesugar { contains_else_clause };
-                (pats, scrutinee, desugar)
+                let pat = self.lower_pat_top_hack(pat);
+                (pat, scrutinee, hir::MatchSource::IfLetDesugar { contains_else_clause })
             }
             // `true => <then>`:
             _ => {
@@ -312,13 +306,11 @@ impl LoweringContext<'_> {
                 // to preserve drop semantics since `if cond { ... }` does not
                 // let temporaries live outside of `cond`.
                 let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new());
-
-                let desugar = hir::MatchSource::IfDesugar { contains_else_clause };
-                let pats = hir_vec![self.pat_bool(span, true)];
-                (pats, cond, desugar)
+                let pat = self.pat_bool(span, true);
+                (hir_vec![pat], cond, hir::MatchSource::IfDesugar { contains_else_clause })
             }
         };
-        let then_arm = self.arm(then_pats, P(then_expr));
+        let then_arm = self.arm(then_pat, P(then_expr));
 
         hir::ExprKind::Match(P(scrutinee), vec![then_arm, else_arm].into(), desugar)
     }
@@ -345,8 +337,8 @@ impl LoweringContext<'_> {
         // Handle then + scrutinee:
         let then_blk = self.lower_block(body, false);
         let then_expr = self.expr_block(then_blk, ThinVec::new());
-        let (then_pats, scrutinee, desugar, source) = match cond.node {
-            ExprKind::Let(ref pats, ref scrutinee) => {
+        let (then_pat, scrutinee, desugar, source) = match cond.node {
+            ExprKind::Let(ref pat, ref scrutinee) => {
                 // to:
                 //
                 //   [opt_ident]: loop {
@@ -356,9 +348,8 @@ impl LoweringContext<'_> {
                 //     }
                 //   }
                 let scrutinee = self.with_loop_condition_scope(|t| t.lower_expr(scrutinee));
-                let pats = pats.iter().map(|pat| self.lower_pat(pat)).collect();
-                let desugar = hir::MatchSource::WhileLetDesugar;
-                (pats, scrutinee, desugar, hir::LoopSource::WhileLet)
+                let pat = self.lower_pat_top_hack(pat);
+                (pat, scrutinee, hir::MatchSource::WhileLetDesugar, hir::LoopSource::WhileLet)
             }
             _ => {
                 // We desugar: `'label: while $cond $body` into:
@@ -383,14 +374,12 @@ impl LoweringContext<'_> {
                 // to preserve drop semantics since `while cond { ... }` does not
                 // let temporaries live outside of `cond`.
                 let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new());
-
-                let desugar = hir::MatchSource::WhileDesugar;
                 // `true => <then>`:
-                let pats = hir_vec![self.pat_bool(span, true)];
-                (pats, cond, desugar, hir::LoopSource::While)
+                let pat = self.pat_bool(span, true);
+                (hir_vec![pat], cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While)
             }
         };
-        let then_arm = self.arm(then_pats, P(then_expr));
+        let then_arm = self.arm(then_pat, P(then_expr));
 
         // `match <scrutinee> { ... }`
         let match_expr = self.expr_match(
@@ -440,7 +429,7 @@ impl LoweringContext<'_> {
         hir::Arm {
             hir_id: self.next_id(),
             attrs: self.lower_attrs(&arm.attrs),
-            pats: arm.pats.iter().map(|x| self.lower_pat(x)).collect(),
+            pats: self.lower_pat_top_hack(&arm.pat),
             guard: match arm.guard {
                 Some(ref x) => Some(hir::Guard::If(P(self.lower_expr(x)))),
                 _ => None,
@@ -450,6 +439,16 @@ 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,
@@ -1255,7 +1254,6 @@ impl LoweringContext<'_> {
                 ThinVec::from(attrs.clone()),
             ));
             let ok_pat = self.pat_ok(span, val_pat);
-
             self.arm(hir_vec![ok_pat], val_expr)
         };
 
@@ -1486,7 +1484,10 @@ impl LoweringContext<'_> {
         }
     }
 
-    fn arm(&mut self, pats: hir::HirVec<P<hir::Pat>>, expr: P<hir::Expr>) -> hir::Arm {
+    /// 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 {
         hir::Arm {
             hir_id: self.next_id(),
             attrs: hir_vec![],

src/librustc_lint/builtin.rs

@@ -772,7 +772,7 @@ impl EarlyLintPass for UnusedDocComment {
     }
 
     fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) {
-        let arm_span = arm.pats[0].span.with_hi(arm.body.span.hi());
+        let arm_span = arm.pat.span.with_hi(arm.body.span.hi());
         self.warn_if_doc(cx, arm_span, "match arms", false, &arm.attrs);
     }
 

src/librustc_lint/unused.rs

@@ -493,10 +493,8 @@ impl EarlyLintPass for UnusedParens {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
         use syntax::ast::ExprKind::*;
         let (value, msg, followed_by_block, left_pos, right_pos) = match e.node {
-            Let(ref pats, ..) => {
-                for p in pats {
-                    self.check_unused_parens_pat(cx, p, false, false);
-                }
+            Let(ref pat, ..) => {
+                self.check_unused_parens_pat(cx, pat, false, false);
                 return;
             }
 
@@ -594,9 +592,7 @@ impl EarlyLintPass for UnusedParens {
     }
 
     fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) {
-        for p in &arm.pats {
-            self.check_unused_parens_pat(cx, p, false, false);
-        }
+        self.check_unused_parens_pat(cx, &arm.pat, false, false);
     }
 }