Skip to content

Commit 71117e6

Browse files
bors[bot]iDawer
andauthored
Merge #8717
8717: Update match checking algorithm r=iDawer a=iDawer I've recently got interest in the match checking to extend the current algo to support reporting witnesses of non-exhaustiveness. It appears the algo is outdated from rustc's implementation. I decided to rewrite it based on the latest rustc's version. It is a diff-based port to ra codebase. That means you can diff-compare these files to rustc. I'm striving to keep minimal ra-related changes in the algo to make it easier to backport future changes from the upstream. Based on upstream algorithm of version rust-lang/rust 1.52.0-nightly (25c15cdbe 2021-04-22) https://github.com/rust-lang/rust/blob/25c15cdbe/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs The goal of this PR is to cover the current `missing-match-arm` diagnostic. What is remaining to do: - [x] Error handling. The errors that are unrelated to match checking will be handled before the check. Just like how it made in rustc. - [x] Lowering `hir_def::expr::Pat` to `hir_ty::diagnostics::match_check::Pat`. rustc's match checking works on top of `rustc_mir_build::thir::Pat`, which is lowered from `hir::Pat` and carries some extra semantics used by the check. All unrelated checks are done there. RA could use this to rule out running the check on unimplemented cases (`Pat::ConstBlock`, etc). - [x] ~~Proper~~Loose typecheck of match arm patterns (#8840, #8875). - [x] Tests from `hir_ty::diagnostics::match_check::tests`. - [x] Clean up `todo`s - [x] Test run on real repos #8717 (comment). Co-authored-by: Dawer <[email protected]>
2 parents 42dfdb8 + e7c4966 commit 71117e6

File tree

7 files changed

+2769
-806
lines changed

7 files changed

+2769
-806
lines changed

crates/hir_def/src/path.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ impl Path {
166166
}
167167

168168
/// Converts a known mod path to `Path`.
169-
pub(crate) fn from_known_path(
169+
pub fn from_known_path(
170170
path: ModPath,
171171
generic_args: Vec<Option<Interned<GenericArgs>>>,
172172
) -> Path {

crates/hir_ty/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ chalk-solve = { version = "0.68", default-features = false }
2222
chalk-ir = "0.68"
2323
chalk-recursive = "0.68"
2424
la-arena = { version = "0.2.0", path = "../../lib/arena" }
25+
once_cell = { version = "1.5.0" }
2526

2627
stdx = { path = "../stdx", version = "0.0.0" }
2728
hir_def = { path = "../hir_def", version = "0.0.0" }

crates/hir_ty/src/diagnostics/expr.rs

Lines changed: 86 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,22 @@
22
//! through the body using inference results: mismatched arg counts, missing
33
//! fields, etc.
44
5-
use std::sync::Arc;
5+
use std::{cell::RefCell, sync::Arc};
66

7-
use hir_def::{expr::Statement, path::path, resolver::HasResolver, AssocItemId, DefWithBodyId};
7+
use hir_def::{
8+
expr::Statement, path::path, resolver::HasResolver, AssocItemId, DefWithBodyId, HasModule,
9+
};
810
use hir_expand::name;
911
use rustc_hash::FxHashSet;
1012
use syntax::{ast, AstPtr};
1113

1214
use crate::{
1315
db::HirDatabase,
1416
diagnostics::{
15-
match_check::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness},
17+
match_check::{
18+
self,
19+
usefulness::{compute_match_usefulness, expand_pattern, MatchCheckCtx, PatternArena},
20+
},
1621
MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
1722
MissingPatFields, RemoveThisSemicolon,
1823
},
@@ -294,12 +299,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
294299
&infer.type_of_expr[match_expr]
295300
};
296301

297-
let cx = MatchCheckCtx { match_expr, body, infer: infer.clone(), db };
298-
let pats = arms.iter().map(|arm| arm.pat);
302+
let pattern_arena = RefCell::new(PatternArena::new());
299303

300-
let mut seen = Matrix::empty();
301-
for pat in pats {
302-
if let Some(pat_ty) = infer.type_of_pat.get(pat) {
304+
let mut m_arms = Vec::new();
305+
let mut has_lowering_errors = false;
306+
for arm in arms {
307+
if let Some(pat_ty) = infer.type_of_pat.get(arm.pat) {
303308
// We only include patterns whose type matches the type
304309
// of the match expression. If we had a InvalidMatchArmPattern
305310
// diagnostic or similar we could raise that in an else
@@ -315,49 +320,99 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
315320
.as_reference()
316321
.map(|(match_expr_ty, ..)| match_expr_ty == pat_ty)
317322
.unwrap_or(false))
318-
&& types_of_subpatterns_do_match(pat, &cx.body, &infer)
323+
&& types_of_subpatterns_do_match(arm.pat, &body, &infer)
319324
{
320325
// If we had a NotUsefulMatchArm diagnostic, we could
321326
// check the usefulness of each pattern as we added it
322327
// to the matrix here.
323-
let v = PatStack::from_pattern(pat);
324-
seen.push(&cx, v);
325-
continue;
328+
let m_arm = match_check::MatchArm {
329+
pat: self.lower_pattern(
330+
arm.pat,
331+
&mut pattern_arena.borrow_mut(),
332+
db,
333+
&body,
334+
&mut has_lowering_errors,
335+
),
336+
has_guard: arm.guard.is_some(),
337+
};
338+
m_arms.push(m_arm);
339+
if !has_lowering_errors {
340+
continue;
341+
}
326342
}
327343
}
328344

329345
// If we can't resolve the type of a pattern, or the pattern type doesn't
330346
// fit the match expression, we skip this diagnostic. Skipping the entire
331347
// diagnostic rather than just not including this match arm is preferred
332348
// to avoid the chance of false positives.
349+
#[cfg(test)]
350+
match_check::tests::report_bail_out(db, self.owner, arm.pat, self.sink);
333351
return;
334352
}
335353

336-
match is_useful(&cx, &seen, &PatStack::from_wild()) {
337-
Ok(Usefulness::Useful) => (),
338-
// if a wildcard pattern is not useful, then all patterns are covered
339-
Ok(Usefulness::NotUseful) => return,
340-
// this path is for unimplemented checks, so we err on the side of not
341-
// reporting any errors
342-
_ => return,
343-
}
354+
let cx = MatchCheckCtx {
355+
module: self.owner.module(db.upcast()),
356+
match_expr,
357+
infer: &infer,
358+
db,
359+
pattern_arena: &pattern_arena,
360+
eprint_panic_context: &|| {
361+
use syntax::AstNode;
362+
if let Ok(scrutinee_sptr) = source_map.expr_syntax(match_expr) {
363+
let root = scrutinee_sptr.file_syntax(db.upcast());
364+
if let Some(match_ast) = scrutinee_sptr.value.to_node(&root).syntax().parent() {
365+
eprintln!(
366+
"Match checking is about to panic on this expression:\n{}",
367+
match_ast.to_string(),
368+
);
369+
}
370+
}
371+
},
372+
};
373+
let report = compute_match_usefulness(&cx, &m_arms);
344374

345-
if let Ok(source_ptr) = source_map.expr_syntax(id) {
346-
let root = source_ptr.file_syntax(db.upcast());
347-
if let ast::Expr::MatchExpr(match_expr) = &source_ptr.value.to_node(&root) {
348-
if let (Some(match_expr), Some(arms)) =
349-
(match_expr.expr(), match_expr.match_arm_list())
350-
{
351-
self.sink.push(MissingMatchArms {
352-
file: source_ptr.file_id,
353-
match_expr: AstPtr::new(&match_expr),
354-
arms: AstPtr::new(&arms),
355-
})
375+
// FIXME Report unreacheble arms
376+
// https://github.com/rust-lang/rust/blob/25c15cdbe/compiler/rustc_mir_build/src/thir/pattern/check_match.rs#L200-L201
377+
378+
let witnesses = report.non_exhaustiveness_witnesses;
379+
// FIXME Report witnesses
380+
// eprintln!("compute_match_usefulness(..) -> {:?}", &witnesses);
381+
if !witnesses.is_empty() {
382+
if let Ok(source_ptr) = source_map.expr_syntax(id) {
383+
let root = source_ptr.file_syntax(db.upcast());
384+
if let ast::Expr::MatchExpr(match_expr) = &source_ptr.value.to_node(&root) {
385+
if let (Some(match_expr), Some(arms)) =
386+
(match_expr.expr(), match_expr.match_arm_list())
387+
{
388+
self.sink.push(MissingMatchArms {
389+
file: source_ptr.file_id,
390+
match_expr: AstPtr::new(&match_expr),
391+
arms: AstPtr::new(&arms),
392+
})
393+
}
356394
}
357395
}
358396
}
359397
}
360398

399+
fn lower_pattern(
400+
&self,
401+
pat: PatId,
402+
pattern_arena: &mut PatternArena,
403+
db: &dyn HirDatabase,
404+
body: &Body,
405+
have_errors: &mut bool,
406+
) -> match_check::PatId {
407+
let mut patcx = match_check::PatCtxt::new(db, &self.infer, body);
408+
let pattern = patcx.lower_pattern(pat);
409+
let pattern = pattern_arena.alloc(expand_pattern(pattern));
410+
if !patcx.errors.is_empty() {
411+
*have_errors = true;
412+
}
413+
pattern
414+
}
415+
361416
fn validate_results_in_tail_expr(&mut self, body_id: ExprId, id: ExprId, db: &dyn HirDatabase) {
362417
// the mismatch will be on the whole block currently
363418
let mismatch = match self.infer.type_mismatch_for_expr(body_id) {

0 commit comments

Comments
 (0)