let_chains: Remove ast_validation logic in favor of lowering with recovery.

Author: Centril

src/librustc/hir/lowering.rs

@@ -4344,12 +4344,39 @@ impl<'a> LoweringContext<'a> {
                 let ohs = P(self.lower_expr(ohs));
                 hir::ExprKind::AddrOf(m, ohs)
             }
-            ExprKind::Let(..) => {
-                // This should have been caught `ast_validation`!
-                self.sess.span_err(e.span, "`let` expressions only supported in `if`");
-                // ^-- FIXME(53667): Change to `delay_span_bug` when let_chains handled in lowering.
-                self.sess.abort_if_errors();
-                hir::ExprKind::Err
+            ExprKind::Let(ref pats, ref scrutinee) => {
+                // If we got here, the `let` expression is not allowed.
+                self.sess
+                    .struct_span_err(e.span, "`let` expressions are not supported here")
+                    .note("only supported directly in conditions of `if`- and `while`-expressions")
+                    .note("as well as when nested within `&&` and parenthesis in those conditions")
+                    .emit();
+
+                // For better recovery, we emit:
+                // ```
+                // match scrutinee { pats => 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.
+                // 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 expr = self.expr_bool(e.span, true);
+                    self.arm(pats, P(expr))
+                };
+                let else_arm = {
+                    let pats = hir_vec![self.pat_wild(e.span)];
+                    let expr = self.expr_bool(e.span, false);
+                    self.arm(pats, P(expr))
+                };
+                hir::ExprKind::Match(
+                    P(scrutinee),
+                    vec![then_arm, else_arm].into(),
+                    hir::MatchSource::Normal,
+                )
             }
             // FIXME(#53667): handle lowering of && and parens.
             ExprKind::If(ref cond, ref then, ref else_opt) => {
@@ -5431,10 +5458,15 @@ impl<'a> LoweringContext<'a> {
         )
     }
 
+    /// Constructs a `true` or `false` literal expression.
+    fn expr_bool(&mut self, span: Span, val: bool) -> hir::Expr {
+        let lit = Spanned { span, node: LitKind::Bool(val) };
+        self.expr(span, hir::ExprKind::Lit(lit), ThinVec::new())
+    }
+
     /// Constructs a `true` or `false` literal pattern.
     fn pat_bool(&mut self, span: Span, val: bool) -> P<hir::Pat> {
-        let lit = Spanned { span, node: LitKind::Bool(val) };
-        let expr = self.expr(span, hir::ExprKind::Lit(lit), ThinVec::new());
+        let expr = self.expr_bool(span, val);
         self.pat(span, hir::PatKind::Lit(P(expr)))
     }
 

src/librustc_passes/ast_validation.rs

@@ -74,9 +74,6 @@ struct AstValidator<'a> {
     /// these booleans.
     warning_period_57979_didnt_record_next_impl_trait: bool,
     warning_period_57979_impl_trait_in_proj: bool,
-
-    /// Used to ban `let` expressions in inappropriate places.
-    is_let_allowed: bool,
 }
 
 /// With the `new` value in `store`,
@@ -114,12 +111,6 @@ impl<'a> AstValidator<'a> {
         with(self, outer, |this| &mut this.outer_impl_trait, f)
     }
 
-    fn with_let_allowed(&mut self, v: bool, f: impl FnOnce(&mut Self, bool)) {
-        let old = mem::replace(&mut self.is_let_allowed, v);
-        f(self, old);
-        self.is_let_allowed = old;
-    }
-
     fn visit_assoc_ty_constraint_from_generic_args(&mut self, constraint: &'a AssocTyConstraint) {
         match constraint.kind {
             AssocTyConstraintKind::Equality { ref ty } => {
@@ -335,15 +326,6 @@ impl<'a> AstValidator<'a> {
         }
     }
 
-    /// Emits an error banning the `let` expression provided.
-    fn ban_let_expr(&self, expr: &'a Expr) {
-        self.err_handler()
-            .struct_span_err(expr.span, "`let` expressions are not supported here")
-            .note("only supported directly in conditions of `if`- and `while`-expressions")
-            .note("as well as when nested within `&&` and parenthesis in those conditions")
-            .emit();
-    }
-
     fn check_fn_decl(&self, fn_decl: &FnDecl) {
         fn_decl
             .inputs
@@ -470,48 +452,17 @@ fn validate_generics_order<'a>(
 
 impl<'a> Visitor<'a> for AstValidator<'a> {
     fn visit_expr(&mut self, expr: &'a Expr) {
-        self.with_let_allowed(false, |this, let_allowed| {
-            match &expr.node {
-                ExprKind::Let(_, _) if !let_allowed => {
-                    this.ban_let_expr(expr);
-                }
-                // Assuming the context permits, `($expr)` does not impose additional constraints.
-                ExprKind::Paren(_) => {
-                    this.with_let_allowed(let_allowed, |this, _| visit::walk_expr(this, expr));
-                    return; // We've already walked into `expr`.
-                }
-                // Assuming the context permits,
-                // l && r` allows decendants in `l` and `r` to be `let` expressions.
-                ExprKind::Binary(op, ..) if op.node == BinOpKind::And => {
-                    this.with_let_allowed(let_allowed, |this, _| visit::walk_expr(this, expr));
-                    return; // We've already walked into `expr`.
-                }
-                // However, we do allow it in the condition of the `if` expression.
-                // We do not allow `let` in `then` and `opt_else` directly.
-                ExprKind::If(cond, then, opt_else) => {
-                    this.visit_block(then);
-                    walk_list!(this, visit_expr, opt_else);
-                    this.with_let_allowed(true, |this, _| this.visit_expr(cond));
-                    return; // We've already walked into `expr`.
-                }
-                // The same logic applies to `While`.
-                ExprKind::While(cond, then, opt_label) => {
-                    walk_list!(this, visit_label, opt_label);
-                    this.visit_block(then);
-                    this.with_let_allowed(true, |this, _| this.visit_expr(cond));
-                    return; // We've already walked into `expr`.
-                }
-                ExprKind::Closure(_, _, _, fn_decl, _, _) => {
-                    this.check_fn_decl(fn_decl);
-                }
-                ExprKind::InlineAsm(..) if !this.session.target.target.options.allow_asm => {
-                    span_err!(this.session, expr.span, E0472, "asm! is unsupported on this target");
-                }
-                _ => {}
+        match &expr.node {
+            ExprKind::Closure(_, _, _, fn_decl, _, _) => {
+                self.check_fn_decl(fn_decl);
             }
+            ExprKind::InlineAsm(..) if !self.session.target.target.options.allow_asm => {
+                span_err!(self.session, expr.span, E0472, "asm! is unsupported on this target");
+            }
+            _ => {}
+        }
 
-            visit::walk_expr(this, expr);
-        });
+        visit::walk_expr(self, expr);
     }
 
     fn visit_ty(&mut self, ty: &'a Ty) {
@@ -923,7 +874,6 @@ pub fn check_crate(session: &Session, krate: &Crate) -> (bool, bool) {
         is_assoc_ty_bound_banned: false,
         warning_period_57979_didnt_record_next_impl_trait: false,
         warning_period_57979_impl_trait_in_proj: false,
-        is_let_allowed: false,
     };
     visit::walk_crate(&mut validator, krate);