Skip to content

Commit

Permalink
Auto merge of #17853 - ShoyuVanilla:min-exhaustive-pat, r=ShoyuVanilla
Browse files Browse the repository at this point in the history
feat: `min-exhaustive-patterns`

Resolves #17851
  • Loading branch information
bors committed Aug 13, 2024
2 parents f3d9c9d + 588fa2c commit e25abba
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 46 deletions.
25 changes: 12 additions & 13 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 10 additions & 11 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ tt = { path = "./crates/tt", version = "0.0.0" }
vfs-notify = { path = "./crates/vfs-notify", version = "0.0.0" }
vfs = { path = "./crates/vfs", version = "0.0.0" }

ra-ap-rustc_lexer = { version = "0.53.0", default-features = false }
ra-ap-rustc_parse_format = { version = "0.53.0", default-features = false }
ra-ap-rustc_index = { version = "0.53.0", default-features = false }
ra-ap-rustc_abi = { version = "0.53.0", default-features = false }
ra-ap-rustc_pattern_analysis = { version = "0.53.0", default-features = false }
ra-ap-rustc_lexer = { version = "0.63.0", default-features = false }
ra-ap-rustc_parse_format = { version = "0.63.0", default-features = false }
ra-ap-rustc_index = { version = "0.63.0", default-features = false }
ra-ap-rustc_abi = { version = "0.63.0", default-features = false }
ra-ap-rustc_pattern_analysis = { version = "0.63.0", default-features = false }

# local crates that aren't published to crates.io. These should not have versions.
test-fixture = { path = "./crates/test-fixture" }
Expand Down Expand Up @@ -125,11 +125,11 @@ memmap2 = "0.5.4"
nohash-hasher = "0.2.0"
oorandom = "11.1.3"
object = { version = "0.33.0", default-features = false, features = [
"std",
"read_core",
"elf",
"macho",
"pe",
"std",
"read_core",
"elf",
"macho",
"pe",
] }
process-wrap = { version = "8.0.2", features = ["std"] }
pulldown-cmark-to-cmark = "10.0.4"
Expand Down Expand Up @@ -159,7 +159,6 @@ url = "2.3.1"
xshell = "0.2.5"



# We need to freeze the version of the crate, as the raw-api feature is considered unstable
dashmap = { version = "=5.5.3", features = ["raw-api"] }

Expand Down
65 changes: 57 additions & 8 deletions crates/hir-ty/src/diagnostics/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,33 @@

use std::fmt;

use chalk_solve::rust_ir::AdtKind;
use either::Either;
use hir_def::lang_item::LangItem;
use hir_def::{resolver::HasResolver, AdtId, AssocItemId, DefWithBodyId, HasModule};
use hir_def::{ItemContainerId, Lookup};
use hir_def::{
lang_item::LangItem,
resolver::{HasResolver, ValueNs},
AdtId, AssocItemId, DefWithBodyId, HasModule, ItemContainerId, Lookup,
};
use intern::sym;
use itertools::Itertools;
use rustc_hash::FxHashSet;
use rustc_pattern_analysis::constructor::Constructor;
use syntax::{ast, AstNode};
use syntax::{
ast::{self, UnaryOp},
AstNode,
};
use tracing::debug;
use triomphe::Arc;
use typed_arena::Arena;

use crate::Interner;
use crate::{
db::HirDatabase,
diagnostics::match_check::{
self,
pat_analysis::{self, DeconstructedPat, MatchCheckCtx, WitnessPat},
},
display::HirDisplay,
InferenceResult, Ty, TyExt,
Adjust, InferenceResult, Interner, Ty, TyExt, TyKind,
};

pub(crate) use hir_def::{
Expand Down Expand Up @@ -236,7 +241,12 @@ impl ExprValidator {
return;
}

let report = match cx.compute_match_usefulness(m_arms.as_slice(), scrut_ty.clone()) {
let known_valid_scrutinee = Some(self.is_known_valid_scrutinee(scrutinee_expr, db));
let report = match cx.compute_match_usefulness(
m_arms.as_slice(),
scrut_ty.clone(),
known_valid_scrutinee,
) {
Ok(report) => report,
Err(()) => return,
};
Expand All @@ -253,6 +263,45 @@ impl ExprValidator {
}
}

// [rustc's `is_known_valid_scrutinee`](https://github.com/rust-lang/rust/blob/c9bd03cb724e13cca96ad320733046cbdb16fbbe/compiler/rustc_mir_build/src/thir/pattern/check_match.rs#L288)
//
// While the above function in rustc uses thir exprs, r-a doesn't have them.
// So, the logic here is getting same result as "hir lowering + match with lowered thir"
// with "hir only"
fn is_known_valid_scrutinee(&self, scrutinee_expr: ExprId, db: &dyn HirDatabase) -> bool {
if self
.infer
.expr_adjustments
.get(&scrutinee_expr)
.is_some_and(|adjusts| adjusts.iter().any(|a| matches!(a.kind, Adjust::Deref(..))))
{
return false;
}

match &self.body[scrutinee_expr] {
Expr::UnaryOp { op: UnaryOp::Deref, .. } => false,
Expr::Path(path) => {
let value_or_partial = self
.owner
.resolver(db.upcast())
.resolve_path_in_value_ns_fully(db.upcast(), path);
value_or_partial.map_or(true, |v| !matches!(v, ValueNs::StaticId(_)))
}
Expr::Field { expr, .. } => match self.infer.type_of_expr[*expr].kind(Interner) {
TyKind::Adt(adt, ..)
if db.adt_datum(self.owner.krate(db.upcast()), *adt).kind == AdtKind::Union =>
{
false
}
_ => self.is_known_valid_scrutinee(*expr, db),
},
Expr::Index { base, .. } => self.is_known_valid_scrutinee(*base, db),
Expr::Cast { expr, .. } => self.is_known_valid_scrutinee(*expr, db),
Expr::Missing => false,
_ => true,
}
}

fn validate_block(&mut self, db: &dyn HirDatabase, expr: &Expr) {
let (Expr::Block { statements, .. }
| Expr::Async { statements, .. }
Expand Down Expand Up @@ -285,7 +334,7 @@ impl ExprValidator {
has_guard: false,
arm_data: (),
};
let report = match cx.compute_match_usefulness(&[match_arm], ty.clone()) {
let report = match cx.compute_match_usefulness(&[match_arm], ty.clone(), None) {
Ok(v) => v,
Err(e) => {
debug!(?e, "match usefulness error");
Expand Down
25 changes: 11 additions & 14 deletions crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,22 +69,20 @@ pub(crate) struct MatchCheckCtx<'db> {
body: DefWithBodyId,
pub(crate) db: &'db dyn HirDatabase,
exhaustive_patterns: bool,
min_exhaustive_patterns: bool,
}

impl<'db> MatchCheckCtx<'db> {
pub(crate) fn new(module: ModuleId, body: DefWithBodyId, db: &'db dyn HirDatabase) -> Self {
let def_map = db.crate_def_map(module.krate());
let exhaustive_patterns = def_map.is_unstable_feature_enabled(&sym::exhaustive_patterns);
let min_exhaustive_patterns =
def_map.is_unstable_feature_enabled(&sym::min_exhaustive_patterns);
Self { module, body, db, exhaustive_patterns, min_exhaustive_patterns }
Self { module, body, db, exhaustive_patterns }
}

pub(crate) fn compute_match_usefulness(
&self,
arms: &[MatchArm<'db>],
scrut_ty: Ty,
known_valid_scrutinee: Option<bool>,
) -> Result<UsefulnessReport<'db, Self>, ()> {
if scrut_ty.contains_unknown() {
return Err(());
Expand All @@ -95,8 +93,7 @@ impl<'db> MatchCheckCtx<'db> {
}
}

// FIXME: Determine place validity correctly. For now, err on the safe side.
let place_validity = PlaceValidity::MaybeInvalid;
let place_validity = PlaceValidity::from_bool(known_valid_scrutinee.unwrap_or(true));
// Measured to take ~100ms on modern hardware.
let complexity_limit = Some(500000);
compute_match_usefulness(self, arms, scrut_ty, place_validity, complexity_limit)
Expand Down Expand Up @@ -307,7 +304,8 @@ impl<'db> MatchCheckCtx<'db> {
&Str(void) => match void {},
Wildcard | NonExhaustive | Hidden | PrivateUninhabited => PatKind::Wild,
Never => PatKind::Never,
Missing | F32Range(..) | F64Range(..) | Opaque(..) | Or => {
Missing | F16Range(..) | F32Range(..) | F64Range(..) | F128Range(..) | Opaque(..)
| Or => {
never!("can't convert to pattern: {:?}", pat.ctor());
PatKind::Wild
}
Expand All @@ -327,9 +325,6 @@ impl<'db> PatCx for MatchCheckCtx<'db> {
fn is_exhaustive_patterns_feature_on(&self) -> bool {
self.exhaustive_patterns
}
fn is_min_exhaustive_patterns_feature_on(&self) -> bool {
self.min_exhaustive_patterns
}

fn ctor_arity(
&self,
Expand All @@ -356,8 +351,9 @@ impl<'db> PatCx for MatchCheckCtx<'db> {
},
Ref => 1,
Slice(..) => unimplemented!(),
Never | Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..)
| Opaque(..) | NonExhaustive | PrivateUninhabited | Hidden | Missing | Wildcard => 0,
Never | Bool(..) | IntRange(..) | F16Range(..) | F32Range(..) | F64Range(..)
| F128Range(..) | Str(..) | Opaque(..) | NonExhaustive | PrivateUninhabited
| Hidden | Missing | Wildcard => 0,
Or => {
never!("The `Or` constructor doesn't have a fixed arity");
0
Expand Down Expand Up @@ -419,8 +415,9 @@ impl<'db> PatCx for MatchCheckCtx<'db> {
}
},
Slice(_) => unreachable!("Found a `Slice` constructor in match checking"),
Never | Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..)
| Opaque(..) | NonExhaustive | PrivateUninhabited | Hidden | Missing | Wildcard => {
Never | Bool(..) | IntRange(..) | F16Range(..) | F32Range(..) | F64Range(..)
| F128Range(..) | Str(..) | Opaque(..) | NonExhaustive | PrivateUninhabited
| Hidden | Missing | Wildcard => {
smallvec![]
}
Or => {
Expand Down
38 changes: 38 additions & 0 deletions crates/ide-diagnostics/src/handlers/missing_match_arms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,44 @@ fn f() {
check_diagnostics_no_bails(&code);
}

#[test]
fn min_exhaustive() {
check_diagnostics(
r#"
//- minicore: result
fn test(x: Result<i32, !>) {
match x {
Ok(_y) => {}
}
}
"#,
);
check_diagnostics(
r#"
//- minicore: result
fn test(ptr: *const Result<i32, !>) {
unsafe {
match *ptr {
//^^^^ error: missing match arm: `Err(!)` not covered
Ok(_x) => {}
}
}
}
"#,
);
check_diagnostics(
r#"
//- minicore: result
fn test(x: Result<i32, &'static !>) {
match x {
//^ error: missing match arm: `Err(_)` not covered
Ok(_y) => {}
}
}
"#,
);
}

mod rust_unstable {
use super::*;

Expand Down
22 changes: 22 additions & 0 deletions crates/ide-diagnostics/src/handlers/non_exhaustive_let.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,28 @@ fn main() {
//^^^^ error: non-exhaustive pattern: `Some(_)` not covered
}
}
"#,
);
}

#[test]
fn min_exhaustive() {
check_diagnostics(
r#"
//- minicore: result
fn test(x: Result<i32, !>) {
let Ok(_y) = x;
}
"#,
);

check_diagnostics(
r#"
//- minicore: result
fn test(x: Result<i32, &'static !>) {
let Ok(_y) = x;
//^^^^^^ error: non-exhaustive pattern: `Err(_)` not covered
}
"#,
);
}
Expand Down

0 comments on commit e25abba

Please sign in to comment.