From 68dfe190ab993caf7c949caf33b1b18961ea6603 Mon Sep 17 00:00:00 2001 From: Dawer <7803845+iDawer@users.noreply.github.com> Date: Sun, 12 Sep 2021 16:03:12 +0500 Subject: [PATCH] Improve resilience of match checking In bug condition the match checking strives to recover giving false no-error diagnostic. --- crates/hir_ty/src/diagnostics/expr.rs | 20 +-------- .../match_check/deconstruct_pat.rs | 44 ++++++++++++------- .../src/diagnostics/match_check/usefulness.rs | 8 +--- 3 files changed, 32 insertions(+), 40 deletions(-) diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 899a8b793db..abcb84401b4 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -22,7 +22,7 @@ }; pub(crate) use hir_def::{ - body::{Body, BodySourceMap}, + body::Body, expr::{Expr, ExprId, MatchArm, Pat, PatId}, LocalFieldId, VariantId, }; @@ -264,8 +264,7 @@ fn validate_match( db: &dyn HirDatabase, infer: Arc, ) { - let (body, source_map): (Arc, Arc) = - 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 @@ fn validate_match( 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("") - ) - }, }; 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 index ecaa3daeafd..2fa456a035a 100644 --- 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 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 @@ pub(super) fn from_pat(cx: &MatchCheckCtx<'_>, pat: PatId) -> Self { 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 @@ pub(super) fn split<'a>( /// 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 @@ pub(super) fn is_covered_by(&self, pcx: PatCtxt<'_>, other: &Self) -> bool { // 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 @@ fn is_covered_by_any(&self, pcx: PatCtxt<'_>, used_ctors: &[Constructor]) -> boo // 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 @@ pub(crate) fn wildcards(pcx: PatCtxt<'_>, constructor: &Constructor) -> Self { } } 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 @@ pub(super) fn apply(self, pcx: PatCtxt<'_>, ctor: &Constructor) -> Pat { // 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 @@ pub(super) fn apply(self, pcx: PatCtxt<'_>, ctor: &Constructor) -> Pat { 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 index b5d238116ff..bb072ae70ad 100644 --- 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, - pub(crate) panic_context: &'a dyn Fn() -> String, } impl<'a> MatchCheckCtx<'a> { @@ -328,11 +327,6 @@ pub(super) fn alloc_pat(&self, pat: Pat) -> PatId { 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| {