From 3cfcd4ed962ffede6dc1405f26c34ac4b9c2462c Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Mar 2024 16:33:25 +0100 Subject: [PATCH 1/4] Abstract over the uses of `compute_match_usefulness` --- crates/hir-ty/src/diagnostics/expr.rs | 17 ++--------------- .../src/diagnostics/match_check/pat_analysis.rs | 9 +++++++++ 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/crates/hir-ty/src/diagnostics/expr.rs b/crates/hir-ty/src/diagnostics/expr.rs index b3128712dc4c..20b0da441daa 100644 --- a/crates/hir-ty/src/diagnostics/expr.rs +++ b/crates/hir-ty/src/diagnostics/expr.rs @@ -11,7 +11,6 @@ use hir_def::{ItemContainerId, Lookup}; use hir_expand::name; use itertools::Itertools; use rustc_hash::FxHashSet; -use rustc_pattern_analysis::usefulness::{compute_match_usefulness, PlaceValidity}; use syntax::{ast, AstNode}; use tracing::debug; use triomphe::Arc; @@ -234,13 +233,7 @@ impl ExprValidator { return; } - let report = match compute_match_usefulness( - &cx, - m_arms.as_slice(), - scrut_ty.clone(), - PlaceValidity::ValidOnly, - None, - ) { + let report = match cx.compute_match_usefulness(m_arms.as_slice(), scrut_ty.clone()) { Ok(report) => report, Err(()) => return, }; @@ -282,13 +275,7 @@ impl ExprValidator { continue; } - let report = match compute_match_usefulness( - &cx, - &[match_arm], - ty.clone(), - PlaceValidity::ValidOnly, - None, - ) { + let report = match cx.compute_match_usefulness(&[match_arm], ty.clone()) { Ok(v) => v, Err(e) => { debug!(?e, "match usefulness error"); diff --git a/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs b/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs index 81ce51429c50..82b80a53e30b 100644 --- a/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs +++ b/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs @@ -8,6 +8,7 @@ use rustc_hash::FxHashMap; use rustc_pattern_analysis::{ constructor::{Constructor, ConstructorSet, VariantVisibility}, index::IdxContainer, + usefulness::{compute_match_usefulness, PlaceValidity, UsefulnessReport}, Captures, PatCx, PrivateUninhabitedField, }; use smallvec::{smallvec, SmallVec}; @@ -59,6 +60,14 @@ impl<'p> MatchCheckCtx<'p> { Self { module, body, db, exhaustive_patterns, min_exhaustive_patterns } } + pub(crate) fn compute_match_usefulness( + &self, + arms: &[MatchArm<'p>], + scrut_ty: Ty, + ) -> Result, ()> { + compute_match_usefulness(self, arms, scrut_ty, PlaceValidity::ValidOnly, None) + } + fn is_uninhabited(&self, ty: &Ty) -> bool { is_ty_uninhabited_from(ty, self.module, self.db) } From e67adf40c98c95bb25a27a66c5b3f20a8094b296 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Mar 2024 16:35:47 +0100 Subject: [PATCH 2/4] Don't assume place validity when we don't know --- crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs b/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs index 82b80a53e30b..dfe082cb5606 100644 --- a/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs +++ b/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs @@ -65,7 +65,9 @@ impl<'p> MatchCheckCtx<'p> { arms: &[MatchArm<'p>], scrut_ty: Ty, ) -> Result, ()> { - compute_match_usefulness(self, arms, scrut_ty, PlaceValidity::ValidOnly, None) + // FIXME: Determine place validity correctly. For now, err on the safe side. + let place_validity = PlaceValidity::MaybeInvalid; + compute_match_usefulness(self, arms, scrut_ty, place_validity, None) } fn is_uninhabited(&self, ty: &Ty) -> bool { From 040f37a99d430395b0d9ace6ea468f895f50fc16 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Mar 2024 16:38:01 +0100 Subject: [PATCH 3/4] Avoid hanging on complex matches --- .../diagnostics/match_check/pat_analysis.rs | 5 ++-- .../src/handlers/missing_match_arms.rs | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs b/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs index dfe082cb5606..f45beb4c92bf 100644 --- a/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs +++ b/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs @@ -67,7 +67,9 @@ impl<'p> MatchCheckCtx<'p> { ) -> Result, ()> { // FIXME: Determine place validity correctly. For now, err on the safe side. let place_validity = PlaceValidity::MaybeInvalid; - compute_match_usefulness(self, arms, scrut_ty, place_validity, None) + // Measured to take ~100ms on modern hardware. + let complexity_limit = Some(500000); + compute_match_usefulness(self, arms, scrut_ty, place_validity, complexity_limit) } fn is_uninhabited(&self, ty: &Ty) -> bool { @@ -476,7 +478,6 @@ impl<'p> PatCx for MatchCheckCtx<'p> { } fn complexity_exceeded(&self) -> Result<(), Self::Error> { - // FIXME(Nadrieril): make use of the complexity counter. Err(()) } } diff --git a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs index 67daa172b27e..c03bb3aeb530 100644 --- a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs +++ b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs @@ -1004,6 +1004,29 @@ fn f() { ); } + #[test] + fn exponential_match() { + // Constructs a match where match checking takes exponential time. Ensures we bail early. + use std::fmt::Write; + let struct_arity = 30; + let mut code = String::new(); + write!(code, "struct BigStruct {{").unwrap(); + for i in 0..struct_arity { + write!(code, " field{i}: bool,").unwrap(); + } + write!(code, "}}").unwrap(); + write!(code, "fn big_match(s: BigStruct) {{").unwrap(); + write!(code, " match s {{").unwrap(); + for i in 0..struct_arity { + write!(code, " BigStruct {{ field{i}: true, ..}} => {{}},").unwrap(); + write!(code, " BigStruct {{ field{i}: false, ..}} => {{}},").unwrap(); + } + write!(code, " _ => {{}},").unwrap(); + write!(code, " }}").unwrap(); + write!(code, "}}").unwrap(); + check_diagnostics_no_bails(&code); + } + mod rust_unstable { use super::*; From 08a5f1e52ad519766ef27c5f61d56c322a394cce Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 19 Mar 2024 15:06:58 +0100 Subject: [PATCH 4/4] Skip the test when testing locally --- crates/ide-diagnostics/src/handlers/missing_match_arms.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs index c03bb3aeb530..045154614f80 100644 --- a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs +++ b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs @@ -23,6 +23,7 @@ mod tests { }, DiagnosticsConfig, }; + use test_utils::skip_slow_tests; #[track_caller] fn check_diagnostics_no_bails(ra_fixture: &str) { @@ -1006,9 +1007,12 @@ fn f() { #[test] fn exponential_match() { + if skip_slow_tests() { + return; + } // Constructs a match where match checking takes exponential time. Ensures we bail early. use std::fmt::Write; - let struct_arity = 30; + let struct_arity = 50; let mut code = String::new(); write!(code, "struct BigStruct {{").unwrap(); for i in 0..struct_arity {