Unnamed repository; edit this file 'description' to name the repository.
Merge #10213
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 https://github.com/rust-analyzer/rust-analyzer/pull/9105#discussion_r644656085
Co-authored-by: Dawer <[email protected]>
| -rw-r--r-- | crates/hir_ty/src/diagnostics/expr.rs | 20 | ||||
| -rw-r--r-- | crates/hir_ty/src/diagnostics/match_check/deconstruct_pat.rs | 44 | ||||
| -rw-r--r-- | crates/hir_ty/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 899a8b793d..abcb84401b 100644 --- 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 index ecaa3daeaf..2fa456a035 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 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 index b5d238116f..bb072ae70a 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<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| { |