Bail out of MIR construction if check_match fails

clubby789 · clubby789 · commit f967076ee31d · 2023-04-30T18:16:52.000+01:00
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
@@ -1022,7 +1022,7 @@ rustc_queries! {
         desc { "converting literal to mir constant" }
     }
 
-    query check_match(key: LocalDefId) {
+    query check_match(key: LocalDefId) -> Result<(), rustc_errors::ErrorGuaranteed> {
         desc { |tcx| "match-checking `{}`", tcx.def_path_str(key) }
         cache_on_disk_if { true }
     }
diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs
@@ -42,7 +42,9 @@ fn mir_build(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> {
     // Ensure unsafeck and abstract const building is ran before we steal the THIR.
     tcx.ensure_with_value().thir_check_unsafety(def);
     tcx.ensure_with_value().thir_abstract_const(def);
-    tcx.ensure_with_value().check_match(def);
+    if let Err(e) = tcx.check_match(def) {
+        return construct_error(tcx, def, e);
+    }
 
     let body = match tcx.thir_body(def) {
         Err(error_reported) => construct_error(tcx, def, error_reported),
diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
@@ -26,8 +26,11 @@ use rustc_session::Session;
 use rustc_span::hygiene::DesugaringKind;
 use rustc_span::Span;
 
-pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) {
-    let Ok((thir, expr)) = tcx.thir_body(def_id) else { return };
+pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), ErrorGuaranteed> {
+    let (thir, expr) = match tcx.thir_body(def_id) {
+        Ok((thir, expr)) => (thir, expr),
+        Err(e) => return Err(e),
+    };
     let thir = thir.borrow();
     let pattern_arena = TypedArena::default();
     let mut visitor = MatchVisitor {
@@ -37,13 +40,18 @@ pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) {
         lint_level: tcx.hir().local_def_id_to_hir_id(def_id),
         let_source: LetSource::None,
         pattern_arena: &pattern_arena,
+        errors: vec![],
     };
     visitor.visit_expr(&thir[expr]);
+    if let Some(e) = visitor.errors.first() {
+        return Err(*e);
+    }
     for param in thir.params.iter() {
         if let Some(box ref pattern) = param.pat {
-            visitor.check_irrefutable(pattern, "function argument", None);
+            visitor.check_irrefutable(pattern, "function argument", None)?;
         }
     }
+    Ok(())
 }
 
 fn create_e0004(
@@ -77,6 +85,7 @@ struct MatchVisitor<'a, 'p, 'tcx> {
     lint_level: HirId,
     let_source: LetSource,
     pattern_arena: &'p TypedArena<DeconstructedPat<'p, 'tcx>>,
+    errors: Vec<ErrorGuaranteed>,
 }
 
 impl<'a, 'tcx> Visitor<'a, 'tcx> for MatchVisitor<'a, '_, 'tcx> {
@@ -139,7 +148,9 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for MatchVisitor<'a, '_, 'tcx> {
                     Some(DesugaringKind::Await) => hir::MatchSource::AwaitDesugar,
                     _ => hir::MatchSource::Normal,
                 };
-                self.check_match(scrutinee, arms, source, ex.span);
+                if let Err(e) = self.check_match(scrutinee, arms, source, ex.span) {
+                    self.errors.push(e)
+                }
             }
             ExprKind::Let { box ref pat, expr } => {
                 self.check_let(pat, expr, self.let_source, ex.span);
@@ -166,8 +177,9 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for MatchVisitor<'a, '_, 'tcx> {
                     self.check_let(pattern, initializer, LetSource::LetElse, span);
                 }
 
-                if else_block.is_none() {
-                    self.check_irrefutable(pattern, "local binding", Some(span));
+                if else_block.is_none()
+                    && let Err(e) = self.check_irrefutable(pattern, "local binding", Some(span)) {
+                        self.errors.push(e);
                 }
             }
             _ => {}
@@ -226,7 +238,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
         arms: &[ArmId],
         source: hir::MatchSource,
         expr_span: Span,
-    ) {
+    ) -> Result<(), ErrorGuaranteed> {
         let mut cx = self.new_cx(self.lint_level, true);
 
         for &arm in arms {
@@ -274,13 +286,14 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
                 debug_assert_eq!(pat.span.desugaring_kind(), Some(DesugaringKind::ForLoop));
                 let PatKind::Variant { ref subpatterns, .. } = pat.kind else { bug!() };
                 let [pat_field] = &subpatterns[..] else { bug!() };
-                self.check_irrefutable(&pat_field.pattern, "`for` loop binding", None);
+                self.check_irrefutable(&pat_field.pattern, "`for` loop binding", None)?;
             } else {
-                non_exhaustive_match(
+                return Err(non_exhaustive_match(
                     &cx, self.thir, scrut_ty, scrut.span, witnesses, arms, expr_span,
-                );
+                ));
             }
         }
+        Ok(())
     }
 
     fn check_let_reachability(
@@ -409,7 +422,12 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
     }
 
     #[instrument(level = "trace", skip(self))]
-    fn check_irrefutable(&self, pat: &Pat<'tcx>, origin: &str, sp: Option<Span>) {
+    fn check_irrefutable(
+        &self,
+        pat: &Pat<'tcx>,
+        origin: &str,
+        sp: Option<Span>,
+    ) -> Result<(), ErrorGuaranteed> {
         let mut cx = self.new_cx(self.lint_level, false);
 
         let pattern = self.lower_pattern(&mut cx, pat);
@@ -423,7 +441,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
         if witnesses.is_empty() {
             // The pattern is irrefutable.
             self.check_patterns(pat, Irrefutable);
-            return;
+            return Ok(());
         }
 
         let inform = sp.is_some().then_some(Inform);
@@ -478,7 +496,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
             AdtDefinedHere { adt_def_span, ty, variants }
         };
 
-        self.tcx.sess.emit_err(PatternNotCovered {
+        Err(self.tcx.sess.emit_err(PatternNotCovered {
             span: pat.span,
             origin,
             uncovered: Uncovered::new(pat.span, &cx, witnesses),
@@ -489,7 +507,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
             let_suggestion,
             misc_suggestion,
             adt_defined_here,
-        });
+        }))
     }
 }
 
@@ -631,7 +649,7 @@ fn non_exhaustive_match<'p, 'tcx>(
     witnesses: Vec<DeconstructedPat<'p, 'tcx>>,
     arms: &[ArmId],
     expr_span: Span,
-) {
+) -> ErrorGuaranteed {
     let is_empty_match = arms.is_empty();
     let non_empty_enum = match scrut_ty.kind() {
         ty::Adt(def, _) => def.is_enum() && !def.variants().is_empty(),
@@ -643,13 +661,12 @@ fn non_exhaustive_match<'p, 'tcx>(
     let pattern;
     let patterns_len;
     if is_empty_match && !non_empty_enum {
-        cx.tcx.sess.emit_err(NonExhaustivePatternsTypeNotEmpty {
+        return cx.tcx.sess.emit_err(NonExhaustivePatternsTypeNotEmpty {
             cx,
             expr_span,
             span: sp,
             ty: scrut_ty,
         });
-        return;
     } else {
         // FIXME: migration of this diagnostic will require list support
         let joined_patterns = joined_uncovered_patterns(cx, &witnesses);
@@ -800,7 +817,7 @@ fn non_exhaustive_match<'p, 'tcx>(
     } else {
         err.help(&msg);
     }
-    err.emit();
+    err.emit()
 }
 
 pub(crate) fn joined_uncovered_patterns<'p, 'tcx>(
diff --git a/tests/ui/closures/2229_closure_analysis/bad-pattern.rs b/tests/ui/closures/2229_closure_analysis/bad-pattern.rs
@@ -0,0 +1,23 @@
+// regression test for #108683
+// edition:2021
+
+enum Refutable {
+    A,
+    B,
+}
+
+fn example(v1: u32, v2: [u32; 4], v3: Refutable) {
+    const PAT: u32 = 0;
+    let v4 = &v2[..];
+    || {
+        let 0 = v1; //~ ERROR refutable pattern in local binding
+        let (0 | 1) = v1; //~ ERROR refutable pattern in local binding
+        let 1.. = v1; //~ ERROR refutable pattern in local binding
+        let [0, 0, 0, 0] = v2; //~ ERROR refutable pattern in local binding
+        let [0] = v4; //~ ERROR refutable pattern in local binding
+        let Refutable::A = v3; //~ ERROR refutable pattern in local binding
+        let PAT = v1; //~ ERROR refutable pattern in local binding
+    };
+}
+
+fn main() {}
diff --git a/tests/ui/closures/2229_closure_analysis/bad-pattern.stderr b/tests/ui/closures/2229_closure_analysis/bad-pattern.stderr
@@ -0,0 +1,113 @@
+error[E0005]: refutable pattern in local binding
+  --> $DIR/bad-pattern.rs:13:13
+   |
+LL |         let 0 = v1;
+   |             ^ pattern `1_u32..=u32::MAX` not covered
+   |
+   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
+   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
+   = note: the matched value is of type `u32`
+help: you might want to use `if let` to ignore the variant that isn't matched
+   |
+LL |         if let 0 = v1 { todo!() };
+   |         ++            +++++++++++
+help: alternatively, you could prepend the pattern with an underscore to define a new named variable; identifiers cannot begin with digits
+   |
+LL |         let _0 = v1;
+   |             +
+
+error[E0005]: refutable pattern in local binding
+  --> $DIR/bad-pattern.rs:14:14
+   |
+LL |         let (0 | 1) = v1;
+   |              ^^^^^ pattern `2_u32..=u32::MAX` not covered
+   |
+   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
+   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
+   = note: the matched value is of type `u32`
+help: you might want to use `if let` to ignore the variant that isn't matched
+   |
+LL |         if let (0 | 1) = v1 { todo!() };
+   |         ++                  +++++++++++
+
+error[E0005]: refutable pattern in local binding
+  --> $DIR/bad-pattern.rs:15:13
+   |
+LL |         let 1.. = v1;
+   |             ^^^ pattern `0_u32` not covered
+   |
+   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
+   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
+   = note: the matched value is of type `u32`
+help: you might want to use `if let` to ignore the variant that isn't matched
+   |
+LL |         if let 1.. = v1 { todo!() };
+   |         ++              +++++++++++
+
+error[E0005]: refutable pattern in local binding
+  --> $DIR/bad-pattern.rs:16:13
+   |
+LL |         let [0, 0, 0, 0] = v2;
+   |             ^^^^^^^^^^^^ pattern `[1_u32..=u32::MAX, _, _, _]` not covered
+   |
+   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
+   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
+   = note: the matched value is of type `[u32; 4]`
+help: you might want to use `if let` to ignore the variant that isn't matched
+   |
+LL |         if let [0, 0, 0, 0] = v2 { todo!() };
+   |         ++                       +++++++++++
+
+error[E0005]: refutable pattern in local binding
+  --> $DIR/bad-pattern.rs:17:13
+   |
+LL |         let [0] = v4;
+   |             ^^^ patterns `&[]` and `&[_, _, ..]` not covered
+   |
+   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
+   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
+   = note: the matched value is of type `&[u32]`
+help: you might want to use `if let` to ignore the variants that aren't matched
+   |
+LL |         if let [0] = v4 { todo!() };
+   |         ++              +++++++++++
+
+error[E0005]: refutable pattern in local binding
+  --> $DIR/bad-pattern.rs:18:13
+   |
+LL |         let Refutable::A = v3;
+   |             ^^^^^^^^^^^^ pattern `Refutable::B` not covered
+   |
+   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
+   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
+note: `Refutable` defined here
+  --> $DIR/bad-pattern.rs:4:6
+   |
+LL | enum Refutable {
+   |      ^^^^^^^^^
+LL |     A,
+LL |     B,
+   |     - not covered
+   = note: the matched value is of type `Refutable`
+help: you might want to use `if let` to ignore the variant that isn't matched
+   |
+LL |         if let Refutable::A = v3 { todo!() };
+   |         ++                       +++++++++++
+
+error[E0005]: refutable pattern in local binding
+  --> $DIR/bad-pattern.rs:19:13
+   |
+LL |         let PAT = v1;
+   |             ^^^
+   |             |
+   |             pattern `1_u32..=u32::MAX` not covered
+   |             missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable
+   |             help: introduce a variable instead: `PAT_var`
+   |
+   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
+   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
+   = note: the matched value is of type `u32`
+
+error: aborting due to 7 previous errors
+
+For more information about this error, try `rustc --explain E0005`.

Original file line number	Diff line number	Diff line change
`@@ -1022,7 +1022,7 @@ rustc_queries! {`
`1022`	`1022`	`desc { "converting literal to mir constant" }`
`1023`	`1023`	`}`
`1024`	`1024`
`1025`		`- query check_match(key: LocalDefId) {`
	`1025`	`+ query check_match(key: LocalDefId) -> Result<(), rustc_errors::ErrorGuaranteed> {`
`1026`	`1026`	desc { \|tcx\| "match-checking `{}`", tcx.def_path_str(key) }
`1027`	`1027`	`cache_on_disk_if { true }`
`1028`	`1028`	`}`