Lint against empty match on not-known-valid place

Author: Nadrieril

compiler/rustc_lint_defs/src/builtin.rs

@@ -39,6 +39,7 @@ declare_lint_pass! {
         DUPLICATE_MACRO_ATTRIBUTES,
         ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
         ELIDED_LIFETIMES_IN_PATHS,
+        EMPTY_MATCH_ON_UNSAFE_PLACE,
         EXPORTED_PRIVATE_DEPENDENCIES,
         FFI_UNWIND_CALLS,
         FORBIDDEN_LINT_GROUPS,
@@ -4018,6 +4019,76 @@ declare_lint! {
     @feature_gate = sym::non_exhaustive_omitted_patterns_lint;
 }
 
+declare_lint! {
+    /// The `empty_match_on_unsafe_place` lint detects uses of `match ... {}` on an empty type where
+    /// the matched place could contain invalid data in a well-defined program. These matches are
+    /// considered exhaustive for backwards-compatibility, but they shouldn't be since a `_` arm
+    /// would be reachable.
+    ///
+    /// This will become an error in the future.
+    ///
+    /// ### Example
+    ///
+    /// ```compile_fail
+    /// #![feature(min_exhaustive_patterns)]
+    /// enum Void {}
+    /// let ptr: *const Void = ...;
+    /// unsafe {
+    ///     match *ptr {}
+    /// }
+    /// ```
+    ///
+    /// This will produce:
+    ///
+    /// ```text
+    /// warning: empty match on potentially-invalid data
+    ///   --> $DIR/empty-types.rs:157:9
+    ///    |
+    /// LL |         match *ptr {}
+    ///    |         ^^^^^^^^^^^^^
+    ///    |
+    /// note: this place can hold invalid data, which would make the match reachable
+    ///   --> $DIR/empty-types.rs:157:15
+    ///    |
+    /// LL |         match *ptr {}
+    ///    |               ^^^^
+    ///    = note: `#[warn(empty_match_on_unsafe_place)]` on by default
+    /// help: consider forcing a read of the value
+    ///    |
+    /// LL |         match { *ptr } {}
+    ///    |               +      +
+    /// ```
+    ///
+    /// ### Explanation
+    ///
+    /// Some place expressions (namely pointer dereferences, union field accesses, and
+    /// (conservatively) reference dereferences) can hold invalid data without causing UB. For
+    /// example, the following is a well-defined program that prints "reachable!".
+    ///
+    /// ```rust
+    /// #[derive(Copy, Clone)]
+    /// enum Void {}
+    /// union Uninit<T: Copy> {
+    ///     value: T,
+    ///     uninit: (),
+    /// }
+    /// unsafe {
+    ///     let x: Uninit<Void> = Uninit { uninit: () };
+    ///     match x.value {
+    ///         _ => println!("reachable!"),
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// Therefore when the matched place can hold invalid data, a match with no arm should not be
+    /// considered exhaustive. For backwards-compatibility we consider them exhaustive but warn with
+    /// this lint. It will become an error in the future.
+    pub EMPTY_MATCH_ON_UNSAFE_PLACE,
+    Warn,
+    "warn about empty matches on a place with potentially-invalid data",
+    @feature_gate = sym::min_exhaustive_patterns;
+}
+
 declare_lint! {
     /// The `text_direction_codepoint_in_comment` lint detects Unicode codepoints in comments that
     /// change the visual representation of text on screen in a way that does not correspond to

compiler/rustc_pattern_analysis/messages.ftl

@@ -1,3 +1,10 @@
+pattern_analysis_empty_match_on_unsafe_place =
+    empty match on potentially-invalid data
+    .note = this place can hold invalid data, which would make the match reachable
+
+pattern_analysis_empty_match_on_unsafe_place_wrap_suggestion =
+    consider forcing a read of the value
+
 pattern_analysis_non_exhaustive_omitted_pattern = some variants are not matched explicitly
     .help = ensure that all variants are matched explicitly by adding the suggested match arms
     .note = the matched value is of type `{$scrut_ty}` and the `non_exhaustive_omitted_patterns` attribute was found

compiler/rustc_pattern_analysis/src/errors.rs

@@ -43,6 +43,27 @@ impl<'tcx> Uncovered<'tcx> {
     }
 }
 
+#[derive(LintDiagnostic)]
+#[diag(pattern_analysis_empty_match_on_unsafe_place)]
+pub struct EmptyMatchOnUnsafePlace {
+    #[note]
+    pub scrut_span: Span,
+    #[subdiagnostic]
+    pub suggestion: EmptyMatchOnUnsafePlaceWrapSuggestion,
+}
+
+#[derive(Subdiagnostic)]
+#[multipart_suggestion(
+    pattern_analysis_empty_match_on_unsafe_place_wrap_suggestion,
+    applicability = "maybe-incorrect"
+)]
+pub struct EmptyMatchOnUnsafePlaceWrapSuggestion {
+    #[suggestion_part(code = "{{ ")]
+    pub scrut_start: Span,
+    #[suggestion_part(code = " }}")]
+    pub scrut_end: Span,
+}
+
 #[derive(LintDiagnostic)]
 #[diag(pattern_analysis_overlapping_range_endpoints)]
 #[note]

compiler/rustc_pattern_analysis/src/lib.rs

@@ -26,15 +26,7 @@ use rustc_index::Idx;
 use rustc_middle::ty::Ty;
 
 use crate::constructor::{Constructor, ConstructorSet};
-#[cfg(feature = "rustc")]
-use crate::lints::{
-    lint_nonexhaustive_missing_variants, lint_overlapping_range_endpoints, PatternColumn,
-};
 use crate::pat::DeconstructedPat;
-#[cfg(feature = "rustc")]
-use crate::rustc::RustcMatchCheckCtxt;
-#[cfg(feature = "rustc")]
-use crate::usefulness::{compute_match_usefulness, ValidityConstraint};
 
 // It's not possible to only enable the `typed_arena` dependency when the `rustc` feature is off, so
 // we use another feature instead. The crate won't compile if one of these isn't enabled.
@@ -110,26 +102,63 @@ impl<'p, Cx: TypeCx> Copy for MatchArm<'p, Cx> {}
 /// useful, and runs some lints.
 #[cfg(feature = "rustc")]
 pub fn analyze_match<'p, 'tcx>(
-    tycx: &RustcMatchCheckCtxt<'p, 'tcx>,
+    tycx: &rustc::RustcMatchCheckCtxt<'p, 'tcx>,
     arms: &[rustc::MatchArm<'p, 'tcx>],
     scrut_ty: Ty<'tcx>,
 ) -> rustc::UsefulnessReport<'p, 'tcx> {
+    use rustc_middle::ty;
+    use rustc_session::lint;
+
     // Arena to store the extra wildcards we construct during analysis.
     let wildcard_arena = tycx.pattern_arena;
-    let scrut_validity = ValidityConstraint::from_bool(tycx.known_valid_scrutinee);
+    let scrut_validity = usefulness::ValidityConstraint::from_bool(tycx.known_valid_scrutinee);
     let cx = MatchCtxt { tycx, wildcard_arena };
 
-    let report = compute_match_usefulness(cx, arms, scrut_ty, scrut_validity);
+    if !tycx.known_valid_scrutinee && arms.iter().all(|arm| arm.has_guard) {
+        let is_directly_empty = match scrut_ty.kind() {
+            ty::Adt(def, ..) => {
+                def.is_enum()
+                    && def.variants().is_empty()
+                    && !tycx.is_foreign_non_exhaustive_enum(scrut_ty)
+            }
+            ty::Never => true,
+            _ => false,
+        };
+        if is_directly_empty {
+            if tycx.tcx.features().min_exhaustive_patterns {
+                tycx.tcx.emit_spanned_lint(
+                    lint::builtin::EMPTY_MATCH_ON_UNSAFE_PLACE,
+                    tycx.match_lint_level,
+                    tycx.whole_match_span.unwrap_or(tycx.scrut_span),
+                    errors::EmptyMatchOnUnsafePlace {
+                        scrut_span: tycx.scrut_span,
+                        suggestion: errors::EmptyMatchOnUnsafePlaceWrapSuggestion {
+                            scrut_start: tycx.scrut_span.shrink_to_lo(),
+                            scrut_end: tycx.scrut_span.shrink_to_hi(),
+                        },
+                    },
+                );
+            }
+
+            // For backwards compability we allow an empty match in this case.
+            return rustc::UsefulnessReport {
+                arm_usefulness: Vec::new(),
+                non_exhaustiveness_witnesses: Vec::new(),
+            };
+        }
+    }
+
+    let report = usefulness::compute_match_usefulness(cx, arms, scrut_ty, scrut_validity);
 
-    let pat_column = PatternColumn::new(arms);
+    let pat_column = lints::PatternColumn::new(arms);
 
     // Lint on ranges that overlap on their endpoints, which is likely a mistake.
-    lint_overlapping_range_endpoints(cx, &pat_column);
+    lints::lint_overlapping_range_endpoints(cx, &pat_column);
 
     // Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hitting
     // `if let`s. Only run if the match is exhaustive otherwise the error is redundant.
     if tycx.refutable && report.non_exhaustiveness_witnesses.is_empty() {
-        lint_nonexhaustive_missing_variants(cx, arms, &pat_column, scrut_ty)
+        lints::lint_nonexhaustive_missing_variants(cx, arms, &pat_column, scrut_ty)
     }
 
     report

compiler/rustc_pattern_analysis/src/usefulness.rs

@@ -1199,23 +1199,19 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
 
     debug!("ty: {ty:?}");
     let pcx = &PlaceCtxt { mcx, ty, is_scrutinee: is_top_level };
-    let ctors_for_ty = pcx.ctors_for_ty();
-    let is_integers = matches!(ctors_for_ty, ConstructorSet::Integers { .. }); // For diagnostics.
 
     // Whether the place/column we are inspecting is known to contain valid data.
     let mut place_validity = matrix.place_validity[0];
-    if !mcx.tycx.is_min_exhaustive_patterns_feature_on()
-        || (is_top_level && matches!(ctors_for_ty, ConstructorSet::NoConstructors))
-    {
-        // For backwards compability we allow omitting some empty arms that we ideally shouldn't.
+    if mcx.tycx.is_exhaustive_patterns_feature_on() {
+        // Under `exhaustive_patterns` we allow omitting empty arms even when they aren't redundant.
         place_validity = place_validity.allow_omitting_side_effecting_arms();
     }
 
     // Analyze the constructors present in this column.
+    let ctors_for_ty = pcx.ctors_for_ty();
+    let is_integers = matches!(ctors_for_ty, ConstructorSet::Integers { .. }); // For diagnostics.
     let ctors = matrix.heads().map(|p| p.ctor());
     let split_set = ctors_for_ty.split(pcx, ctors);
-
-    // Decide what constructors to report.
     let all_missing = split_set.present.is_empty();
 
     // Build the set of constructors we will specialize with. It must cover the whole type.