Auto merge of #92080 - Aaron1011:pattern-ice, r=cjgillot

Move `PatKind::Lit` checking from ast_validation to ast lowering

Fixes #92074

This allows us to insert an `ExprKind::Err` when an invalid expression
is used in a literal pattern, preventing later stages of compilation
from seeing an unexpected literal pattern.

Author: bors

compiler/rustc_ast_lowering/src/pat.rs

@@ -24,7 +24,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         let lower_sub = |this: &mut Self| sub.as_ref().map(|s| this.lower_pat(&*s));
                         break self.lower_pat_ident(pattern, binding_mode, ident, lower_sub);
                     }
-                    PatKind::Lit(ref e) => break hir::PatKind::Lit(self.lower_expr(e)),
+                    PatKind::Lit(ref e) => {
+                        break hir::PatKind::Lit(self.lower_expr_within_pat(e, false));
+                    }
                     PatKind::TupleStruct(ref qself, ref path, ref pats) => {
                         let qpath = self.lower_qpath(
                             pattern.id,
@@ -81,8 +83,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     }
                     PatKind::Range(ref e1, ref e2, Spanned { node: ref end, .. }) => {
                         break hir::PatKind::Range(
-                            e1.as_deref().map(|e| self.lower_expr(e)),
-                            e2.as_deref().map(|e| self.lower_expr(e)),
+                            e1.as_deref().map(|e| self.lower_expr_within_pat(e, true)),
+                            e2.as_deref().map(|e| self.lower_expr_within_pat(e, true)),
                             self.lower_range_end(end, e2.is_some()),
                         );
                     }
@@ -314,4 +316,33 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             RangeEnd::Excluded | RangeEnd::Included(_) => hir::RangeEnd::Included,
         }
     }
+
+    /// Matches `'-' lit | lit (cf. parser::Parser::parse_literal_maybe_minus)`,
+    /// or paths for ranges.
+    //
+    // FIXME: do we want to allow `expr -> pattern` conversion to create path expressions?
+    // That means making this work:
+    //
+    // ```rust,ignore (FIXME)
+    // struct S;
+    // macro_rules! m {
+    //     ($a:expr) => {
+    //         let $a = S;
+    //     }
+    // }
+    // m!(S);
+    // ```
+    fn lower_expr_within_pat(&mut self, expr: &Expr, allow_paths: bool) -> &'hir hir::Expr<'hir> {
+        match expr.kind {
+            ExprKind::Lit(..) | ExprKind::ConstBlock(..) | ExprKind::Err => {}
+            ExprKind::Path(..) if allow_paths => {}
+            ExprKind::Unary(UnOp::Neg, ref inner) if matches!(inner.kind, ExprKind::Lit(_)) => {}
+            _ => {
+                self.diagnostic()
+                    .span_err(expr.span, "arbitrary expressions aren't allowed in patterns");
+                return self.arena.alloc(self.expr_err(expr.span));
+            }
+        }
+        self.lower_expr(expr)
+    }
 }

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -302,34 +302,6 @@ impl<'a> AstValidator<'a> {
         }
     }
 
-    /// Matches `'-' lit | lit (cf. parser::Parser::parse_literal_maybe_minus)`,
-    /// or paths for ranges.
-    //
-    // FIXME: do we want to allow `expr -> pattern` conversion to create path expressions?
-    // That means making this work:
-    //
-    // ```rust,ignore (FIXME)
-    // struct S;
-    // macro_rules! m {
-    //     ($a:expr) => {
-    //         let $a = S;
-    //     }
-    // }
-    // m!(S);
-    // ```
-    fn check_expr_within_pat(&self, expr: &Expr, allow_paths: bool) {
-        match expr.kind {
-            ExprKind::Lit(..) | ExprKind::ConstBlock(..) | ExprKind::Err => {}
-            ExprKind::Path(..) if allow_paths => {}
-            ExprKind::Unary(UnOp::Neg, ref inner) if matches!(inner.kind, ExprKind::Lit(_)) => {}
-            _ => self.err_handler().span_err(
-                expr.span,
-                "arbitrary expressions aren't allowed \
-                                                         in patterns",
-            ),
-        }
-    }
-
     fn check_late_bound_lifetime_defs(&self, params: &[GenericParam]) {
         // Check only lifetime parameters are present and that the lifetime
         // parameters that are present have no bounds.
@@ -1426,25 +1398,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         visit::walk_param_bound(self, bound)
     }
 
-    fn visit_pat(&mut self, pat: &'a Pat) {
-        match &pat.kind {
-            PatKind::Lit(expr) => {
-                self.check_expr_within_pat(expr, false);
-            }
-            PatKind::Range(start, end, _) => {
-                if let Some(expr) = start {
-                    self.check_expr_within_pat(expr, true);
-                }
-                if let Some(expr) = end {
-                    self.check_expr_within_pat(expr, true);
-                }
-            }
-            _ => {}
-        }
-
-        visit::walk_pat(self, pat)
-    }
-
     fn visit_poly_trait_ref(&mut self, t: &'a PolyTraitRef, m: &'a TraitBoundModifier) {
         self.check_late_bound_lifetime_defs(&t.bound_generic_params);
         visit::walk_poly_trait_ref(self, t, m);

src/test/ui/match/expr_before_ident_pat.stderr

@@ -1,14 +1,14 @@
-error: arbitrary expressions aren't allowed in patterns
+error[E0425]: cannot find value `a` in this scope
   --> $DIR/expr_before_ident_pat.rs:12:12
    |
 LL |     funny!(a, a);
-   |            ^
+   |            ^ not found in this scope
 
-error[E0425]: cannot find value `a` in this scope
+error: arbitrary expressions aren't allowed in patterns
   --> $DIR/expr_before_ident_pat.rs:12:12
    |
 LL |     funny!(a, a);
-   |            ^ not found in this scope
+   |            ^
 
 error: aborting due to 2 previous errors
 

src/test/ui/pattern/issue-92074-macro-ice.rs

@@ -0,0 +1,36 @@
+pub enum En {
+    A(Vec<u8>)
+}
+
+fn get_usize() -> usize {
+    0
+}
+
+macro_rules! force_expr {
+    ($e:expr) => { $e }
+}
+
+macro_rules! force_pat {
+    ($a:expr, $b:expr) => { $a..=$b }
+}
+
+macro_rules! make_vec {
+    () => { force_expr!(Vec::new()) } //~ ERROR arbitrary expressions aren't allowed
+}
+
+macro_rules! make_pat {
+    () => { force_pat!(get_usize(), get_usize()) }
+    //~^ ERROR arbitrary expressions aren't allowed
+    //~| ERROR arbitrary expressions aren't allowed
+}
+
+#[allow(unreachable_code)]
+fn f() -> Result<(), impl core::fmt::Debug> {
+    let x: En = loop {};
+
+    assert!(matches!(x, En::A(make_vec!())));
+    assert!(matches!(5, make_pat!()));
+    Ok::<(), &'static str>(())
+}
+
+fn main() {}

src/test/ui/pattern/issue-92074-macro-ice.stderr

@@ -0,0 +1,35 @@
+error: arbitrary expressions aren't allowed in patterns
+  --> $DIR/issue-92074-macro-ice.rs:18:25
+   |
+LL |     () => { force_expr!(Vec::new()) }
+   |                         ^^^^^^^^^^
+...
+LL |     assert!(matches!(x, En::A(make_vec!())));
+   |                               ----------- in this macro invocation
+   |
+   = note: this error originates in the macro `make_vec` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: arbitrary expressions aren't allowed in patterns
+  --> $DIR/issue-92074-macro-ice.rs:22:24
+   |
+LL |     () => { force_pat!(get_usize(), get_usize()) }
+   |                        ^^^^^^^^^^^
+...
+LL |     assert!(matches!(5, make_pat!()));
+   |                         ----------- in this macro invocation
+   |
+   = note: this error originates in the macro `make_pat` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: arbitrary expressions aren't allowed in patterns
+  --> $DIR/issue-92074-macro-ice.rs:22:37
+   |
+LL |     () => { force_pat!(get_usize(), get_usize()) }
+   |                                     ^^^^^^^^^^^
+...
+LL |     assert!(matches!(5, make_pat!()));
+   |                         ----------- in this macro invocation
+   |
+   = note: this error originates in the macro `make_pat` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 3 previous errors
+