Rollup merge of #128536 - Zalathar:print-cleanup, r=Nadrieril

Preliminary cleanup of `WitnessPat` hoisting/printing

Follow-up to #128430.

The eventual goal is to remove `print::Pat` entirely, but in the course of working towards that I made so many small improvements that it seems wise to let those be reviewed/merged on their own first.

Best reviewed commit-by-commit, most of which should be pretty simple and straightforward.

r? ``@Nadrieril``

compiler/rustc_pattern_analysis/src/lib.rs

@@ -5,6 +5,7 @@
 // tidy-alphabetical-start
 #![allow(rustc::diagnostic_outside_of_impl)]
 #![allow(rustc::untranslatable_diagnostic)]
+#![cfg_attr(feature = "rustc", feature(let_chains))]
 // tidy-alphabetical-end
 
 pub mod constructor;

compiler/rustc_pattern_analysis/src/rustc.rs

@@ -23,6 +23,7 @@ use crate::constructor::{
 };
 use crate::lints::lint_nonexhaustive_missing_variants;
 use crate::pat_column::PatternColumn;
+use crate::rustc::print::EnumInfo;
 use crate::usefulness::{compute_match_usefulness, PlaceValidity};
 use crate::{errors, Captures, PatCx, PrivateUninhabitedField};
 
@@ -824,77 +825,64 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
     fn hoist_witness_pat(&self, pat: &WitnessPat<'p, 'tcx>) -> print::Pat<'tcx> {
         use print::{FieldPat, Pat, PatKind};
         let cx = self;
-        let is_wildcard = |pat: &Pat<'_>| matches!(pat.kind, PatKind::Wild);
+        let hoist = |p| Box::new(cx.hoist_witness_pat(p));
-        let mut subpatterns = pat.iter_fields().map(|p| Box::new(cx.hoist_witness_pat(p)));
         let kind = match pat.ctor() {
             Bool(b) => PatKind::Constant { value: mir::Const::from_bool(cx.tcx, *b) },
             IntRange(range) => return self.hoist_pat_range(range, *pat.ty()),
-            Struct | Variant(_) | UnionField => match pat.ty().kind() {
+            Struct if pat.ty().is_box() => {
-                ty::Tuple(..) => PatKind::Leaf {
+                // Outside of the `alloc` crate, the only way to create a struct pattern
-                    subpatterns: subpatterns
+                // of type `Box` is to use a `box` pattern via #[feature(box_patterns)].
-                        .enumerate()
+                PatKind::Box { subpattern: hoist(&pat.fields[0]) }
-                        .map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern })
+            }
-                        .collect(),
+            Struct | Variant(_) | UnionField => {
-                },
+                let enum_info = match *pat.ty().kind() {
-                ty::Adt(adt_def, _) if adt_def.is_box() => {
+                    ty::Adt(adt_def, _) if adt_def.is_enum() => EnumInfo::Enum {
-                    // Without `box_patterns`, the only legal pattern of type `Box` is `_` (outside
+                        adt_def,
-                    // of `std`). So this branch is only reachable when the feature is enabled and
+                        variant_index: RustcPatCtxt::variant_index_for_adt(pat.ctor(), adt_def),
-                    // the pattern is a box pattern.
-                    PatKind::Deref { subpattern: subpatterns.next().unwrap() }
-                }
-                ty::Adt(adt_def, _args) => {
-                    let variant_index = RustcPatCtxt::variant_index_for_adt(&pat.ctor(), *adt_def);
-                    let subpatterns = subpatterns
-                        .enumerate()
-                        .map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern })
-                        .collect();
-
-                    if adt_def.is_enum() {
-                        PatKind::Variant { adt_def: *adt_def, variant_index, subpatterns }
-                    } else {
-                        PatKind::Leaf { subpatterns }
-                    }
-                }
-                _ => bug!("unexpected ctor for type {:?} {:?}", pat.ctor(), *pat.ty()),
-            },
-            // Note: given the expansion of `&str` patterns done in `expand_pattern`, we should
-            // be careful to reconstruct the correct constant pattern here. However a string
-            // literal pattern will never be reported as a non-exhaustiveness witness, so we
-            // ignore this issue.
-            Ref => PatKind::Deref { subpattern: subpatterns.next().unwrap() },
-            Slice(slice) => {
-                match slice.kind {
-                    SliceKind::FixedLen(_) => PatKind::Slice {
-                        prefix: subpatterns.collect(),
-                        slice: None,
-                        suffix: Box::new([]),
                     },
-                    SliceKind::VarLen(prefix, _) => {
+                    ty::Adt(..) | ty::Tuple(..) => EnumInfo::NotEnum,
-                        let mut subpatterns = subpatterns.peekable();
+                    _ => bug!("unexpected ctor for type {:?} {:?}", pat.ctor(), *pat.ty()),
-                        let mut prefix: Vec<_> = subpatterns.by_ref().take(prefix).collect();
+                };
-                        if slice.array_len.is_some() {
+
-                            // Improves diagnostics a bit: if the type is a known-size array, instead
+                let subpatterns = pat
-                            // of reporting `[x, _, .., _, y]`, we prefer to report `[x, .., y]`.
+                    .iter_fields()
-                            // This is incorrect if the size is not known, since `[_, ..]` captures
+                    .enumerate()
-                            // arrays of lengths `>= 1` whereas `[..]` captures any length.
+                    .map(|(i, pat)| FieldPat { field: FieldIdx::new(i), pattern: hoist(pat) })
-                            while !prefix.is_empty() && is_wildcard(prefix.last().unwrap()) {
+                    .collect::<Vec<_>>();
-                                prefix.pop();
+
-                            }
+                PatKind::StructLike { enum_info, subpatterns }
-                            while subpatterns.peek().is_some()
+            }
-                                && is_wildcard(subpatterns.peek().unwrap())
+            Ref => PatKind::Deref { subpattern: hoist(&pat.fields[0]) },
-                            {
+            Slice(slice) => {
-                                subpatterns.next();
+                let (prefix_len, has_dot_dot) = match slice.kind {
-                            }
+                    SliceKind::FixedLen(len) => (len, false),
-                        }
+                    SliceKind::VarLen(prefix_len, _) => (prefix_len, true),
-                        let suffix: Box<[_]> = subpatterns.collect();
+                };
-                        let wild = Pat { ty: pat.ty().inner(), kind: PatKind::Wild };
+
-                        PatKind::Slice {
+                let (mut prefix, mut suffix) = pat.fields.split_at(prefix_len);
-                            prefix: prefix.into_boxed_slice(),
+
-                            slice: Some(Box::new(wild)),
+                // If the pattern contains a `..`, but is applied to values of statically-known
-                            suffix,
+                // length (arrays), then we can slightly simplify diagnostics by merging any
-                        }
+                // adjacent wildcard patterns into the `..`: `[x, _, .., _, y]` => `[x, .., y]`.
+                // (This simplification isn't allowed for slice values, because in that case
+                // `[x, .., y]` would match some slices that `[x, _, .., _, y]` would not.)
+                if has_dot_dot && slice.array_len.is_some() {
+                    while let [rest @ .., last] = prefix
+                        && would_print_as_wildcard(cx.tcx, last)
+                    {
+                        prefix = rest;
+                    }
+                    while let [first, rest @ ..] = suffix
+                        && would_print_as_wildcard(cx.tcx, first)
+                    {
+                        suffix = rest;
                     }
                 }
+
+                let prefix = prefix.iter().map(hoist).collect();
+                let suffix = suffix.iter().map(hoist).collect();
+
+                PatKind::Slice { prefix, has_dot_dot, suffix }
             }
             &Str(value) => PatKind::Constant { value },
             Never if self.tcx.features().never_patterns => PatKind::Never,
@@ -912,6 +900,22 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
     }
 }
 
+/// Returns `true` if the given pattern would be printed as a wildcard (`_`).
+fn would_print_as_wildcard(tcx: TyCtxt<'_>, p: &WitnessPat<'_, '_>) -> bool {
+    match p.ctor() {
+        Constructor::IntRange(IntRange {
+            lo: MaybeInfiniteInt::NegInfinity,
+            hi: MaybeInfiniteInt::PosInfinity,
+        })
+        | Constructor::Wildcard
+        | Constructor::NonExhaustive
+        | Constructor::Hidden
+        | Constructor::PrivateUninhabited => true,
+        Constructor::Never if !tcx.features().never_patterns => true,
+        _ => false,
+    }
+}
+
 impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> {
     type Ty = RevealedTy<'tcx>;
     type Error = ErrorGuaranteed;

compiler/rustc_pattern_analysis/src/rustc/print.rs

@@ -12,7 +12,7 @@
 use std::fmt;
 
 use rustc_middle::thir::PatRange;
-use rustc_middle::ty::{self, AdtDef, Ty};
+use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt};
 use rustc_middle::{bug, mir};
 use rustc_span::sym;
 use rustc_target::abi::{FieldIdx, VariantIdx};
@@ -33,14 +33,13 @@ pub(crate) struct Pat<'tcx> {
 pub(crate) enum PatKind<'tcx> {
     Wild,
 
-    Variant {
+    StructLike {
-        adt_def: AdtDef<'tcx>,
+        enum_info: EnumInfo<'tcx>,
-        variant_index: VariantIdx,
         subpatterns: Vec<FieldPat<'tcx>>,
     },
 
-    Leaf {
+    Box {
-        subpatterns: Vec<FieldPat<'tcx>>,
+        subpattern: Box<Pat<'tcx>>,
     },
 
     Deref {
@@ -55,7 +54,9 @@ pub(crate) enum PatKind<'tcx> {
 
     Slice {
         prefix: Box<[Box<Pat<'tcx>>]>,
-        slice: Option<Box<Pat<'tcx>>>,
+        /// True if this slice-like pattern should include a `..` between the
+        /// prefix and suffix.
+        has_dot_dot: bool,
         suffix: Box<[Box<Pat<'tcx>>]>,
     },
 
@@ -64,130 +65,155 @@ pub(crate) enum PatKind<'tcx> {
 
 impl<'tcx> fmt::Display for Pat<'tcx> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // Printing lists is a chore.
-        let mut first = true;
-        let mut start_or_continue = |s| {
-            if first {
-                first = false;
-                ""
-            } else {
-                s
-            }
-        };
-        let mut start_or_comma = || start_or_continue(", ");
-
         match self.kind {
             PatKind::Wild => write!(f, "_"),
             PatKind::Never => write!(f, "!"),
-            PatKind::Variant { ref subpatterns, .. } | PatKind::Leaf { ref subpatterns } => {
+            PatKind::Box { ref subpattern } => write!(f, "box {subpattern}"),
-                let variant_and_name = match self.kind {
+            PatKind::StructLike { ref enum_info, ref subpatterns } => {
-                    PatKind::Variant { adt_def, variant_index, .. } => ty::tls::with(|tcx| {
+                ty::tls::with(|tcx| write_struct_like(f, tcx, self.ty, enum_info, subpatterns))
-                        let variant = adt_def.variant(variant_index);
-                        let adt_did = adt_def.did();
-                        let name = if tcx.get_diagnostic_item(sym::Option) == Some(adt_did)
-                            || tcx.get_diagnostic_item(sym::Result) == Some(adt_did)
-                        {
-                            variant.name.to_string()
-                        } else {
-                            format!("{}::{}", tcx.def_path_str(adt_def.did()), variant.name)
-                        };
-                        Some((variant, name))
-                    }),
-                    _ => self.ty.ty_adt_def().and_then(|adt_def| {
-                        if !adt_def.is_enum() {
-                            ty::tls::with(|tcx| {
-                                Some((adt_def.non_enum_variant(), tcx.def_path_str(adt_def.did())))
-                            })
-                        } else {
-                            None
-                        }
-                    }),
-                };
-
-                if let Some((variant, name)) = &variant_and_name {
-                    write!(f, "{name}")?;
-
-                    // Only for Adt we can have `S {...}`,
-                    // which we handle separately here.
-                    if variant.ctor.is_none() {
-                        write!(f, " {{ ")?;
-
-                        let mut printed = 0;
-                        for p in subpatterns {
-                            if let PatKind::Wild = p.pattern.kind {
-                                continue;
-                            }
-                            let name = variant.fields[p.field].name;
-                            write!(f, "{}{}: {}", start_or_comma(), name, p.pattern)?;
-                            printed += 1;
-                        }
-
-                        let is_union = self.ty.ty_adt_def().is_some_and(|adt| adt.is_union());
-                        if printed < variant.fields.len() && (!is_union || printed == 0) {
-                            write!(f, "{}..", start_or_comma())?;
-                        }
-
-                        return write!(f, " }}");
-                    }
-                }
-
-                let num_fields =
-                    variant_and_name.as_ref().map_or(subpatterns.len(), |(v, _)| v.fields.len());
-                if num_fields != 0 || variant_and_name.is_none() {
-                    write!(f, "(")?;
-                    for i in 0..num_fields {
-                        write!(f, "{}", start_or_comma())?;
-
-                        // Common case: the field is where we expect it.
-                        if let Some(p) = subpatterns.get(i) {
-                            if p.field.index() == i {
-                                write!(f, "{}", p.pattern)?;
-                                continue;
-                            }
-                        }
-
-                        // Otherwise, we have to go looking for it.
-                        if let Some(p) = subpatterns.iter().find(|p| p.field.index() == i) {
-                            write!(f, "{}", p.pattern)?;
-                        } else {
-                            write!(f, "_")?;
-                        }
-                    }
-                    write!(f, ")")?;
-                }
-
-                Ok(())
-            }
-            PatKind::Deref { ref subpattern } => {
-                match self.ty.kind() {
-                    ty::Adt(def, _) if def.is_box() => write!(f, "box ")?,
-                    ty::Ref(_, _, mutbl) => {
-                        write!(f, "&{}", mutbl.prefix_str())?;
-                    }
-                    _ => bug!("{} is a bad Deref pattern type", self.ty),
-                }
-                write!(f, "{subpattern}")
             }
+            PatKind::Deref { ref subpattern } => write_ref_like(f, self.ty, subpattern),
             PatKind::Constant { value } => write!(f, "{value}"),
             PatKind::Range(ref range) => write!(f, "{range}"),
-            PatKind::Slice { ref prefix, ref slice, ref suffix } => {
+            PatKind::Slice { ref prefix, has_dot_dot, ref suffix } => {
-                write!(f, "[")?;
+                write_slice_like(f, prefix, has_dot_dot, suffix)
-                for p in prefix.iter() {
-                    write!(f, "{}{}", start_or_comma(), p)?;
-                }
-                if let Some(ref slice) = *slice {
-                    write!(f, "{}", start_or_comma())?;
-                    match slice.kind {
-                        PatKind::Wild => {}
-                        _ => write!(f, "{slice}")?,
-                    }
-                    write!(f, "..")?;
-                }
-                for p in suffix.iter() {
-                    write!(f, "{}{}", start_or_comma(), p)?;
-                }
-                write!(f, "]")
             }
         }
     }
 }
+
+/// Returns a closure that will return `""` when called the first time,
+/// and then return `", "` when called any subsequent times.
+/// Useful for printing comma-separated lists.
+fn start_or_comma() -> impl FnMut() -> &'static str {
+    let mut first = true;
+    move || {
+        if first {
+            first = false;
+            ""
+        } else {
+            ", "
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub(crate) enum EnumInfo<'tcx> {
+    Enum { adt_def: AdtDef<'tcx>, variant_index: VariantIdx },
+    NotEnum,
+}
+
+fn write_struct_like<'tcx>(
+    f: &mut impl fmt::Write,
+    tcx: TyCtxt<'_>,
+    ty: Ty<'tcx>,
+    enum_info: &EnumInfo<'tcx>,
+    subpatterns: &[FieldPat<'tcx>],
+) -> fmt::Result {
+    let variant_and_name = match *enum_info {
+        EnumInfo::Enum { adt_def, variant_index } => {
+            let variant = adt_def.variant(variant_index);
+            let adt_did = adt_def.did();
+            let name = if tcx.is_diagnostic_item(sym::Option, adt_did)
+                || tcx.is_diagnostic_item(sym::Result, adt_did)
+            {
+                variant.name.to_string()
+            } else {
+                format!("{}::{}", tcx.def_path_str(adt_def.did()), variant.name)
+            };
+            Some((variant, name))
+        }
+        EnumInfo::NotEnum => ty.ty_adt_def().and_then(|adt_def| {
+            Some((adt_def.non_enum_variant(), tcx.def_path_str(adt_def.did())))
+        }),
+    };
+
+    let mut start_or_comma = start_or_comma();
+
+    if let Some((variant, name)) = &variant_and_name {
+        write!(f, "{name}")?;
+
+        // Only for Adt we can have `S {...}`,
+        // which we handle separately here.
+        if variant.ctor.is_none() {
+            write!(f, " {{ ")?;
+
+            let mut printed = 0;
+            for p in subpatterns {
+                if let PatKind::Wild = p.pattern.kind {
+                    continue;
+                }
+                let name = variant.fields[p.field].name;
+                write!(f, "{}{}: {}", start_or_comma(), name, p.pattern)?;
+                printed += 1;
+            }
+
+            let is_union = ty.ty_adt_def().is_some_and(|adt| adt.is_union());
+            if printed < variant.fields.len() && (!is_union || printed == 0) {
+                write!(f, "{}..", start_or_comma())?;
+            }
+
+            return write!(f, " }}");
+        }
+    }
+
+    let num_fields = variant_and_name.as_ref().map_or(subpatterns.len(), |(v, _)| v.fields.len());
+    if num_fields != 0 || variant_and_name.is_none() {
+        write!(f, "(")?;
+        for i in 0..num_fields {
+            write!(f, "{}", start_or_comma())?;
+
+            // Common case: the field is where we expect it.
+            if let Some(p) = subpatterns.get(i) {
+                if p.field.index() == i {
+                    write!(f, "{}", p.pattern)?;
+                    continue;
+                }
+            }
+
+            // Otherwise, we have to go looking for it.
+            if let Some(p) = subpatterns.iter().find(|p| p.field.index() == i) {
+                write!(f, "{}", p.pattern)?;
+            } else {
+                write!(f, "_")?;
+            }
+        }
+        write!(f, ")")?;
+    }
+
+    Ok(())
+}
+
+fn write_ref_like<'tcx>(
+    f: &mut impl fmt::Write,
+    ty: Ty<'tcx>,
+    subpattern: &Pat<'tcx>,
+) -> fmt::Result {
+    match ty.kind() {
+        ty::Ref(_, _, mutbl) => {
+            write!(f, "&{}", mutbl.prefix_str())?;
+        }
+        _ => bug!("{ty} is a bad ref pattern type"),
+    }
+    write!(f, "{subpattern}")
+}
+
+fn write_slice_like<'tcx>(
+    f: &mut impl fmt::Write,
+    prefix: &[Box<Pat<'tcx>>],
+    has_dot_dot: bool,
+    suffix: &[Box<Pat<'tcx>>],
+) -> fmt::Result {
+    let mut start_or_comma = start_or_comma();
+    write!(f, "[")?;
+    for p in prefix.iter() {
+        write!(f, "{}{}", start_or_comma(), p)?;
+    }
+    if has_dot_dot {
+        write!(f, "{}..", start_or_comma())?;
+    }
+    for p in suffix.iter() {
+        write!(f, "{}{}", start_or_comma(), p)?;
+    }
+    write!(f, "]")
+}