Skip to content

Commit

Permalink
Gracefully abort on type incompatibility
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Nadrieril committed Jan 28, 2024
1 parent 6351247 commit b15141a
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 17 deletions.
27 changes: 17 additions & 10 deletions compiler/rustc_pattern_analysis/src/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,12 +733,17 @@ impl<Cx: TypeCx> Constructor<Cx> {
/// 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, pcx: &PlaceCtxt<'_, Cx>, other: &Self) -> bool {
match (self, other) {
(Wildcard, _) => pcx
.mcx
.tycx
.bug(format_args!("Constructor splitting should not have returned `Wildcard`")),
pub(crate) fn is_covered_by(
&self,
pcx: &PlaceCtxt<'_, Cx>,
other: &Self,
) -> Result<bool, Cx::Error> {
Ok(match (self, other) {
(Wildcard, _) => {
return Err(pcx.mcx.tycx.bug(format_args!(
"Constructor splitting should not have returned `Wildcard`"
)));
}
// Wildcards cover anything
(_, Wildcard) => true,
// Only a wildcard pattern can match these special constructors.
Expand Down Expand Up @@ -779,10 +784,12 @@ impl<Cx: TypeCx> Constructor<Cx> {
(Opaque(self_id), Opaque(other_id)) => self_id == other_id,
(Opaque(..), _) | (_, Opaque(..)) => false,

_ => pcx.mcx.tycx.bug(format_args!(
"trying to compare incompatible constructors {self:?} and {other:?}"
)),
}
_ => {
return Err(pcx.mcx.tycx.bug(format_args!(
"trying to compare incompatible constructors {self:?} and {other:?}"
)));
}
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_pattern_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,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.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_pattern_analysis/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
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(pcx, pat.ctor()));
self.patterns.iter().filter(|pat| ctor.is_covered_by(pcx, 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) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -901,7 +901,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)
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
pcx: &PlaceCtxt<'_, Cx>,
ctor: &Constructor<Cx>,
ctor_is_relevant: bool,
) -> Matrix<'p, Cx> {
) -> Result<Matrix<'p, Cx>, 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();
Expand All @@ -1075,12 +1075,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, row.head().ctor()) {
if ctor.is_covered_by(pcx, 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)
}
}

Expand Down Expand Up @@ -1490,7 +1490,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)
})?;
Expand Down

0 comments on commit b15141a

Please sign in to comment.