Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exhaustiveness: simplify empty pattern logic #119835

Merged
merged 5 commits into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
fn is_known_valid_scrutinee(&self, scrutinee: &Expr<'tcx>) -> bool {
use ExprKind::*;
match &scrutinee.kind {
// Both pointers and references can validly point to a place with invalid data.
// Pointers can validly point to a place with invalid data. It is undecided whether
// references can too, so we conservatively assume they can.
Deref { .. } => false,
// Inherit validity of the parent place, unless the parent is an union.
Field { lhs, .. } => {
Expand Down
19 changes: 5 additions & 14 deletions compiler/rustc_pattern_analysis/src/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,12 +858,14 @@ impl<Cx: TypeCx> ConstructorSet<Cx> {
/// any) are missing; 2/ split constructors to handle non-trivial intersections e.g. on ranges
/// or slices. This can get subtle; see [`SplitConstructorSet`] for details of this operation
/// and its invariants.
#[instrument(level = "debug", skip(self, pcx, ctors), ret)]
#[instrument(level = "debug", skip(self, ctors), ret)]
pub(crate) fn split<'a>(
&self,
pcx: &PlaceCtxt<'a, Cx>,
ctors: impl Iterator<Item = &'a Constructor<Cx>> + Clone,
) -> SplitConstructorSet<Cx> {
) -> SplitConstructorSet<Cx>
where
Cx: 'a,
{
let mut present: SmallVec<[_; 1]> = SmallVec::new();
// Empty constructors found missing.
let mut missing_empty = Vec::new();
Expand Down Expand Up @@ -1003,17 +1005,6 @@ impl<Cx: TypeCx> ConstructorSet<Cx> {
}
}

// We have now grouped all the constructors into 3 buckets: present, missing, missing_empty.
// In the absence of the `exhaustive_patterns` feature however, we don't count nested empty
// types as empty. Only non-nested `!` or `enum Foo {}` are considered empty.
if !pcx.mcx.tycx.is_exhaustive_patterns_feature_on()
&& !(pcx.is_scrutinee && matches!(self, Self::NoConstructors))
{
// Treat all missing constructors as nonempty.
// This clears `missing_empty`.
missing.append(&mut missing_empty);
}

SplitConstructorSet { present, missing, missing_empty }
}
}
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 @@ -59,7 +59,7 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
) -> Result<SplitConstructorSet<'p, 'tcx>, ErrorGuaranteed> {
let column_ctors = self.patterns.iter().map(|p| p.ctor());
let ctors_for_ty = &pcx.ctors_for_ty()?;
Ok(ctors_for_ty.split(pcx, column_ctors))
Ok(ctors_for_ty.split(column_ctors))
}

/// Does specialization: given a constructor, this takes the patterns from the column that match
Expand Down
57 changes: 22 additions & 35 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,15 +737,13 @@ pub(crate) struct PlaceCtxt<'a, Cx: TypeCx> {
pub(crate) mcx: MatchCtxt<'a, Cx>,
/// Type of the place under investigation.
pub(crate) ty: Cx::Ty,
/// Whether the place is the original scrutinee place, as opposed to a subplace of it.
pub(crate) is_scrutinee: bool,
}

impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> {
/// A `PlaceCtxt` when code other than `is_useful` needs one.
#[cfg_attr(not(feature = "rustc"), allow(dead_code))]
pub(crate) fn new_dummy(mcx: MatchCtxt<'a, Cx>, ty: Cx::Ty) -> Self {
PlaceCtxt { mcx, ty, is_scrutinee: false }
PlaceCtxt { mcx, ty }
}

pub(crate) fn ctor_arity(&self, ctor: &Constructor<Cx>) -> usize {
Expand All @@ -768,30 +766,16 @@ impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> {
pub enum ValidityConstraint {
ValidOnly,
MaybeInvalid,
/// Option for backwards compatibility: the place is not known to be valid but we allow omitting
/// `useful && !reachable` arms anyway.
MaybeInvalidButAllowOmittingArms,
}

impl ValidityConstraint {
pub fn from_bool(is_valid_only: bool) -> Self {
if is_valid_only { ValidOnly } else { MaybeInvalid }
}

fn allow_omitting_side_effecting_arms(self) -> Self {
match self {
MaybeInvalid | MaybeInvalidButAllowOmittingArms => MaybeInvalidButAllowOmittingArms,
// There are no side-effecting empty arms here, nothing to do.
ValidOnly => ValidOnly,
}
}

fn is_known_valid(self) -> bool {
matches!(self, ValidOnly)
}
fn allows_omitting_empty_arms(self) -> bool {
matches!(self, ValidOnly | MaybeInvalidButAllowOmittingArms)
}

/// If the place has validity given by `self` and we read that the value at the place has
/// constructor `ctor`, this computes what we can assume about the validity of the constructor
Expand All @@ -814,7 +798,7 @@ impl fmt::Display for ValidityConstraint {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let s = match self {
ValidOnly => "✓",
MaybeInvalid | MaybeInvalidButAllowOmittingArms => "?",
MaybeInvalid => "?",
};
write!(f, "{s}")
}
Expand Down Expand Up @@ -1456,41 +1440,44 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
};

debug!("ty: {ty:?}");
let pcx = &PlaceCtxt { mcx, ty, is_scrutinee: is_top_level };
let pcx = &PlaceCtxt { mcx, ty };
let ctors_for_ty = pcx.ctors_for_ty()?;

// Whether the place/column we are inspecting is known to contain valid data.
let place_validity = matrix.place_validity[0];
// For backwards compability we allow omitting some empty arms that we ideally shouldn't.
let place_validity = place_validity.allow_omitting_side_effecting_arms();
// We treat match scrutinees of type `!` or `EmptyEnum` differently.
let is_toplevel_exception =
is_top_level && matches!(ctors_for_ty, ConstructorSet::NoConstructors);
// Whether empty patterns can be omitted for exhaustiveness.
let can_omit_empty_arms = is_toplevel_exception || mcx.tycx.is_exhaustive_patterns_feature_on();
// Whether empty patterns are counted as useful or not.
let empty_arms_are_unreachable = place_validity.is_known_valid() && can_omit_empty_arms;

// Analyze the constructors present in this column.
let ctors = matrix.heads().map(|p| p.ctor());
let ctors_for_ty = pcx.ctors_for_ty()?;
let is_integers = matches!(ctors_for_ty, ConstructorSet::Integers { .. }); // For diagnostics.
let split_set = ctors_for_ty.split(pcx, ctors);
let mut split_set = ctors_for_ty.split(ctors);
let all_missing = split_set.present.is_empty();

// Build the set of constructors we will specialize with. It must cover the whole type.
// We need to iterate over a full set of constructors, so we add `Missing` to represent the
// missing ones. This is explained under "Constructor Splitting" at the top of this file.
let mut split_ctors = split_set.present;
if !split_set.missing.is_empty() {
// We need to iterate over a full set of constructors, so we add `Missing` to represent the
// missing ones. This is explained under "Constructor Splitting" at the top of this file.
split_ctors.push(Constructor::Missing);
} else if !split_set.missing_empty.is_empty() && !place_validity.is_known_valid() {
// The missing empty constructors are reachable if the place can contain invalid data.
if !(split_set.missing.is_empty()
&& (split_set.missing_empty.is_empty() || empty_arms_are_unreachable))
{
split_ctors.push(Constructor::Missing);
}

// Decide what constructors to report.
let is_integers = matches!(ctors_for_ty, ConstructorSet::Integers { .. });
let always_report_all = is_top_level && !is_integers;
// Whether we should report "Enum::A and Enum::C are missing" or "_ is missing".
let report_individual_missing_ctors = always_report_all || !all_missing;
// Which constructors are considered missing. We ensure that `!missing_ctors.is_empty() =>
// split_ctors.contains(Missing)`. The converse usually holds except in the
// `MaybeInvalidButAllowOmittingArms` backwards-compatibility case.
// split_ctors.contains(Missing)`. The converse usually holds except when
// `!place_validity.is_known_valid()`.
let mut missing_ctors = split_set.missing;
if !place_validity.allows_omitting_empty_arms() {
missing_ctors.extend(split_set.missing_empty);
if !can_omit_empty_arms {
missing_ctors.append(&mut split_set.missing_empty);
}

let mut ret = WitnessMatrix::empty();
Expand Down
Loading