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

Add the min_exhaustive_patterns feature gate #118803

Merged
merged 2 commits into from
Jan 26, 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: 3 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,9 @@ declare_features! (
(unstable, macro_metavar_expr, "1.61.0", Some(83527)),
/// Allows `#[marker]` on certain traits allowing overlapping implementations.
(unstable, marker_trait_attr, "1.30.0", Some(29864)),
/// Allows exhaustive pattern matching on types that contain uninhabited types in cases that are
/// unambiguously sound.
(incomplete, min_exhaustive_patterns, "CURRENT_RUSTC_VERSION", Some(119612)),
/// A minimal, sound subset of specialization intended to be used by the
/// standard library until the soundness issues with specialization
/// are fixed.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_pattern_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ pub trait TypeCx: Sized + fmt::Debug {
type PatData: Clone;

fn is_exhaustive_patterns_feature_on(&self) -> bool;
fn is_min_exhaustive_patterns_feature_on(&self) -> bool;

/// The number of fields for this constructor.
fn ctor_arity(&self, ctor: &Constructor<Self>, ty: &Self::Ty) -> usize;
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ impl<'p, 'tcx> RustcMatchCheckCtxt<'p, 'tcx> {
// `field.ty()` doesn't normalize after substituting.
let ty = cx.tcx.normalize_erasing_regions(cx.param_env, ty);
let is_visible = adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx);
let is_uninhabited = cx.tcx.features().exhaustive_patterns && cx.is_uninhabited(ty);
let is_uninhabited = (cx.tcx.features().exhaustive_patterns
|| cx.tcx.features().min_exhaustive_patterns)
&& cx.is_uninhabited(ty);

if is_uninhabited && (!is_visible || is_non_exhaustive) {
None
Expand Down Expand Up @@ -960,6 +962,9 @@ impl<'p, 'tcx> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
fn is_exhaustive_patterns_feature_on(&self) -> bool {
self.tcx.features().exhaustive_patterns
}
fn is_min_exhaustive_patterns_feature_on(&self) -> bool {
self.tcx.features().min_exhaustive_patterns
}

fn ctor_arity(&self, ctor: &crate::constructor::Constructor<Self>, ty: &Self::Ty) -> usize {
self.ctor_arity(ctor, *ty)
Expand Down
26 changes: 17 additions & 9 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,11 +548,12 @@
//! [`ValidityConstraint::specialize`].
//!
//! Having said all that, in practice we don't fully follow what's been presented in this section.
//! Under `exhaustive_patterns`, we allow omitting empty arms even in `!known_valid` places, for
//! backwards-compatibility until we have a better alternative. Without `exhaustive_patterns`, we
//! mostly treat empty types as inhabited, except specifically a non-nested `!` or empty enum. In
//! this specific case we also allow the empty match regardless of place validity, for
//! backwards-compatibility. Hopefully we can eventually deprecate this.
//! Let's call "toplevel exception" the case where the match scrutinee itself has type `!` or
//! `EmptyEnum`. First, on stable rust, we require `_` patterns for empty types in all cases apart
//! from the toplevel exception. The `exhaustive_patterns` and `min_exaustive_patterns` allow
//! omitting patterns in the cases described above. There's a final detail: in the toplevel
//! exception or with the `exhaustive_patterns` feature, we ignore place validity when checking
//! whether a pattern is required for exhaustiveness. I (Nadrieril) hope to deprecate this behavior.
//!
//!
//!
Expand Down Expand Up @@ -1440,10 +1441,17 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
// 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;
// Whether empty patterns are counted as useful or not. We only warn an empty arm unreachable if
// it is guaranteed unreachable by the opsem (i.e. if the place is `known_valid`).
let empty_arms_are_unreachable = place_validity.is_known_valid()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was hard to understand what was going on here until i noticed that:

  1. these bools flipped order
  2. the old can_omit_empty_arms was "inlined" into empty_arms_are_unreachable
  3. the new can_omit_empty_arms is not enabled for the new min feature gate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried so hard to make it legible :') The new can_omit_empty_arms is enabled with the feature gate, but only exactly when empty_arms_are_unreachable is true, i.e. the place must be known_valid.

I tried to present it as "the base case is can_omit_empty_arms == empty_arms_are_unreachable, on top of which there are two exceptions" but I guess that wasn't clear

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right -- empty_arms_are_unreachable || in the new can_omit_empty_arms.

&& (is_toplevel_exception
|| mcx.tycx.is_exhaustive_patterns_feature_on()
|| mcx.tycx.is_min_exhaustive_patterns_feature_on());
// Whether empty patterns can be omitted for exhaustiveness. We ignore place validity in the
// toplevel exception and `exhaustive_patterns` cases for backwards compatibility.
let can_omit_empty_arms = empty_arms_are_unreachable
|| is_toplevel_exception
|| mcx.tycx.is_exhaustive_patterns_feature_on();

// Analyze the constructors present in this column.
let ctors = matrix.heads().map(|p| p.ctor());
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,7 @@ symbols! {
min_const_fn,
min_const_generics,
min_const_unsafe_fn,
min_exhaustive_patterns,
min_specialization,
min_type_alias_impl_trait,
minnumf32,
Expand Down
Loading
Loading