From ccfd94e33473b610127aec56ab72c9eef4a9d1c8 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 19:55:16 +1000 Subject: [PATCH 01/11] Break up `print::Pat` printing into several helper functions --- .../rustc_pattern_analysis/src/rustc/print.rs | 255 ++++++++++-------- 1 file changed, 142 insertions(+), 113 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc/print.rs b/compiler/rustc_pattern_analysis/src/rustc/print.rs index 4b76764e8b136..59e6b1480c701 100644 --- a/compiler/rustc_pattern_analysis/src/rustc/print.rs +++ b/compiler/rustc_pattern_analysis/src/rustc/print.rs @@ -64,130 +64,159 @@ 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 } => { - let variant_and_name = match self.kind { - PatKind::Variant { adt_def, variant_index, .. } => ty::tls::with(|tcx| { - 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}") + write_struct_like(f, self.ty, &self.kind, subpatterns) } + 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 } => { - write!(f, "[")?; - 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, "..")?; + write_slice_like(f, prefix, slice, suffix) + } + } + } +} + +/// 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 { + ", " + } + } +} + +fn write_struct_like<'tcx>( + f: &mut impl fmt::Write, + ty: Ty<'tcx>, + kind: &PatKind<'tcx>, + subpatterns: &[FieldPat<'tcx>], +) -> fmt::Result { + let variant_and_name = match *kind { + PatKind::Variant { adt_def, variant_index, .. } => ty::tls::with(|tcx| { + 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)) + }), + _ => 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 + } + }), + }; + + 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; } - for p in suffix.iter() { - write!(f, "{}{}", start_or_comma(), p)?; + 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; } - write!(f, "]") + } + + // 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::Adt(def, _) if def.is_box() => write!(f, "box ")?, + 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>], + slice: &Option>>, + suffix: &[Box>], +) -> fmt::Result { + let mut start_or_comma = start_or_comma(); + write!(f, "[")?; + 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, "]") } From 74f76ae5ea1a9fcd0b3d7adbac93b98237e4063e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 20:05:44 +1000 Subject: [PATCH 02/11] Unify `Variant` and `Leaf` into `print::PatKind::StructLike` --- compiler/rustc_pattern_analysis/src/rustc.rs | 18 ++++++++----- .../rustc_pattern_analysis/src/rustc/print.rs | 27 ++++++++++--------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 6290aeb252312..7fe2c14a74588 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/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}; @@ -832,7 +833,8 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { 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() { - ty::Tuple(..) => PatKind::Leaf { + ty::Tuple(..) => PatKind::StructLike { + enum_info: EnumInfo::NotEnum, subpatterns: subpatterns .enumerate() .map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern }) @@ -844,18 +846,20 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { // 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); + &ty::Adt(adt_def, _) => { + 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 } + let enum_info = if adt_def.is_enum() { + EnumInfo::Enum { adt_def, variant_index } } else { - PatKind::Leaf { subpatterns } - } + EnumInfo::NotEnum + }; + + PatKind::StructLike { enum_info, subpatterns } } _ => bug!("unexpected ctor for type {:?} {:?}", pat.ctor(), *pat.ty()), }, diff --git a/compiler/rustc_pattern_analysis/src/rustc/print.rs b/compiler/rustc_pattern_analysis/src/rustc/print.rs index 59e6b1480c701..b85c15d83920b 100644 --- a/compiler/rustc_pattern_analysis/src/rustc/print.rs +++ b/compiler/rustc_pattern_analysis/src/rustc/print.rs @@ -33,13 +33,8 @@ pub(crate) struct Pat<'tcx> { pub(crate) enum PatKind<'tcx> { Wild, - Variant { - adt_def: AdtDef<'tcx>, - variant_index: VariantIdx, - subpatterns: Vec>, - }, - - Leaf { + StructLike { + enum_info: EnumInfo<'tcx>, subpatterns: Vec>, }, @@ -67,8 +62,8 @@ impl<'tcx> fmt::Display for Pat<'tcx> { match self.kind { PatKind::Wild => write!(f, "_"), PatKind::Never => write!(f, "!"), - PatKind::Variant { ref subpatterns, .. } | PatKind::Leaf { ref subpatterns } => { - write_struct_like(f, self.ty, &self.kind, subpatterns) + PatKind::StructLike { ref enum_info, ref subpatterns } => { + write_struct_like(f, self.ty, enum_info, subpatterns) } PatKind::Deref { ref subpattern } => write_ref_like(f, self.ty, subpattern), PatKind::Constant { value } => write!(f, "{value}"), @@ -95,14 +90,20 @@ fn start_or_comma() -> impl FnMut() -> &'static str { } } +#[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, ty: Ty<'tcx>, - kind: &PatKind<'tcx>, + enum_info: &EnumInfo<'tcx>, subpatterns: &[FieldPat<'tcx>], ) -> fmt::Result { - let variant_and_name = match *kind { - PatKind::Variant { adt_def, variant_index, .. } => ty::tls::with(|tcx| { + let variant_and_name = match *enum_info { + EnumInfo::Enum { adt_def, variant_index } => ty::tls::with(|tcx| { 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) @@ -114,7 +115,7 @@ fn write_struct_like<'tcx>( }; Some((variant, name)) }), - _ => ty.ty_adt_def().and_then(|adt_def| { + EnumInfo::NotEnum => 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()))) From 4cd800503f0c0ba3c26f30c98aff755be6b9be43 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 20:10:17 +1000 Subject: [PATCH 03/11] Remove an impossible case under `EnumInfo::NotEnum` --- compiler/rustc_pattern_analysis/src/rustc/print.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc/print.rs b/compiler/rustc_pattern_analysis/src/rustc/print.rs index b85c15d83920b..d14233ee0242b 100644 --- a/compiler/rustc_pattern_analysis/src/rustc/print.rs +++ b/compiler/rustc_pattern_analysis/src/rustc/print.rs @@ -116,13 +116,7 @@ fn write_struct_like<'tcx>( Some((variant, name)) }), EnumInfo::NotEnum => 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 - } + ty::tls::with(|tcx| Some((adt_def.non_enum_variant(), tcx.def_path_str(adt_def.did())))) }), }; From e98e19e49108b6e49e452c3bd22c1aeccd43e507 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 20:09:12 +1000 Subject: [PATCH 04/11] Replace an unnecessary slice pattern with `has_dot_dot: bool` --- compiler/rustc_pattern_analysis/src/rustc.rs | 5 ++--- .../rustc_pattern_analysis/src/rustc/print.rs | 19 ++++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 7fe2c14a74588..f27c279912f78 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -872,7 +872,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { match slice.kind { SliceKind::FixedLen(_) => PatKind::Slice { prefix: subpatterns.collect(), - slice: None, + has_dot_dot: false, suffix: Box::new([]), }, SliceKind::VarLen(prefix, _) => { @@ -893,10 +893,9 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { } } let suffix: Box<[_]> = subpatterns.collect(); - let wild = Pat { ty: pat.ty().inner(), kind: PatKind::Wild }; PatKind::Slice { prefix: prefix.into_boxed_slice(), - slice: Some(Box::new(wild)), + has_dot_dot: true, suffix, } } diff --git a/compiler/rustc_pattern_analysis/src/rustc/print.rs b/compiler/rustc_pattern_analysis/src/rustc/print.rs index d14233ee0242b..e7568b9e2bd3e 100644 --- a/compiler/rustc_pattern_analysis/src/rustc/print.rs +++ b/compiler/rustc_pattern_analysis/src/rustc/print.rs @@ -50,7 +50,9 @@ pub(crate) enum PatKind<'tcx> { Slice { prefix: Box<[Box>]>, - slice: Option>>, + /// True if this slice-like pattern should include a `..` between the + /// prefix and suffix. + has_dot_dot: bool, suffix: Box<[Box>]>, }, @@ -68,8 +70,8 @@ impl<'tcx> fmt::Display for Pat<'tcx> { 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 } => { - write_slice_like(f, prefix, slice, suffix) + PatKind::Slice { ref prefix, has_dot_dot, ref suffix } => { + write_slice_like(f, prefix, has_dot_dot, suffix) } } } @@ -194,7 +196,7 @@ fn write_ref_like<'tcx>( fn write_slice_like<'tcx>( f: &mut impl fmt::Write, prefix: &[Box>], - slice: &Option>>, + has_dot_dot: bool, suffix: &[Box>], ) -> fmt::Result { let mut start_or_comma = start_or_comma(); @@ -202,13 +204,8 @@ fn write_slice_like<'tcx>( 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, "..")?; + if has_dot_dot { + write!(f, "{}..", start_or_comma())?; } for p in suffix.iter() { write!(f, "{}{}", start_or_comma(), p)?; From 7f48851416c0faf0333a23ba37904f4a407c723b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 1 Aug 2024 20:49:52 +1000 Subject: [PATCH 05/11] Split out a `hoist` helper in `hoist_witness_pat` --- compiler/rustc_pattern_analysis/src/rustc.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index f27c279912f78..9fa5222452ea0 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -828,7 +828,8 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { use print::{FieldPat, Pat, PatKind}; let cx = self; let is_wildcard = |pat: &Pat<'_>| matches!(pat.kind, PatKind::Wild); - let mut subpatterns = pat.iter_fields().map(|p| Box::new(cx.hoist_witness_pat(p))); + let hoist = |p| Box::new(cx.hoist_witness_pat(p)); + let mut subpatterns = pat.iter_fields().map(hoist); 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()), From a5ed6fb646f554a3e2071dfdd3a3de2f83b759ec Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 1 Aug 2024 20:29:06 +1000 Subject: [PATCH 06/11] Split out hoisting/printing of `box` patterns --- compiler/rustc_pattern_analysis/src/rustc.rs | 11 +++++------ compiler/rustc_pattern_analysis/src/rustc/print.rs | 6 +++++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 9fa5222452ea0..7eee5220ce4dd 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -833,6 +833,11 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { 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 if pat.ty().is_box() => { + // Outside of the `alloc` crate, the only way to create a struct pattern + // of type `Box` is to use a `box` pattern via #[feature(box_patterns)]. + PatKind::Box { subpattern: hoist(&pat.fields[0]) } + } Struct | Variant(_) | UnionField => match pat.ty().kind() { ty::Tuple(..) => PatKind::StructLike { enum_info: EnumInfo::NotEnum, @@ -841,12 +846,6 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { .map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern }) .collect(), }, - ty::Adt(adt_def, _) if adt_def.is_box() => { - // Without `box_patterns`, the only legal pattern of type `Box` is `_` (outside - // of `std`). So this branch is only reachable when the feature is enabled and - // the pattern is a box pattern. - PatKind::Deref { subpattern: subpatterns.next().unwrap() } - } &ty::Adt(adt_def, _) => { let variant_index = RustcPatCtxt::variant_index_for_adt(&pat.ctor(), adt_def); let subpatterns = subpatterns diff --git a/compiler/rustc_pattern_analysis/src/rustc/print.rs b/compiler/rustc_pattern_analysis/src/rustc/print.rs index e7568b9e2bd3e..64f041dc21a76 100644 --- a/compiler/rustc_pattern_analysis/src/rustc/print.rs +++ b/compiler/rustc_pattern_analysis/src/rustc/print.rs @@ -38,6 +38,10 @@ pub(crate) enum PatKind<'tcx> { subpatterns: Vec>, }, + Box { + subpattern: Box>, + }, + Deref { subpattern: Box>, }, @@ -64,6 +68,7 @@ impl<'tcx> fmt::Display for Pat<'tcx> { match self.kind { PatKind::Wild => write!(f, "_"), PatKind::Never => write!(f, "!"), + PatKind::Box { ref subpattern } => write!(f, "box {subpattern}"), PatKind::StructLike { ref enum_info, ref subpatterns } => { write_struct_like(f, self.ty, enum_info, subpatterns) } @@ -184,7 +189,6 @@ fn write_ref_like<'tcx>( subpattern: &Pat<'tcx>, ) -> fmt::Result { match ty.kind() { - ty::Adt(def, _) if def.is_box() => write!(f, "box ")?, ty::Ref(_, _, mutbl) => { write!(f, "&{}", mutbl.prefix_str())?; } From c764bea0c3d8ca9b9fc40b032376d5a1ea1e9096 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 20:28:56 +1000 Subject: [PATCH 07/11] Simplify hoisting of struct-like patterns --- compiler/rustc_pattern_analysis/src/rustc.rs | 39 ++++++++------------ 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 7eee5220ce4dd..78355d6c762b5 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -838,31 +838,24 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { // of type `Box` is to use a `box` pattern via #[feature(box_patterns)]. PatKind::Box { subpattern: hoist(&pat.fields[0]) } } - Struct | Variant(_) | UnionField => match pat.ty().kind() { - ty::Tuple(..) => PatKind::StructLike { - enum_info: EnumInfo::NotEnum, - subpatterns: subpatterns - .enumerate() - .map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern }) - .collect(), - }, - &ty::Adt(adt_def, _) => { - 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(); + Struct | Variant(_) | UnionField => { + let enum_info = match *pat.ty().kind() { + ty::Adt(adt_def, _) if adt_def.is_enum() => EnumInfo::Enum { + adt_def, + variant_index: RustcPatCtxt::variant_index_for_adt(pat.ctor(), adt_def), + }, + ty::Adt(..) | ty::Tuple(..) => EnumInfo::NotEnum, + _ => bug!("unexpected ctor for type {:?} {:?}", pat.ctor(), *pat.ty()), + }; - let enum_info = if adt_def.is_enum() { - EnumInfo::Enum { adt_def, variant_index } - } else { - EnumInfo::NotEnum - }; + let subpatterns = pat + .iter_fields() + .enumerate() + .map(|(i, pat)| FieldPat { field: FieldIdx::new(i), pattern: hoist(pat) }) + .collect::>(); - PatKind::StructLike { enum_info, subpatterns } - } - _ => bug!("unexpected ctor for type {:?} {:?}", pat.ctor(), *pat.ty()), - }, + PatKind::StructLike { enum_info, subpatterns } + } // 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 From a245bfa61792cfa4c9cf29adae7dbe1b833f705c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 20:55:53 +1000 Subject: [PATCH 08/11] Simplify hoisting of array/slice patterns We can replace some tricky iterator-mutation code with a much simpler version that uses `while let` to shrink a slice. We also check whether a subpattern would be a wildcard _before_ hoisting it, which will be very useful when trying to get rid of `print::PatKind` later. --- compiler/rustc_pattern_analysis/src/lib.rs | 1 + compiler/rustc_pattern_analysis/src/rustc.rs | 73 ++++++++++++-------- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index a5c0b13c90be1..4c8c865e721ae 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/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; diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 78355d6c762b5..bb882e79647a4 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -827,7 +827,6 @@ 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(hoist); let kind = match pat.ctor() { @@ -862,37 +861,35 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { // ignore this issue. Ref => PatKind::Deref { subpattern: subpatterns.next().unwrap() }, Slice(slice) => { - match slice.kind { - SliceKind::FixedLen(_) => PatKind::Slice { - prefix: subpatterns.collect(), - has_dot_dot: false, - suffix: Box::new([]), - }, - SliceKind::VarLen(prefix, _) => { - let mut subpatterns = subpatterns.peekable(); - 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 - // of reporting `[x, _, .., _, y]`, we prefer to report `[x, .., y]`. - // This is incorrect if the size is not known, since `[_, ..]` captures - // arrays of lengths `>= 1` whereas `[..]` captures any length. - while !prefix.is_empty() && is_wildcard(prefix.last().unwrap()) { - prefix.pop(); - } - while subpatterns.peek().is_some() - && is_wildcard(subpatterns.peek().unwrap()) - { - subpatterns.next(); - } - } - let suffix: Box<[_]> = subpatterns.collect(); - PatKind::Slice { - prefix: prefix.into_boxed_slice(), - has_dot_dot: true, - suffix, - } + let (prefix_len, has_dot_dot) = match slice.kind { + SliceKind::FixedLen(len) => (len, false), + SliceKind::VarLen(prefix_len, _) => (prefix_len, true), + }; + + let (mut prefix, mut suffix) = pat.fields.split_at(prefix_len); + + // If the pattern contains a `..`, but is applied to values of statically-known + // 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, @@ -910,6 +907,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; From 582208b0f26dd448b7a869f3a3e11ad07cd76346 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 20:59:25 +1000 Subject: [PATCH 09/11] Simplify hoisting of ref patterns (`&` and `&mut`) --- compiler/rustc_pattern_analysis/src/rustc.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index bb882e79647a4..2ddd35923bccc 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -828,7 +828,6 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { use print::{FieldPat, Pat, PatKind}; let cx = self; let hoist = |p| Box::new(cx.hoist_witness_pat(p)); - let mut subpatterns = pat.iter_fields().map(hoist); 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()), @@ -855,11 +854,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { PatKind::StructLike { enum_info, subpatterns } } - // 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() }, + Ref => PatKind::Deref { subpattern: hoist(&pat.fields[0]) }, Slice(slice) => { let (prefix_len, has_dot_dot) = match slice.kind { SliceKind::FixedLen(len) => (len, false), From 29245ec75914a32fcb33ba5282f73d6d741ab2f7 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 21:12:04 +1000 Subject: [PATCH 10/11] Avoid using `ty::tls::with` in `write_struct_like` --- compiler/rustc_pattern_analysis/src/rustc/print.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc/print.rs b/compiler/rustc_pattern_analysis/src/rustc/print.rs index 64f041dc21a76..dba6022320220 100644 --- a/compiler/rustc_pattern_analysis/src/rustc/print.rs +++ b/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}; @@ -70,7 +70,7 @@ impl<'tcx> fmt::Display for Pat<'tcx> { PatKind::Never => write!(f, "!"), PatKind::Box { ref subpattern } => write!(f, "box {subpattern}"), PatKind::StructLike { ref enum_info, ref subpatterns } => { - write_struct_like(f, self.ty, enum_info, subpatterns) + ty::tls::with(|tcx| write_struct_like(f, tcx, self.ty, enum_info, subpatterns)) } PatKind::Deref { ref subpattern } => write_ref_like(f, self.ty, subpattern), PatKind::Constant { value } => write!(f, "{value}"), @@ -105,12 +105,13 @@ pub(crate) enum EnumInfo<'tcx> { 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 } => ty::tls::with(|tcx| { + EnumInfo::Enum { adt_def, variant_index } => { 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) @@ -121,9 +122,9 @@ fn write_struct_like<'tcx>( format!("{}::{}", tcx.def_path_str(adt_def.did()), variant.name) }; Some((variant, name)) - }), + } EnumInfo::NotEnum => ty.ty_adt_def().and_then(|adt_def| { - ty::tls::with(|tcx| Some((adt_def.non_enum_variant(), tcx.def_path_str(adt_def.did())))) + Some((adt_def.non_enum_variant(), tcx.def_path_str(adt_def.did()))) }), }; From 482412c98ad714ae386a85fc44222197be3d3ffc Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 2 Aug 2024 21:14:08 +1000 Subject: [PATCH 11/11] Use `TyCtxt::is_diagnostic_item` --- compiler/rustc_pattern_analysis/src/rustc/print.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc/print.rs b/compiler/rustc_pattern_analysis/src/rustc/print.rs index dba6022320220..7d6387146054e 100644 --- a/compiler/rustc_pattern_analysis/src/rustc/print.rs +++ b/compiler/rustc_pattern_analysis/src/rustc/print.rs @@ -114,8 +114,8 @@ fn write_struct_like<'tcx>( EnumInfo::Enum { adt_def, variant_index } => { 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) + let name = if tcx.is_diagnostic_item(sym::Option, adt_did) + || tcx.is_diagnostic_item(sym::Result, adt_did) { variant.name.to_string() } else {