Skip to content

Commit

Permalink
Auto merge of #16533 - Nadrieril:update-pat-ana, r=HKalbasi
Browse files Browse the repository at this point in the history
Update to latest `rustc_pattern_analysis`

Here I go again. Two improvements this time.

1. I've removed the need for an arena for patterns. Turns out this wasn't gaining any performance on the rustc side so may as well allocate normally.
2. I've added a clean error path when types don't match, so rustc_pattern_analysis should never panic except internal logic errors. For now `cx.bug()` calls `never!()` but I'm not sure what `never!()` does. Does it display anything to the user? Otherwise a `debug!()` should be sufficient.

Point 2 should fix #15883 but I haven't tested it because I'm not sure how to reproduce. Could someone give me pointers as to how to write a test for the pattern code?
  • Loading branch information
bors committed Feb 12, 2024
2 parents c06ca6c + 43caf83 commit b7972e5
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 43 deletions.
16 changes: 8 additions & 8 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ ra-ap-rustc_lexer = { version = "0.35.0", default-features = false }
ra-ap-rustc_parse_format = { version = "0.35.0", default-features = false }
ra-ap-rustc_index = { version = "0.35.0", default-features = false }
ra-ap-rustc_abi = { version = "0.35.0", default-features = false }
ra-ap-rustc_pattern_analysis = { version = "0.36.0", default-features = false }
ra-ap-rustc_pattern_analysis = { version = "0.37.0", default-features = false }

# local crates that aren't published to crates.io. These should not have versions.
sourcegen = { path = "./crates/sourcegen" }
Expand Down
13 changes: 7 additions & 6 deletions crates/hir-ty/src/diagnostics/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ impl ExprValidator {
return;
}

let pattern_arena = Arena::new();
let cx = MatchCheckCtx::new(self.owner.module(db.upcast()), self.owner, db, &pattern_arena);
let cx = MatchCheckCtx::new(self.owner.module(db.upcast()), self.owner, db);

let pattern_arena = Arena::new();
let mut m_arms = Vec::with_capacity(arms.len());
let mut has_lowering_errors = false;
for arm in arms {
Expand All @@ -196,8 +196,9 @@ impl ExprValidator {
// If we had a NotUsefulMatchArm diagnostic, we could
// check the usefulness of each pattern as we added it
// to the matrix here.
let pat = self.lower_pattern(&cx, arm.pat, db, &body, &mut has_lowering_errors);
let m_arm = pat_analysis::MatchArm {
pat: self.lower_pattern(&cx, arm.pat, db, &body, &mut has_lowering_errors),
pat: pattern_arena.alloc(pat),
has_guard: arm.guard.is_some(),
arm_data: (),
};
Expand All @@ -223,7 +224,7 @@ impl ExprValidator {
ValidityConstraint::ValidOnly,
) {
Ok(report) => report,
Err(void) => match void {},
Err(()) => return,
};

// FIXME Report unreachable arms
Expand All @@ -245,10 +246,10 @@ impl ExprValidator {
db: &dyn HirDatabase,
body: &Body,
have_errors: &mut bool,
) -> &'p DeconstructedPat<'p> {
) -> DeconstructedPat<'p> {
let mut patcx = match_check::PatCtxt::new(db, &self.infer, body);
let pattern = patcx.lower_pattern(pat);
let pattern = cx.pattern_arena.alloc(cx.lower_pat(&pattern));
let pattern = cx.lower_pat(&pattern);
if !patcx.errors.is_empty() {
*have_errors = true;
}
Expand Down
46 changes: 18 additions & 28 deletions crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Interface with `rustc_pattern_analysis`.
use std::fmt;
use tracing::debug;

use hir_def::{DefWithBodyId, EnumVariantId, HasModule, LocalFieldId, ModuleId, VariantId};
use rustc_hash::FxHashMap;
Expand All @@ -11,7 +12,6 @@ use rustc_pattern_analysis::{
};
use smallvec::{smallvec, SmallVec};
use stdx::never;
use typed_arena::Arena;

use crate::{
db::HirDatabase,
Expand All @@ -26,7 +26,7 @@ use Constructor::*;

// Re-export r-a-specific versions of all these types.
pub(crate) type DeconstructedPat<'p> =
rustc_pattern_analysis::pat::DeconstructedPat<'p, MatchCheckCtx<'p>>;
rustc_pattern_analysis::pat::DeconstructedPat<MatchCheckCtx<'p>>;
pub(crate) type MatchArm<'p> = rustc_pattern_analysis::MatchArm<'p, MatchCheckCtx<'p>>;
pub(crate) type WitnessPat<'p> = rustc_pattern_analysis::pat::WitnessPat<MatchCheckCtx<'p>>;

Expand All @@ -40,7 +40,6 @@ pub(crate) struct MatchCheckCtx<'p> {
module: ModuleId,
body: DefWithBodyId,
pub(crate) db: &'p dyn HirDatabase,
pub(crate) pattern_arena: &'p Arena<DeconstructedPat<'p>>,
exhaustive_patterns: bool,
min_exhaustive_patterns: bool,
}
Expand All @@ -52,17 +51,12 @@ pub(crate) struct PatData<'p> {
}

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

fn is_uninhabited(&self, ty: &Ty) -> bool {
Expand Down Expand Up @@ -131,15 +125,15 @@ impl<'p> MatchCheckCtx<'p> {
}

pub(crate) fn lower_pat(&self, pat: &Pat) -> DeconstructedPat<'p> {
let singleton = |pat| std::slice::from_ref(self.pattern_arena.alloc(pat));
let singleton = |pat| vec![pat];
let ctor;
let fields: &[_];
let fields: Vec<_>;

match pat.kind.as_ref() {
PatKind::Binding { subpattern: Some(subpat), .. } => return self.lower_pat(subpat),
PatKind::Binding { subpattern: None, .. } | PatKind::Wild => {
ctor = Wildcard;
fields = &[];
fields = Vec::new();
}
PatKind::Deref { subpattern } => {
ctor = match pat.ty.kind(Interner) {
Expand All @@ -157,7 +151,7 @@ impl<'p> MatchCheckCtx<'p> {
match pat.ty.kind(Interner) {
TyKind::Tuple(_, substs) => {
ctor = Struct;
let mut wilds: SmallVec<[_; 2]> = substs
let mut wilds: Vec<_> = substs
.iter(Interner)
.map(|arg| arg.assert_ty_ref(Interner).clone())
.map(DeconstructedPat::wildcard)
Expand All @@ -166,7 +160,7 @@ impl<'p> MatchCheckCtx<'p> {
let idx: u32 = pat.field.into_raw().into();
wilds[idx as usize] = self.lower_pat(&pat.pattern);
}
fields = self.pattern_arena.alloc_extend(wilds)
fields = wilds
}
TyKind::Adt(adt, substs) if is_box(self.db, adt.0) => {
// The only legal patterns of type `Box` (outside `std`) are `_` and box
Expand Down Expand Up @@ -216,33 +210,29 @@ impl<'p> MatchCheckCtx<'p> {
field_id_to_id[field_idx as usize] = Some(i);
ty
});
let mut wilds: SmallVec<[_; 2]> =
tys.map(DeconstructedPat::wildcard).collect();
let mut wilds: Vec<_> = tys.map(DeconstructedPat::wildcard).collect();
for pat in subpatterns {
let field_idx: u32 = pat.field.into_raw().into();
if let Some(i) = field_id_to_id[field_idx as usize] {
wilds[i] = self.lower_pat(&pat.pattern);
}
}
fields = self.pattern_arena.alloc_extend(wilds);
fields = wilds;
}
_ => {
never!("pattern has unexpected type: pat: {:?}, ty: {:?}", pat, &pat.ty);
ctor = Wildcard;
fields = &[];
fields = Vec::new();
}
}
}
&PatKind::LiteralBool { value } => {
ctor = Bool(value);
fields = &[];
fields = Vec::new();
}
PatKind::Or { pats } => {
ctor = Or;
// Collect here because `Arena::alloc_extend` panics on reentrancy.
let subpats: SmallVec<[_; 2]> =
pats.iter().map(|pat| self.lower_pat(pat)).collect();
fields = self.pattern_arena.alloc_extend(subpats);
fields = pats.iter().map(|pat| self.lower_pat(pat)).collect();
}
}
let data = PatData { db: self.db };
Expand Down Expand Up @@ -307,7 +297,7 @@ impl<'p> MatchCheckCtx<'p> {
}

impl<'p> TypeCx for MatchCheckCtx<'p> {
type Error = Void;
type Error = ();
type Ty = Ty;
type VariantIdx = EnumVariantId;
type StrLit = Void;
Expand Down Expand Up @@ -463,7 +453,7 @@ impl<'p> TypeCx for MatchCheckCtx<'p> {

fn write_variant_name(
f: &mut fmt::Formatter<'_>,
pat: &rustc_pattern_analysis::pat::DeconstructedPat<'_, Self>,
pat: &rustc_pattern_analysis::pat::DeconstructedPat<Self>,
) -> fmt::Result {
let variant =
pat.ty().as_adt().and_then(|(adt, _)| Self::variant_id_for_adt(pat.ctor(), adt));
Expand All @@ -485,8 +475,8 @@ impl<'p> TypeCx for MatchCheckCtx<'p> {
Ok(())
}

fn bug(&self, fmt: fmt::Arguments<'_>) -> ! {
panic!("{}", fmt)
fn bug(&self, fmt: fmt::Arguments<'_>) {
debug!("{}", fmt)
}
}

Expand Down
18 changes: 18 additions & 0 deletions crates/ide-diagnostics/src/handlers/missing_match_arms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,24 @@ fn main() {
);
}

#[test]
fn mismatched_types_issue_15883() {
// Check we don't panic.
check_diagnostics_no_bails(
r#"
//- minicore: option
fn main() {
match Some((true, false)) {
Some(true) | Some(false) => {}
// ^^^^ error: expected (bool, bool), found bool
// ^^^^^ error: expected (bool, bool), found bool
None => {}
}
}
"#,
);
}

#[test]
fn mismatched_types_in_or_patterns() {
cov_mark::check_count!(validate_match_bailed_out, 2);
Expand Down

0 comments on commit b7972e5

Please sign in to comment.