From 400dc46a05f41ba1e1137f11073cec1951442668 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 24 Jan 2024 16:24:52 +0100 Subject: [PATCH] Gracefully abort on type incompatibility Since the only use of `TypeCx::bug` is in `Constructor::is_covered_by`, it is tempting to return `false` instead of `Err()`, but that would cause "non-exhaustive match" false positives. --- .../rustc_pattern_analysis/src/constructor.rs | 18 +++++++++++------- compiler/rustc_pattern_analysis/src/lib.rs | 2 +- .../rustc_pattern_analysis/src/pat_column.rs | 2 +- compiler/rustc_pattern_analysis/src/rustc.rs | 2 +- .../rustc_pattern_analysis/src/usefulness.rs | 8 ++++---- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 4be564b1d7b1e..7a91e14930961 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -826,10 +826,12 @@ impl Constructor { /// this checks for inclusion. // We inline because this has a single call site in `Matrix::specialize_constructor`. #[inline] - pub(crate) fn is_covered_by(&self, cx: &Cx, other: &Self) -> bool { - match (self, other) { + pub(crate) fn is_covered_by(&self, cx: &Cx, other: &Self) -> Result { + Ok(match (self, other) { (Wildcard, _) => { - cx.bug(format_args!("Constructor splitting should not have returned `Wildcard`")) + return Err(cx.bug(format_args!( + "Constructor splitting should not have returned `Wildcard`" + ))); } // Wildcards cover anything (_, Wildcard) => true, @@ -871,10 +873,12 @@ impl Constructor { (Opaque(self_id), Opaque(other_id)) => self_id == other_id, (Opaque(..), _) | (_, Opaque(..)) => false, - _ => cx.bug(format_args!( - "trying to compare incompatible constructors {self:?} and {other:?}" - )), - } + _ => { + return Err(cx.bug(format_args!( + "trying to compare incompatible constructors {self:?} and {other:?}" + ))); + } + }) } } diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 3d0eb117d174f..8e16d4d7bf4e2 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -123,7 +123,7 @@ pub trait TypeCx: Sized + fmt::Debug { ) -> fmt::Result; /// Raise a bug. - fn bug(&self, fmt: fmt::Arguments<'_>) -> !; + fn bug(&self, fmt: fmt::Arguments<'_>) -> Self::Error; /// Lint that the range `pat` overlapped with all the ranges in `overlaps_with`, where the range /// they overlapped over is `overlaps_on`. We only detect singleton overlaps. diff --git a/compiler/rustc_pattern_analysis/src/pat_column.rs b/compiler/rustc_pattern_analysis/src/pat_column.rs index 3cacfc491b981..0339818d61d31 100644 --- a/compiler/rustc_pattern_analysis/src/pat_column.rs +++ b/compiler/rustc_pattern_analysis/src/pat_column.rs @@ -78,7 +78,7 @@ impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { let mut specialized_columns: Vec<_> = (0..arity).map(|_| Self { patterns: Vec::new() }).collect(); let relevant_patterns = - self.patterns.iter().filter(|pat| ctor.is_covered_by(cx, pat.ctor())); + self.patterns.iter().filter(|pat| ctor.is_covered_by(cx, pat.ctor()).unwrap_or(false)); for pat in relevant_patterns { let specialized = pat.specialize(ctor, arity); for (subpat, column) in specialized.into_iter().zip(&mut specialized_columns) { diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index ec37e20211860..adcb5f827d5e7 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -900,7 +900,7 @@ impl<'p, 'tcx> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> { Ok(()) } - fn bug(&self, fmt: fmt::Arguments<'_>) -> ! { + fn bug(&self, fmt: fmt::Arguments<'_>) -> Self::Error { span_bug!(self.scrut_span, "{}", fmt) } diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 3d45d032a9934..d9da9bbbad079 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -1082,7 +1082,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { pcx: &PlaceCtxt<'_, Cx>, ctor: &Constructor, ctor_is_relevant: bool, - ) -> Matrix<'p, Cx> { + ) -> Result, Cx::Error> { let ctor_sub_tys = pcx.ctor_sub_tys(ctor); let arity = ctor_sub_tys.len(); let specialized_place_ty = ctor_sub_tys.chain(self.place_ty[1..].iter().cloned()).collect(); @@ -1098,12 +1098,12 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { wildcard_row_is_relevant: self.wildcard_row_is_relevant && ctor_is_relevant, }; for (i, row) in self.rows().enumerate() { - if ctor.is_covered_by(pcx.cx, row.head().ctor()) { + if ctor.is_covered_by(pcx.cx, row.head().ctor())? { let new_row = row.pop_head_constructor(ctor, arity, ctor_is_relevant, i); matrix.expand_and_push(new_row); } } - matrix + Ok(matrix) } } @@ -1533,7 +1533,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( // strictly fewer rows. In that case we can sometimes skip it. See the top of the file for // details. let ctor_is_relevant = matches!(ctor, Constructor::Missing) || missing_ctors.is_empty(); - let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor, ctor_is_relevant); + let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor, ctor_is_relevant)?; let mut witnesses = ensure_sufficient_stack(|| { compute_exhaustiveness_and_usefulness(mcx, &mut spec_matrix, false) })?;