Skip to content

Commit

Permalink
Rollup merge of #121823 - Nadrieril:never-witnesses, r=compiler-errors
Browse files Browse the repository at this point in the history
never patterns: suggest `!` patterns on non-exhaustive matches

When a match is non-exhaustive we now suggest never patterns whenever it makes sense.

r? ``@compiler-errors``
  • Loading branch information
matthiaskrgr authored Mar 18, 2024
2 parents 05f7633 + 1b31e14 commit 2d3dcfa
Show file tree
Hide file tree
Showing 11 changed files with 982 additions and 242 deletions.
74 changes: 39 additions & 35 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -938,43 +938,30 @@ fn report_non_exhaustive_match<'p, 'tcx>(
};
// In the case of an empty match, replace the '`_` not covered' diagnostic with something more
// informative.
let mut err;
let pattern;
let patterns_len;
if is_empty_match && !non_empty_enum {
return cx.tcx.dcx().emit_err(NonExhaustivePatternsTypeNotEmpty {
cx,
expr_span,
span: sp,
ty: scrut_ty,
});
} else {
// FIXME: migration of this diagnostic will require list support
let joined_patterns = joined_uncovered_patterns(cx, &witnesses);
err = create_e0004(
cx.tcx.sess,
sp,
format!("non-exhaustive patterns: {joined_patterns} not covered"),
);
err.span_label(
sp,
format!(
"pattern{} {} not covered",
rustc_errors::pluralize!(witnesses.len()),
joined_patterns
),
);
patterns_len = witnesses.len();
pattern = if witnesses.len() < 4 {
witnesses
.iter()
.map(|witness| cx.hoist_witness_pat(witness).to_string())
.collect::<Vec<String>>()
.join(" | ")
} else {
"_".to_string()
};
};
}

// FIXME: migration of this diagnostic will require list support
let joined_patterns = joined_uncovered_patterns(cx, &witnesses);
let mut err = create_e0004(
cx.tcx.sess,
sp,
format!("non-exhaustive patterns: {joined_patterns} not covered"),
);
err.span_label(
sp,
format!(
"pattern{} {} not covered",
rustc_errors::pluralize!(witnesses.len()),
joined_patterns
),
);

// Point at the definition of non-covered `enum` variants.
if let Some(AdtDefinedHere { adt_def_span, ty, variants }) =
Expand Down Expand Up @@ -1021,6 +1008,23 @@ fn report_non_exhaustive_match<'p, 'tcx>(
}
}

// Whether we suggest the actual missing patterns or `_`.
let suggest_the_witnesses = witnesses.len() < 4;
let suggested_arm = if suggest_the_witnesses {
let pattern = witnesses
.iter()
.map(|witness| cx.hoist_witness_pat(witness).to_string())
.collect::<Vec<String>>()
.join(" | ");
if witnesses.iter().all(|p| p.is_never_pattern()) && cx.tcx.features().never_patterns {
// Arms with a never pattern don't take a body.
pattern
} else {
format!("{pattern} => todo!()")
}
} else {
format!("_ => todo!()")
};
let mut suggestion = None;
let sm = cx.tcx.sess.source_map();
match arms {
Expand All @@ -1033,7 +1037,7 @@ fn report_non_exhaustive_match<'p, 'tcx>(
};
suggestion = Some((
sp.shrink_to_hi().with_hi(expr_span.hi()),
format!(" {{{indentation}{more}{pattern} => todo!(),{indentation}}}",),
format!(" {{{indentation}{more}{suggested_arm},{indentation}}}",),
));
}
[only] => {
Expand All @@ -1059,7 +1063,7 @@ fn report_non_exhaustive_match<'p, 'tcx>(
};
suggestion = Some((
only.span.shrink_to_hi(),
format!("{comma}{pre_indentation}{pattern} => todo!()"),
format!("{comma}{pre_indentation}{suggested_arm}"),
));
}
[.., prev, last] => {
Expand All @@ -1082,7 +1086,7 @@ fn report_non_exhaustive_match<'p, 'tcx>(
if let Some(spacing) = spacing {
suggestion = Some((
last.span.shrink_to_hi(),
format!("{comma}{spacing}{pattern} => todo!()"),
format!("{comma}{spacing}{suggested_arm}"),
));
}
}
Expand All @@ -1093,13 +1097,13 @@ fn report_non_exhaustive_match<'p, 'tcx>(
let msg = format!(
"ensure that all possible cases are being handled by adding a match arm with a wildcard \
pattern{}{}",
if patterns_len > 1 && patterns_len < 4 && suggestion.is_some() {
if witnesses.len() > 1 && suggest_the_witnesses && suggestion.is_some() {
", a match arm with multiple or-patterns"
} else {
// we are either not suggesting anything, or suggesting `_`
""
},
match patterns_len {
match witnesses.len() {
// non-exhaustive enum case
0 if suggestion.is_some() => " as shown",
0 => "",
Expand Down
39 changes: 33 additions & 6 deletions compiler/rustc_pattern_analysis/src/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,15 +678,19 @@ pub enum Constructor<Cx: PatCx> {
Or,
/// Wildcard pattern.
Wildcard,
/// Never pattern. Only used in `WitnessPat`. An actual never pattern should be lowered as
/// `Wildcard`.
Never,
/// Fake extra constructor for enums that aren't allowed to be matched exhaustively. Also used
/// for those types for which we cannot list constructors explicitly, like `f64` and `str`.
/// for those types for which we cannot list constructors explicitly, like `f64` and `str`. Only
/// used in `WitnessPat`.
NonExhaustive,
/// Fake extra constructor for variants that should not be mentioned in diagnostics.
/// We use this for variants behind an unstable gate as well as
/// `#[doc(hidden)]` ones.
/// Fake extra constructor for variants that should not be mentioned in diagnostics. We use this
/// for variants behind an unstable gate as well as `#[doc(hidden)]` ones. Only used in
/// `WitnessPat`.
Hidden,
/// Fake extra constructor for constructors that are not seen in the matrix, as explained at the
/// top of the file.
/// top of the file. Only used for specialization.
Missing,
/// Fake extra constructor that indicates and empty field that is private. When we encounter one
/// we skip the column entirely so we don't observe its emptiness. Only used for specialization.
Expand All @@ -708,6 +712,7 @@ impl<Cx: PatCx> Clone for Constructor<Cx> {
Constructor::Str(value) => Constructor::Str(value.clone()),
Constructor::Opaque(inner) => Constructor::Opaque(inner.clone()),
Constructor::Or => Constructor::Or,
Constructor::Never => Constructor::Never,
Constructor::Wildcard => Constructor::Wildcard,
Constructor::NonExhaustive => Constructor::NonExhaustive,
Constructor::Hidden => Constructor::Hidden,
Expand Down Expand Up @@ -1040,10 +1045,32 @@ impl<Cx: PatCx> ConstructorSet<Cx> {
// In a `MaybeInvalid` place even an empty pattern may be reachable. We therefore
// add a dummy empty constructor here, which will be ignored if the place is
// `ValidOnly`.
missing_empty.push(NonExhaustive);
missing_empty.push(Never);
}
}

SplitConstructorSet { present, missing, missing_empty }
}

/// Whether this set only contains empty constructors.
pub(crate) fn all_empty(&self) -> bool {
match self {
ConstructorSet::Bool
| ConstructorSet::Integers { .. }
| ConstructorSet::Ref
| ConstructorSet::Union
| ConstructorSet::Unlistable => false,
ConstructorSet::NoConstructors => true,
ConstructorSet::Struct { empty } => *empty,
ConstructorSet::Variants { variants, non_exhaustive } => {
!*non_exhaustive
&& variants
.iter()
.all(|visibility| matches!(visibility, VariantVisibility::Empty))
}
ConstructorSet::Slice { array_len, subtype_is_empty } => {
*subtype_is_empty && matches!(array_len, Some(1..))
}
}
}
}
21 changes: 18 additions & 3 deletions compiler/rustc_pattern_analysis/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ impl<Cx: PatCx> fmt::Debug for DeconstructedPat<Cx> {
}
Ok(())
}
Never => write!(f, "!"),
Wildcard | Missing | NonExhaustive | Hidden | PrivateUninhabited => {
write!(f, "_ : {:?}", pat.ty())
}
Expand Down Expand Up @@ -311,18 +312,24 @@ impl<Cx: PatCx> WitnessPat<Cx> {
pub(crate) fn new(ctor: Constructor<Cx>, fields: Vec<Self>, ty: Cx::Ty) -> Self {
Self { ctor, fields, ty }
}
pub(crate) fn wildcard(ty: Cx::Ty) -> Self {
Self::new(Wildcard, Vec::new(), ty)
/// Create a wildcard pattern for this type. If the type is empty, we create a `!` pattern.
pub(crate) fn wildcard(cx: &Cx, ty: Cx::Ty) -> Self {
let is_empty = cx.ctors_for_ty(&ty).is_ok_and(|ctors| ctors.all_empty());
let ctor = if is_empty { Never } else { Wildcard };
Self::new(ctor, Vec::new(), ty)
}

/// Construct a pattern that matches everything that starts with this constructor.
/// For example, if `ctor` is a `Constructor::Variant` for `Option::Some`, we get the pattern
/// `Some(_)`.
pub(crate) fn wild_from_ctor(cx: &Cx, ctor: Constructor<Cx>, ty: Cx::Ty) -> Self {
if matches!(ctor, Wildcard) {
return Self::wildcard(cx, ty);
}
let fields = cx
.ctor_sub_tys(&ctor, &ty)
.filter(|(_, PrivateUninhabitedField(skip))| !skip)
.map(|(ty, _)| Self::wildcard(ty))
.map(|(ty, _)| Self::wildcard(cx, ty))
.collect();
Self::new(ctor, fields, ty)
}
Expand All @@ -334,6 +341,14 @@ impl<Cx: PatCx> WitnessPat<Cx> {
&self.ty
}

pub fn is_never_pattern(&self) -> bool {
match self.ctor() {
Never => true,
Or => self.fields.iter().all(|p| p.is_never_pattern()),
_ => self.fields.iter().any(|p| p.is_never_pattern()),
}
}

pub fn iter_fields(&self) -> impl Iterator<Item = &WitnessPat<Cx>> {
self.fields.iter()
}
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
_ => bug!("bad slice pattern {:?} {:?}", ctor, ty),
},
Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..)
| NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => &[],
| Never | NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => &[],
Or => {
bug!("called `Fields::wildcards` on an `Or` ctor")
}
Expand Down Expand Up @@ -275,7 +275,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
Ref => 1,
Slice(slice) => slice.arity(),
Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..)
| NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => 0,
| Never | NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => 0,
Or => bug!("The `Or` constructor doesn't have a fixed arity"),
}
}
Expand Down Expand Up @@ -824,7 +824,8 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
}
}
&Str(value) => PatKind::Constant { value },
Wildcard | NonExhaustive | Hidden | PrivateUninhabited => PatKind::Wild,
Never if self.tcx.features().never_patterns => PatKind::Never,
Never | Wildcard | NonExhaustive | Hidden | PrivateUninhabited => PatKind::Wild,
Missing { .. } => bug!(
"trying to convert a `Missing` constructor into a `Pat`; this is probably a bug,
`Missing` should have been processed in `apply_constructors`"
Expand Down
Loading

0 comments on commit 2d3dcfa

Please sign in to comment.