Merge #10213

bors[bot] · iDawer · web-flow · commit 3e056b9e90eb · 2021-09-12T12:23:56.000Z
10213: minor: Improve resilience of match checking r=flodiebold a=iDawer In bug condition the match checking strives to recover giving false no-error diagnostic. Suggested in #9105 (comment) Co-authored-by: Dawer <7803845+iDawer@users.noreply.github.com>
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
@@ -22,7 +22,7 @@ use crate::{
 };
 
 pub(crate) use hir_def::{
-    body::{Body, BodySourceMap},
+    body::Body,
     expr::{Expr, ExprId, MatchArm, Pat, PatId},
     LocalFieldId, VariantId,
 };
@@ -264,8 +264,7 @@ impl ExprValidator {
         db: &dyn HirDatabase,
         infer: Arc<InferenceResult>,
     ) {
-        let (body, source_map): (Arc<Body>, Arc<BodySourceMap>) =
-            db.body_with_source_map(self.owner);
+        let body = db.body(self.owner);
 
         let match_expr_ty = if infer.type_of_expr[match_expr].is_unknown() {
             return;
@@ -330,21 +329,6 @@ impl ExprValidator {
             infer: &infer,
             db,
             pattern_arena: &pattern_arena,
-            panic_context: &|| {
-                use syntax::AstNode;
-                let match_expr_text = source_map
-                    .expr_syntax(match_expr)
-                    .ok()
-                    .and_then(|scrutinee_sptr| {
-                        let root = scrutinee_sptr.file_syntax(db.upcast());
-                        scrutinee_sptr.value.to_node(&root).syntax().parent()
-                    })
-                    .map(|node| node.to_string());
-                format!(
-                    "expression:\n{}",
-                    match_expr_text.as_deref().unwrap_or("<synthesized expr>")
-                )
-            },
         };
         let report = compute_match_usefulness(&cx, &m_arms);
 
diff --git a/crates/hir_ty/src/diagnostics/match_check/deconstruct_pat.rs b/crates/hir_ty/src/diagnostics/match_check/deconstruct_pat.rs
@@ -49,6 +49,7 @@ use std::{
 
 use hir_def::{EnumVariantId, HasModule, LocalFieldId, VariantId};
 use smallvec::{smallvec, SmallVec};
+use stdx::never;
 
 use crate::{AdtId, Interner, Scalar, Ty, TyExt, TyKind};
 
@@ -324,7 +325,10 @@ impl Constructor {
             PatKind::Leaf { .. } | PatKind::Deref { .. } => Single,
             &PatKind::Variant { enum_variant, .. } => Variant(enum_variant),
             &PatKind::LiteralBool { value } => IntRange(IntRange::from_bool(value)),
-            PatKind::Or { .. } => cx.bug("Or-pattern should have been expanded earlier on."),
+            PatKind::Or { .. } => {
+                never!("Or-pattern should have been expanded earlier on.");
+                Wildcard
+            }
         }
     }
 
@@ -371,7 +375,7 @@ impl Constructor {
     /// this checks for inclusion.
     // We inline because this has a single call site in `Matrix::specialize_constructor`.
     #[inline]
-    pub(super) fn is_covered_by(&self, pcx: PatCtxt<'_>, other: &Self) -> bool {
+    pub(super) fn is_covered_by(&self, _pcx: PatCtxt<'_>, other: &Self) -> bool {
         // This must be kept in sync with `is_covered_by_any`.
         match (self, other) {
             // Wildcards cover anything
@@ -396,17 +400,18 @@ impl Constructor {
             // Only a wildcard pattern can match the special extra constructor.
             (NonExhaustive, _) => false,
 
-            _ => pcx.cx.bug(&format!(
-                "trying to compare incompatible constructors {:?} and {:?}",
-                self, other
-            )),
+            _ => {
+                never!("trying to compare incompatible constructors {:?} and {:?}", self, other);
+                // Continue with 'whatever is covered' supposed to result in false no-error diagnostic.
+                true
+            }
         }
     }
 
     /// Faster version of `is_covered_by` when applied to many constructors. `used_ctors` is
     /// assumed to be built from `matrix.head_ctors()` with wildcards filtered out, and `self` is
     /// assumed to have been split from a wildcard.
-    fn is_covered_by_any(&self, pcx: PatCtxt<'_>, used_ctors: &[Constructor]) -> bool {
+    fn is_covered_by_any(&self, _pcx: PatCtxt<'_>, used_ctors: &[Constructor]) -> bool {
         if used_ctors.is_empty() {
             return false;
         }
@@ -427,7 +432,8 @@ impl Constructor {
             // This constructor is never covered by anything else
             NonExhaustive => false,
             Str(..) | FloatRange(..) | Opaque | Missing | Wildcard => {
-                pcx.cx.bug(&format!("found unexpected ctor in all_ctors: {:?}", self))
+                never!("found unexpected ctor in all_ctors: {:?}", self);
+                true
             }
         }
     }
@@ -683,7 +689,8 @@ impl Fields {
                     }
                 }
                 ty_kind => {
-                    cx.bug(&format!("Unexpected type for `Single` constructor: {:?}", ty_kind))
+                    never!("Unexpected type for `Single` constructor: {:?}", ty_kind);
+                    Fields::from_single_pattern(wildcard_from_ty(ty))
                 }
             },
             Slice(..) => {
@@ -745,7 +752,8 @@ impl Fields {
                 // can ignore this issue.
                 TyKind::Ref(..) => PatKind::Deref { subpattern: subpatterns.next().unwrap() },
                 TyKind::Slice(..) | TyKind::Array(..) => {
-                    pcx.cx.bug(&format!("bad slice pattern {:?} {:?}", ctor, pcx.ty))
+                    never!("bad slice pattern {:?} {:?}", ctor, pcx.ty);
+                    PatKind::Wild
                 }
                 _ => PatKind::Wild,
             },
@@ -755,11 +763,17 @@ impl Fields {
             Constructor::IntRange(_) => UNHANDLED,
             NonExhaustive => PatKind::Wild,
             Wildcard => return Pat::wildcard_from_ty(pcx.ty.clone()),
-            Opaque => pcx.cx.bug("we should not try to apply an opaque constructor"),
-            Missing => pcx.cx.bug(
-                "trying to apply the `Missing` constructor;\
-                this should have been done in `apply_constructors`",
-            ),
+            Opaque => {
+                never!("we should not try to apply an opaque constructor");
+                PatKind::Wild
+            }
+            Missing => {
+                never!(
+                    "trying to apply the `Missing` constructor; \
+                    this should have been done in `apply_constructors`",
+                );
+                PatKind::Wild
+            }
         };
 
         Pat { ty: pcx.ty.clone(), kind: Box::new(pat) }
diff --git a/crates/hir_ty/src/diagnostics/match_check/usefulness.rs b/crates/hir_ty/src/diagnostics/match_check/usefulness.rs
@@ -295,7 +295,6 @@ pub(crate) struct MatchCheckCtx<'a> {
     pub(crate) db: &'a dyn HirDatabase,
     /// Lowered patterns from arms plus generated by the check.
     pub(crate) pattern_arena: &'a RefCell<PatternArena>,
-    pub(crate) panic_context: &'a dyn Fn() -> String,
 }
 
 impl<'a> MatchCheckCtx<'a> {
@@ -328,11 +327,6 @@ impl<'a> MatchCheckCtx<'a> {
     pub(super) fn type_of(&self, pat: PatId) -> Ty {
         self.pattern_arena.borrow()[pat].ty.clone()
     }
-
-    #[track_caller]
-    pub(super) fn bug(&self, info: &str) -> ! {
-        panic!("bug: {}\n{}", info, (self.panic_context)());
-    }
 }
 
 #[derive(Copy, Clone)]
@@ -1131,7 +1125,7 @@ pub(crate) fn compute_match_usefulness(
     arms: &[MatchArm],
 ) -> UsefulnessReport {
     let mut matrix = Matrix::empty();
-    let arm_usefulness: Vec<_> = arms
+    let arm_usefulness = arms
         .iter()
         .copied()
         .map(|arm| {
