Auto merge of #16533 - Nadrieril:update-pat-ana, r=HKalbasi

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 https://github.com/rust-lang/rust-analyzer/issues/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?
2024-02-12 07:36:51 +00:00 · 2024-02-12 07:36:51 +00:00 · b7972e5102
commit b7972e5102
parent c06ca6cff5 43caf8323a
5 changed files with 52 additions and 43 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@ -1470,12 +1470,12 @@ dependencies = [

 [[package]]
 name = "ra-ap-rustc_index"
-version = "0.36.0"
+version = "0.37.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f8a41dee58608b1fc93779ea365edaa70ac9927e3335ae914b675be0fa063cd7"
+checksum = "df5a0ba0d08af366cf235dbe8eb7226cced7a4fe502c98aa434ccf416defd746"
 dependencies = [
 "arrayvec",
- "ra-ap-rustc_index_macros 0.36.0",
+ "ra-ap-rustc_index_macros 0.37.0",
 "smallvec",
 ]

@ -1493,9 +1493,9 @@ dependencies = [

 [[package]]
 name = "ra-ap-rustc_index_macros"
-version = "0.36.0"
+version = "0.37.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "fbfe98def54c4337a2f7d8233850bd5d5349972b185fe8a0db2b979164b30ed8"
+checksum = "1971ebf9a701e0e68387c264a32517dcb4861ad3a4862f2e2803c1121ade20d5"
 dependencies = [
 "proc-macro2",
 "quote",
@ -1525,11 +1525,11 @@ dependencies = [

 [[package]]
 name = "ra-ap-rustc_pattern_analysis"
-version = "0.36.0"
+version = "0.37.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b5529bffec7530b4a3425640bfdfd9b95d87c4c620f740266c0de6572561aab4"
+checksum = "2c3c0e7ca9c5bdc66e3b590688e237a22ac47a48e4eac7f46b05b2abbfaf0abd"
 dependencies = [
- "ra-ap-rustc_index 0.36.0",
+ "ra-ap-rustc_index 0.37.0",
 "rustc-hash",
 "rustc_apfloat",
 "smallvec",
--- a/Cargo.toml
+++ b/Cargo.toml
@ -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" }
--- a/crates/hir-ty/src/diagnostics/expr.rs
+++ b/crates/hir-ty/src/diagnostics/expr.rs
@ -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 {
@ -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: (),
                    };
@ -223,7 +224,7 @@ impl ExprValidator {
            ValidityConstraint::ValidOnly,
        ) {
            Ok(report) => report,
-            Err(void) => match void {},
+            Err(()) => return,
        };

        // FIXME Report unreachable arms
@ -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;
        }
--- a/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs
+++ b/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs
@ -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;
@ -11,7 +12,6 @@ use rustc_pattern_analysis::{
 };
 use smallvec::{smallvec, SmallVec};
 use stdx::never;
-use typed_arena::Arena;

 use crate::{
    db::HirDatabase,
@ -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>>;

@ -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,
 }
@ -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 {
@ -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) {
@ -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)
@ -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
@ -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 };
@ -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;
@ -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));
@ -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)
    }
 }

--- a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs
+++ b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs
@ -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);