Skip to content

Commit

Permalink
Rollup merge of rust-lang#118803 - Nadrieril:min-exhaustive-patterns,…
Browse files Browse the repository at this point in the history
… r=compiler-errors

Add the `min_exhaustive_patterns` feature gate

## Motivation

Pattern-matching on empty types is tricky around unsafe code. For that reason, current stable rust conservatively requires arms for empty types in all but the simplest case. It has long been the intention to allow omitting empty arms when it's safe to do so. The [`exhaustive_patterns`](rust-lang#51085) feature allows the omission of all empty arms, but hasn't been stabilized because that was deemed dangerous around unsafe code.

## Proposal

This feature aims to stabilize an uncontroversial subset of exhaustive_patterns. Namely: when `min_exhaustive_patterns` is enabled and the data we're matching on is guaranteed to be valid by rust's operational semantics, then we allow empty arms to be omitted. E.g.:

```rust
let x: Result<T, !> = foo();
match x { // ok
    Ok(y) => ...,
}
let Ok(y) = x; // ok
```

If the place is not guaranteed to hold valid data (namely ptr dereferences, ref dereferences (conservatively) and union field accesses), then we keep stable behavior i.e. we (usually) require arms for the empty cases.

```rust
unsafe {
    let ptr: *const Result<u32, !> = ...;
    match *ptr {
        Ok(x) => { ... }
        Err(_) => { ... } // still required
    }
}
let foo: Result<u32, &!> = ...;
match foo {
    Ok(x) => { ... }
    Err(&_) => { ... } // still required because of the dereference
}
unsafe {
    let ptr: *const ! = ...;
    match *ptr {} // already allowed on stable
}
```

Note that we conservatively consider that a valid reference can point to invalid data, hence we don't allow arms of type `&!` and similar cases to be omitted. This could eventually change depending on [opsem decisions](rust-lang/unsafe-code-guidelines#413). Whenever opsem is undecided on a case, we conservatively keep today's stable behavior.

I proposed this behavior in the [`never_patterns`](rust-lang#118155) feature gate but it makes sense on its own and could be stabilized more quickly. The two proposals nicely complement each other.

## Unresolved Questions

Part of the question is whether this requires an RFC. I'd argue this doesn't need one since there is no design question beyond the intent to omit unreachable patterns, but I'm aware the problem can be framed in ways that require design (I'm thinking of the [original never patterns proposal](https://smallcultfollowing.com/babysteps/blog/2018/08/13/never-patterns-exhaustive-matching-and-uninhabited-types-oh-my/), which would frame this behavior as "auto-nevering" happening).

EDIT: I initially proposed a future-compatibility lint as part of this feature, I don't anymore.
  • Loading branch information
matthiaskrgr authored Jan 26, 2024
2 parents 7749e49 + 95a14d4 commit 2a41d8b
Show file tree
Hide file tree
Showing 9 changed files with 801 additions and 149 deletions.
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 @@ -181,7 +181,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 @@ -863,6 +865,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 @@ -1442,10 +1443,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()
&& (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 @@ -1029,6 +1029,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

0 comments on commit 2a41d8b

Please sign in to comment.