Implement lint as a separate columns-wise pass

Author: Nadrieril

compiler/rustc_lint_defs/src/builtin.rs

@@ -3798,8 +3798,10 @@ declare_lint! {
 }
 
 declare_lint! {
-    /// The `non_exhaustive_omitted_patterns` lint detects when a wildcard (`_` or `..`) in a
-    /// pattern for a `#[non_exhaustive]` struct or enum is reachable.
+    /// The `non_exhaustive_omitted_patterns` lint detects when some variants or fields of a
+    /// `#[non_exhaustive]` struct or enum are not mentioned explicitly in a pattern. This allows
+    /// downstream crates to be warned when new variants or fields are added to the upstream struct
+    /// or enum.
     ///
     /// ### Example
     ///
@@ -3827,8 +3829,8 @@ declare_lint! {
     /// warning: reachable patterns not covered of non exhaustive enum
     ///    --> $DIR/reachable-patterns.rs:70:9
     ///    |
-    /// LL |         _ => {}
-    ///    |         ^ pattern `B` not covered
+    /// LL |         match Bar::A {
+    ///    |               ^ pattern `Bar::B` not covered
     ///    |
     ///  note: the lint level is defined here
     ///   --> $DIR/reachable-patterns.rs:69:16
@@ -3841,12 +3843,11 @@ declare_lint! {
     ///
     /// ### Explanation
     ///
-    /// Structs and enums tagged with `#[non_exhaustive]` force the user to add a
-    /// (potentially redundant) wildcard when pattern-matching, to allow for future
-    /// addition of fields or variants. The `non_exhaustive_omitted_patterns` lint
-    /// detects when such a wildcard happens to actually catch some fields/variants.
-    /// In other words, when the match without the wildcard would not be exhaustive.
-    /// This lets the user be informed if new fields/variants were added.
+    /// Structs and enums tagged with `#[non_exhaustive]` force the user to add a (potentially
+    /// redundant) wildcard when pattern-matching, to allow for future addition of fields or
+    /// variants. The `non_exhaustive_omitted_patterns` lint detects when such a wildcard happens to
+    /// actually catch some fields/variants. This lets the user be informed if new fields/variants
+    /// were added.
     pub NON_EXHAUSTIVE_OMITTED_PATTERNS,
     Allow,
     "detect when patterns of types marked `non_exhaustive` are missed",

compiler/rustc_mir_build/src/thir/pattern/check_match.rs

@@ -250,7 +250,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
 
         let scrut = &self.thir[scrut];
         let scrut_ty = scrut.ty;
-        let report = compute_match_usefulness(&cx, &tarms, self.lint_level, scrut_ty);
+        let report = compute_match_usefulness(&cx, &tarms, self.lint_level, scrut_ty, scrut.span);
 
         match source {
             // Don't report arm reachability of desugared `match $iter.into_iter() { iter => .. }`
@@ -415,7 +415,8 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
         let pattern = self.lower_pattern(&mut cx, pat);
         let pattern_ty = pattern.ty();
         let arm = MatchArm { pat: pattern, hir_id: self.lint_level, has_guard: false };
-        let report = compute_match_usefulness(&cx, &[arm], self.lint_level, pattern_ty);
+        let report =
+            compute_match_usefulness(&cx, &[arm], self.lint_level, pattern_ty, pattern.span());
 
         // Note: we ignore whether the pattern is unreachable (i.e. whether the type is empty). We
         // only care about exhaustiveness here.
@@ -583,7 +584,7 @@ fn is_let_irrefutable<'p, 'tcx>(
     pat: &'p DeconstructedPat<'p, 'tcx>,
 ) -> bool {
     let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }];
-    let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty());
+    let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty(), pat.span());
 
     // Report if the pattern is unreachable, which can only occur when the type is uninhabited.
     // This also reports unreachable sub-patterns though, so we can't just replace it with an

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs

@@ -643,9 +643,8 @@ pub(super) enum Constructor<'tcx> {
     /// for those types for which we cannot list constructors explicitly, like `f64` and `str`.
     NonExhaustive,
     /// Stands for constructors that are not seen in the matrix, as explained in the documentation
-    /// for [`SplitWildcard`]. The carried `bool` is used for the `non_exhaustive_omitted_patterns`
-    /// lint.
-    Missing { nonexhaustive_enum_missing_real_variants: bool },
+    /// for [`SplitWildcard`].
+    Missing,
     /// Wildcard pattern.
     Wildcard,
     /// Or-pattern.
@@ -1053,6 +1052,16 @@ impl<'tcx> SplitWildcard<'tcx> {
         self.all_ctors.iter().filter(move |ctor| !ctor.is_covered_by_any(pcx, &self.matrix_ctors))
     }
 
+    /// Iterate over the constructors for this type that are present in the matrix. This has the
+    /// effect of deduplicating present constructors.
+    /// WARNING: this omits special constructors like `Wildcard` and `Opaque`.
+    pub(super) fn iter_present<'a, 'p>(
+        &'a self,
+        pcx: &'a PatCtxt<'a, 'p, 'tcx>,
+    ) -> impl Iterator<Item = &'a Constructor<'tcx>> + Captures<'p> {
+        self.all_ctors.iter().filter(move |ctor| ctor.is_covered_by_any(pcx, &self.matrix_ctors))
+    }
+
     /// Return the set of constructors resulting from splitting the wildcard. As explained at the
     /// top of the file, if any constructors are missing we can ignore the present ones.
     fn into_ctors(self, pcx: &PatCtxt<'_, '_, 'tcx>) -> SmallVec<[Constructor<'tcx>; 1]> {
@@ -1086,15 +1095,7 @@ impl<'tcx> SplitWildcard<'tcx> {
             // sometimes prefer reporting the list of constructors instead of just `_`.
             let report_when_all_missing = pcx.is_top_level && !IntRange::is_integral(pcx.ty);
             let ctor = if !self.matrix_ctors.is_empty() || report_when_all_missing {
-                if pcx.is_non_exhaustive {
-                    Missing {
-                        nonexhaustive_enum_missing_real_variants: self
-                            .iter_missing(pcx)
-                            .any(|c| !(c.is_non_exhaustive() || c.is_unstable_variant(pcx))),
-                    }
-                } else {
-                    Missing { nonexhaustive_enum_missing_real_variants: false }
-                }
+                Missing
             } else {
                 Wildcard
             };
@@ -1240,9 +1241,10 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
 
 /// Values and patterns can be represented as a constructor applied to some fields. This represents
 /// a pattern in this form.
-/// This also keeps track of whether the pattern has been found reachable during analysis. For this
-/// reason we should be careful not to clone patterns for which we care about that. Use
-/// `clone_and_forget_reachability` if you're sure.
+/// This also uses interior mutability to keep track of whether the pattern has been found reachable
+/// during analysis. For this reason we should be careful not to clone patterns for which we care
+/// about that.
+#[derive(Clone)]
 pub(crate) struct DeconstructedPat<'p, 'tcx> {
     ctor: Constructor<'tcx>,
     fields: Fields<'p, 'tcx>,
@@ -1273,12 +1275,6 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
         DeconstructedPat::new(ctor, fields, pcx.ty, pcx.span)
     }
 
-    /// Clone this value. This method emphasizes that cloning loses reachability information and
-    /// should be done carefully.
-    pub(super) fn clone_and_forget_reachability(&self) -> Self {
-        DeconstructedPat::new(self.ctor.clone(), self.fields, self.ty, self.span)
-    }
-
     pub(crate) fn from_pat(cx: &MatchCheckCtxt<'p, 'tcx>, pat: &Pat<'tcx>) -> Self {
         let mkpat = |pat| DeconstructedPat::from_pat(cx, pat);
         let ctor;
@@ -1522,6 +1518,13 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
     pub(super) fn is_or_pat(&self) -> bool {
         matches!(self.ctor, Or)
     }
+    pub(super) fn flatten_or_pat(&'p self) -> SmallVec<[&'p Self; 1]> {
+        if self.is_or_pat() {
+            self.iter_fields().flat_map(|p| p.flatten_or_pat()).collect()
+        } else {
+            smallvec![self]
+        }
+    }
 
     pub(super) fn ctor(&self) -> &Constructor<'tcx> {
         &self.ctor

compiler/rustc_mir_build/src/thir/pattern/usefulness.rs

@@ -642,14 +642,7 @@ impl<'p, 'tcx> Usefulness<'p, 'tcx> {
                         .into_iter()
                         .flat_map(|witness| {
                             new_patterns.iter().map(move |pat| {
-                                Witness(
-                                    witness
-                                        .0
-                                        .iter()
-                                        .chain(once(pat))
-                                        .map(DeconstructedPat::clone_and_forget_reachability)
-                                        .collect(),
-                                )
+                                Witness(witness.0.iter().chain(once(pat)).cloned().collect())
                             })
                         })
                         .collect()
@@ -843,7 +836,6 @@ fn is_useful<'p, 'tcx>(
         }
         // We split the head constructor of `v`.
         let split_ctors = v_ctor.split(pcx, matrix.heads().map(DeconstructedPat::ctor));
-        let is_non_exhaustive_and_wild = is_non_exhaustive && v_ctor.is_wildcard();
         // For each constructor, we compute whether there's a value that starts with it that would
         // witness the usefulness of `v`.
         let start_matrix = &matrix;
@@ -864,56 +856,6 @@ fn is_useful<'p, 'tcx>(
                 )
             });
             let usefulness = usefulness.apply_constructor(pcx, start_matrix, &ctor);
-
-            // When all the conditions are met we have a match with a `non_exhaustive` enum
-            // that has the potential to trigger the `non_exhaustive_omitted_patterns` lint.
-            // To understand the workings checkout `Constructor::split` and `SplitWildcard::new/into_ctors`
-            if is_non_exhaustive_and_wild
-                // Only emit a lint on refutable patterns.
-                && cx.refutable
-                // We check that the match has a wildcard pattern and that wildcard is useful,
-                // meaning there are variants that are covered by the wildcard. Without the check
-                // for `witness_preference` the lint would trigger on `if let NonExhaustiveEnum::A = foo {}`
-                && usefulness.is_useful() && matches!(witness_preference, RealArm)
-                && matches!(
-                    &ctor,
-                    Constructor::Missing { nonexhaustive_enum_missing_real_variants: true }
-                )
-            {
-                let patterns = {
-                    let mut split_wildcard = SplitWildcard::new(pcx);
-                    split_wildcard.split(pcx, matrix.heads().map(DeconstructedPat::ctor));
-                    // Construct for each missing constructor a "wild" version of this
-                    // constructor, that matches everything that can be built with
-                    // it. For example, if `ctor` is a `Constructor::Variant` for
-                    // `Option::Some`, we get the pattern `Some(_)`.
-                    split_wildcard
-                        .iter_missing(pcx)
-                        // Filter out the `NonExhaustive` because we want to list only real
-                        // variants. Also remove any unstable feature gated variants.
-                        // Because of how we computed `nonexhaustive_enum_missing_real_variants`,
-                        // this will not return an empty `Vec`.
-                        .filter(|c| !(c.is_non_exhaustive() || c.is_unstable_variant(pcx)))
-                        .cloned()
-                        .map(|missing_ctor| DeconstructedPat::wild_from_ctor(pcx, missing_ctor))
-                        .collect::<Vec<_>>()
-                };
-
-                // Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
-                // is not exhaustive enough.
-                //
-                // NB: The partner lint for structs lives in `compiler/rustc_hir_analysis/src/check/pat.rs`.
-                cx.tcx.emit_spanned_lint(
-                    NON_EXHAUSTIVE_OMITTED_PATTERNS,
-                    lint_root,
-                    pcx.span,
-                    NonExhaustiveOmittedPattern {
-                        scrut_ty: pcx.ty,
-                        uncovered: Uncovered::new(pcx.span, pcx.cx, patterns),
-                    },
-                );
-            }
-
             ret.extend(usefulness);
         }
     }
@@ -925,6 +867,96 @@ fn is_useful<'p, 'tcx>(
     ret
 }
 
+/// Traverse the patterns to collect any variants of a non_exhaustive enum that fail to be mentioned
+/// in a given column. This traverses patterns column-by-column, where a column is the intuitive
+/// notion of "subpatterns that inspect the same subvalue".
+/// Despite similarities with `is_useful`, this traversal is different. Notably this is linear in the
+/// depth of patterns, whereas `is_useful` is worst-case exponential (exhaustiveness is NP-complete).
+fn collect_nonexhaustive_missing_variants<'p, 'tcx>(
+    cx: &MatchCheckCtxt<'p, 'tcx>,
+    column: &[&DeconstructedPat<'p, 'tcx>],
+) -> Vec<DeconstructedPat<'p, 'tcx>> {
+    let mut witnesses = Vec::new();
+    // If the column is all wildcards, we don't want to collect anything and we can't recurse
+    // further.
+    let is_all_wildcards = column.iter().all(|p| p.ctor().is_wildcard());
+    if column.is_empty() || is_all_wildcards {
+        return witnesses;
+    }
+
+    let ty = column[0].ty();
+    let span = column[0].span();
+    let is_non_exhaustive = cx.is_foreign_non_exhaustive_enum(ty);
+    let pcx = &PatCtxt { cx, ty, span, is_top_level: false, is_non_exhaustive };
+
+    let mut split_wildcard = SplitWildcard::new(pcx);
+    split_wildcard.split(pcx, column.iter().map(|p| p.ctor()));
+
+    if is_non_exhaustive {
+        witnesses.extend(
+            split_wildcard
+                .iter_missing(pcx)
+                // Filter out the `NonExhaustive` ctor because we want to list only real variants.
+                // Also remove any unstable feature gated variants.
+                .filter(|c| !(c.is_non_exhaustive() || c.is_unstable_variant(pcx)))
+                .cloned()
+                .map(|missing_ctor| DeconstructedPat::wild_from_ctor(pcx, missing_ctor)),
+        )
+    }
+
+    // Recurse into the fields.
+    for ctor in split_wildcard.iter_present(pcx) {
+        let arity = ctor.arity(pcx);
+        if arity == 0 {
+            continue;
+        }
+
+        // We specialize the column by `ctor`. This gives us `arity`-many columns of patterns. These
+        // columns may not have the same length in the presence of or-patterns (this is why we can't
+        // reuse what `Matrix` does).
+        let mut specialized_columns: Vec<Vec<_>> = (0..arity).map(|_| Vec::new()).collect();
+        let relevant_patterns = column.iter().filter(|pat| ctor.is_covered_by(pcx, pat.ctor()));
+        for pat in relevant_patterns {
+            let specialized = pat.specialize(pcx, &ctor);
+            for (subpat, sub_column) in specialized.iter().zip(&mut specialized_columns) {
+                if subpat.is_or_pat() {
+                    sub_column.extend(subpat.iter_fields())
+                } else {
+                    sub_column.push(subpat)
+                }
+            }
+        }
+        assert!(
+            !specialized_columns[0].is_empty(),
+            "ctor {ctor:?} was listed as present but isn't"
+        );
+
+        for (i, col_i) in specialized_columns.iter().enumerate() {
+            // Compute witnesses for each column.
+            let wits_for_col_i = collect_nonexhaustive_missing_variants(cx, col_i.as_slice());
+            if wits_for_col_i.is_empty() {
+                continue;
+            }
+            // For each witness, we build a new pattern in the shape of `ctor(_, _, wit, _, _)`,
+            // adding enough wildcards to match `arity`.
+            let fields = Fields::wildcards(pcx, &ctor);
+            for wit in wits_for_col_i {
+                let fields_with_wit = Fields::from_iter(
+                    cx,
+                    fields
+                        .iter_patterns()
+                        .enumerate()
+                        .map(|(j, wild)| if i == j { &wit } else { wild })
+                        .cloned(),
+                );
+                let pat = DeconstructedPat::new(ctor.clone(), fields_with_wit, ty, DUMMY_SP);
+                witnesses.push(pat);
+            }
+        }
+    }
+    witnesses
+}
+
 /// The arm of a match expression.
 #[derive(Clone, Copy, Debug)]
 pub(crate) struct MatchArm<'p, 'tcx> {
@@ -965,6 +997,7 @@ pub(crate) fn compute_match_usefulness<'p, 'tcx>(
     arms: &[MatchArm<'p, 'tcx>],
     lint_root: HirId,
     scrut_ty: Ty<'tcx>,
+    scrut_span: Span,
 ) -> UsefulnessReport<'p, 'tcx> {
     let mut matrix = Matrix::empty();
     let arm_usefulness: Vec<_> = arms
@@ -989,9 +1022,39 @@ pub(crate) fn compute_match_usefulness<'p, 'tcx>(
     let wild_pattern = cx.pattern_arena.alloc(DeconstructedPat::wildcard(scrut_ty, DUMMY_SP));
     let v = PatStack::from_pattern(wild_pattern);
     let usefulness = is_useful(cx, &matrix, &v, FakeExtraWildcard, lint_root, false, true);
-    let non_exhaustiveness_witnesses = match usefulness {
+    let non_exhaustiveness_witnesses: Vec<_> = match usefulness {
         WithWitnesses(pats) => pats.into_iter().map(|w| w.single_pattern()).collect(),
         NoWitnesses { .. } => bug!(),
     };
+
+    // Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hittin
+    // `if let`s. Only run if the match is exhaustive otherwise the error is redundant.
+    if cx.refutable
+        && non_exhaustiveness_witnesses.is_empty()
+        && !matches!(
+            cx.tcx.lint_level_at_node(NON_EXHAUSTIVE_OMITTED_PATTERNS, lint_root).0,
+            rustc_session::lint::Level::Allow
+        )
+    {
+        let pat_column = arms.iter().flat_map(|arm| arm.pat.flatten_or_pat()).collect::<Vec<_>>();
+        let witnesses = collect_nonexhaustive_missing_variants(cx, &pat_column);
+
+        if !witnesses.is_empty() {
+            // Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
+            // is not exhaustive enough.
+            //
+            // NB: The partner lint for structs lives in `compiler/rustc_hir_analysis/src/check/pat.rs`.
+            cx.tcx.emit_spanned_lint(
+                NON_EXHAUSTIVE_OMITTED_PATTERNS,
+                lint_root,
+                scrut_span,
+                NonExhaustiveOmittedPattern {
+                    scrut_ty,
+                    uncovered: Uncovered::new(scrut_span, cx, witnesses),
+                },
+            );
+        }
+    }
+
     UsefulnessReport { arm_usefulness, non_exhaustiveness_witnesses }
 }