Skip to content

pattern analysis: Store field indices in DeconstructedPat to avoid virtual wildcards #121820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 13, 2024
+140 −95
Merged

pattern analysis: Store field indices in DeconstructedPat to avoid virtual wildcards #121820

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1e6886
Store pattern arity in `DeconstructedPat`
Nadrieril Feb 29, 2024
6ae9fa3
Store field indices in `DeconstructedPat` to avoid virtual wildcards
Nadrieril Feb 29, 2024
d339bda
`DeconstructedPat.data` is always present now
Nadrieril Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
View file
Original file line number Diff line number Diff line change
@@ -420,9 +420,9 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
{
let mut redundant_subpats = redundant_subpats.clone();
// Emit lints in the order in which they occur in the file.
redundant_subpats.sort_unstable_by_key(|pat| pat.data().unwrap().span);
redundant_subpats.sort_unstable_by_key(|pat| pat.data().span);
for pat in redundant_subpats {
report_unreachable_pattern(cx, arm.arm_data, pat.data().unwrap().span, None)
report_unreachable_pattern(cx, arm.arm_data, pat.data().span, None)
}
}
}
@@ -905,10 +905,10 @@ fn report_arm_reachability<'p, 'tcx>(
let mut catchall = None;
for (arm, is_useful) in report.arm_usefulness.iter() {
if matches!(is_useful, Usefulness::Redundant) {
report_unreachable_pattern(cx, arm.arm_data, arm.pat.data().unwrap().span, catchall)
report_unreachable_pattern(cx, arm.arm_data, arm.pat.data().span, catchall)
}
if !arm.has_guard && catchall.is_none() && pat_is_catchall(arm.pat) {
catchall = Some(arm.pat.data().unwrap().span);
catchall = Some(arm.pat.data().span);
}
}
}
@@ -917,7 +917,9 @@ fn report_arm_reachability<'p, 'tcx>(
fn pat_is_catchall(pat: &DeconstructedPat<'_, '_>) -> bool {
match pat.ctor() {
Constructor::Wildcard => true,
Constructor::Struct | Constructor::Ref => pat.iter_fields().all(|pat| pat_is_catchall(pat)),
Constructor::Struct | Constructor::Ref => {
pat.iter_fields().all(|ipat| pat_is_catchall(&ipat.pat))
}
_ => false,
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_pattern_analysis/src/constructor.rs
View file
Original file line number Diff line number Diff line change
@@ -423,7 +423,7 @@ pub enum SliceKind {
}

impl SliceKind {
fn arity(self) -> usize {
pub fn arity(self) -> usize {
match self {
FixedLen(length) => length,
VarLen(prefix, suffix) => prefix + suffix,
@@ -462,7 +462,7 @@ impl Slice {
Slice { array_len, kind }
}

pub(crate) fn arity(self) -> usize {
pub fn arity(self) -> usize {
self.kind.arity()
}

2 changes: 1 addition & 1 deletion compiler/rustc_pattern_analysis/src/lints.rs
View file
Original file line number Diff line number Diff line change
@@ -98,7 +98,7 @@ pub(crate) fn lint_nonexhaustive_missing_variants<'p, 'tcx>(
};

use rustc_errors::LintDiagnostic;
let mut err = rcx.tcx.dcx().struct_span_warn(arm.pat.data().unwrap().span, "");
let mut err = rcx.tcx.dcx().struct_span_warn(arm.pat.data().span, "");
err.primary_message(decorator.msg());
decorator.decorate_lint(&mut err);
err.emit();
122 changes: 71 additions & 51 deletions compiler/rustc_pattern_analysis/src/pat.rs
View file
Original file line number Diff line number Diff line change
@@ -20,32 +20,42 @@ impl PatId {
}
}

/// A pattern with an index denoting which field it corresponds to.
pub struct IndexedPat<Cx: TypeCx> {
pub idx: usize,
pub pat: DeconstructedPat<Cx>,
}

/// Values and patterns can be represented as a constructor applied to some fields. This represents
/// a pattern in this form. A `DeconstructedPat` will almost always come from user input; the only
/// exception are some `Wildcard`s introduced during pattern lowering.
pub struct DeconstructedPat<Cx: TypeCx> {
ctor: Constructor<Cx>,
fields: Vec<DeconstructedPat<Cx>>,
fields: Vec<IndexedPat<Cx>>,
/// The number of fields in this pattern. E.g. if the pattern is `SomeStruct { field12: true, ..
/// }` this would be the total number of fields of the struct.
/// This is also the same as `self.ctor.arity(self.ty)`.
arity: usize,
ty: Cx::Ty,
/// Extra data to store in a pattern. `None` if the pattern is a wildcard that does not
/// correspond to a user-supplied pattern.
data: Option<Cx::PatData>,
/// Extra data to store in a pattern.
data: Cx::PatData,
/// Globally-unique id used to track usefulness at the level of subpatterns.
pub(crate) uid: PatId,
}

impl<Cx: TypeCx> DeconstructedPat<Cx> {
pub fn wildcard(ty: Cx::Ty) -> Self {
DeconstructedPat { ctor: Wildcard, fields: Vec::new(), ty, data: None, uid: PatId::new() }
}

pub fn new(
ctor: Constructor<Cx>,
fields: Vec<DeconstructedPat<Cx>>,
fields: Vec<IndexedPat<Cx>>,
arity: usize,
ty: Cx::Ty,
data: Cx::PatData,
) -> Self {
DeconstructedPat { ctor, fields, ty, data: Some(data), uid: PatId::new() }
DeconstructedPat { ctor, fields, arity, ty, data, uid: PatId::new() }
}

pub fn at_index(self, idx: usize) -> IndexedPat<Cx> {
IndexedPat { idx, pat: self }
}

pub(crate) fn is_or_pat(&self) -> bool {
@@ -58,13 +68,15 @@ impl<Cx: TypeCx> DeconstructedPat<Cx> {
pub fn ty(&self) -> &Cx::Ty {
&self.ty
}
/// Returns the extra data stored in a pattern. Returns `None` if the pattern is a wildcard that
/// does not correspond to a user-supplied pattern.
pub fn data(&self) -> Option<&Cx::PatData> {
self.data.as_ref()
/// Returns the extra data stored in a pattern.
pub fn data(&self) -> &Cx::PatData {
&self.data
}
pub fn arity(&self) -> usize {
self.arity
}

pub fn iter_fields<'a>(&'a self) -> impl Iterator<Item = &'a DeconstructedPat<Cx>> {
pub fn iter_fields<'a>(&'a self) -> impl Iterator<Item = &'a IndexedPat<Cx>> {
self.fields.iter()
}

@@ -73,36 +85,40 @@ impl<Cx: TypeCx> DeconstructedPat<Cx> {
pub(crate) fn specialize<'a>(
&'a self,
other_ctor: &Constructor<Cx>,
ctor_arity: usize,
other_ctor_arity: usize,
) -> SmallVec<[PatOrWild<'a, Cx>; 2]> {
let wildcard_sub_tys = || (0..ctor_arity).map(|_| PatOrWild::Wild).collect();
match (&self.ctor, other_ctor) {
// Return a wildcard for each field of `other_ctor`.
(Wildcard, _) => wildcard_sub_tys(),
if matches!(other_ctor, PrivateUninhabited) {
// Skip this column.
(_, PrivateUninhabited) => smallvec![],
// The only non-trivial case: two slices of different arity. `other_slice` is
// guaranteed to have a larger arity, so we fill the middle part with enough
// wildcards to reach the length of the new, larger slice.
(
&Slice(self_slice @ Slice { kind: SliceKind::VarLen(prefix, suffix), .. }),
&Slice(other_slice),
) if self_slice.arity() != other_slice.arity() => {
// Start with a slice of wildcards of the appropriate length.
let mut fields: SmallVec<[_; 2]> = wildcard_sub_tys();
// Fill in the fields from both ends.
let new_arity = fields.len();
for i in 0..prefix {
fields[i] = PatOrWild::Pat(&self.fields[i]);
return smallvec![];
}

// Start with a slice of wildcards of the appropriate length.
let mut fields: SmallVec<[_; 2]> = (0..other_ctor_arity).map(|_| PatOrWild::Wild).collect();
// Fill `fields` with our fields. The arities are known to be compatible.
match self.ctor {
// The only non-trivial case: two slices of different arity. `other_ctor` is guaranteed
// to have a larger arity, so we adjust the indices of the patterns in the suffix so
// that they are correctly positioned in the larger slice.
Slice(Slice { kind: SliceKind::VarLen(prefix, _), .. })
if self.arity != other_ctor_arity =>
{
for ipat in &self.fields {
let new_idx = if ipat.idx < prefix {
ipat.idx
} else {
// Adjust the indices in the suffix.
ipat.idx + other_ctor_arity - self.arity
};
fields[new_idx] = PatOrWild::Pat(&ipat.pat);
}
for i in 0..suffix {
fields[new_arity - 1 - i] =
PatOrWild::Pat(&self.fields[self.fields.len() - 1 - i]);
}
_ => {
for ipat in &self.fields {
fields[ipat.idx] = PatOrWild::Pat(&ipat.pat);
}
fields
}
_ => self.fields.iter().map(PatOrWild::Pat).collect(),
}
fields
}

/// Walk top-down and call `it` in each place where a pattern occurs
@@ -114,7 +130,7 @@ impl<Cx: TypeCx> DeconstructedPat<Cx> {
}

for p in self.iter_fields() {
p.walk(it)
p.pat.walk(it)
}
}
}
@@ -134,14 +150,19 @@ impl<Cx: TypeCx> fmt::Debug for DeconstructedPat<Cx> {
};
let mut start_or_comma = || start_or_continue(", ");

let mut fields: Vec<_> = (0..self.arity).map(|_| PatOrWild::Wild).collect();
for ipat in self.iter_fields() {
fields[ipat.idx] = PatOrWild::Pat(&ipat.pat);
}

match pat.ctor() {
Struct | Variant(_) | UnionField => {
Cx::write_variant_name(f, pat)?;
// Without `cx`, we can't know which field corresponds to which, so we can't
// get the names of the fields. Instead we just display everything as a tuple
// struct, which should be good enough.
write!(f, "(")?;
for p in pat.iter_fields() {
for p in fields {
write!(f, "{}", start_or_comma())?;
write!(f, "{p:?}")?;
}
@@ -151,25 +172,23 @@ impl<Cx: TypeCx> fmt::Debug for DeconstructedPat<Cx> {
// be careful to detect strings here. However a string literal pattern will never
// be reported as a non-exhaustiveness witness, so we can ignore this issue.
Ref => {
let subpattern = pat.iter_fields().next().unwrap();
write!(f, "&{:?}", subpattern)
write!(f, "&{:?}", &fields[0])
}
Slice(slice) => {
let mut subpatterns = pat.iter_fields();
write!(f, "[")?;
match slice.kind {
SliceKind::FixedLen(_) => {
for p in subpatterns {
for p in fields {
write!(f, "{}{:?}", start_or_comma(), p)?;
}
}
SliceKind::VarLen(prefix_len, _) => {
for p in subpatterns.by_ref().take(prefix_len) {
for p in &fields[..prefix_len] {
write!(f, "{}{:?}", start_or_comma(), p)?;
}
write!(f, "{}", start_or_comma())?;
write!(f, "..")?;
for p in subpatterns {
for p in &fields[prefix_len..] {
write!(f, "{}{:?}", start_or_comma(), p)?;
}
}
@@ -184,7 +203,7 @@ impl<Cx: TypeCx> fmt::Debug for DeconstructedPat<Cx> {
Str(value) => write!(f, "{value:?}"),
Opaque(..) => write!(f, "<constant pattern>"),
Or => {
for pat in pat.iter_fields() {
for pat in fields {
write!(f, "{}{:?}", start_or_continue(" | "), pat)?;
}
Ok(())
@@ -242,9 +261,10 @@ impl<'p, Cx: TypeCx> PatOrWild<'p, Cx> {
/// Expand this (possibly-nested) or-pattern into its alternatives.
pub(crate) fn flatten_or_pat(self) -> SmallVec<[Self; 1]> {
match self {
PatOrWild::Pat(pat) if pat.is_or_pat() => {
pat.iter_fields().flat_map(|p| PatOrWild::Pat(p).flatten_or_pat()).collect()
}
PatOrWild::Pat(pat) if pat.is_or_pat() => pat
.iter_fields()
.flat_map(|ipat| PatOrWild::Pat(&ipat.pat).flatten_or_pat())
.collect(),
_ => smallvec![self],
}
}
78 changes: 49 additions & 29 deletions compiler/rustc_pattern_analysis/src/rustc.rs
View file
Original file line number Diff line number Diff line change
@@ -445,17 +445,20 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
let cx = self;
let ty = cx.reveal_opaque_ty(pat.ty);
let ctor;
let mut fields: Vec<_>;
let arity;
let fields: Vec<_>;
match &pat.kind {
PatKind::AscribeUserType { subpattern, .. }
| PatKind::InlineConstant { subpattern, .. } => return self.lower_pat(subpattern),
PatKind::Binding { subpattern: Some(subpat), .. } => return self.lower_pat(subpat),
PatKind::Binding { subpattern: None, .. } | PatKind::Wild => {
ctor = Wildcard;
fields = vec![];
arity = 0;
}
PatKind::Deref { subpattern } => {
fields = vec![self.lower_pat(subpattern)];
fields = vec![self.lower_pat(subpattern).at_index(0)];
arity = 1;
ctor = match ty.kind() {
// This is a box pattern.
ty::Adt(adt, ..) if adt.is_box() => Struct,
@@ -467,16 +470,13 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
match ty.kind() {
ty::Tuple(fs) => {
ctor = Struct;
fields = fs
arity = fs.len();
fields = subpatterns
.iter()
.map(|ty| cx.reveal_opaque_ty(ty))
.map(|ty| DeconstructedPat::wildcard(ty))
.map(|ipat| self.lower_pat(&ipat.pattern).at_index(ipat.field.index()))
.collect();
for pat in subpatterns {
fields[pat.field.index()] = self.lower_pat(&pat.pattern);
}
}
ty::Adt(adt, args) if adt.is_box() => {
ty::Adt(adt, _) if adt.is_box() => {
// The only legal patterns of type `Box` (outside `std`) are `_` and box
// patterns. If we're here we can assume this is a box pattern.
// FIXME(Nadrieril): A `Box` can in theory be matched either with `Box(_,
@@ -490,13 +490,13 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
// solution when we introduce generalized deref patterns. Also need to
// prevent mixing of those two options.
let pattern = subpatterns.into_iter().find(|pat| pat.field.index() == 0);
let pat = if let Some(pat) = pattern {
self.lower_pat(&pat.pattern)
if let Some(pat) = pattern {
fields = vec![self.lower_pat(&pat.pattern).at_index(0)];
} else {
DeconstructedPat::wildcard(self.reveal_opaque_ty(args.type_at(0)))
};
fields = vec![];
}
ctor = Struct;
fields = vec![pat];
arity = 1;
}
ty::Adt(adt, _) => {
ctor = match pat.kind {
@@ -507,13 +507,11 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
};
let variant =
&adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt));
fields = cx
.variant_sub_tys(ty, variant)
.map(|(_, ty)| DeconstructedPat::wildcard(ty))
arity = variant.fields.len();
fields = subpatterns
.iter()
.map(|ipat| self.lower_pat(&ipat.pattern).at_index(ipat.field.index()))
.collect();
for pat in subpatterns {
fields[pat.field.index()] = self.lower_pat(&pat.pattern);
}
}
_ => bug!("pattern has unexpected type: pat: {:?}, ty: {:?}", pat, ty),
}
@@ -526,6 +524,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
None => Opaque(OpaqueId::new()),
};
fields = vec![];
arity = 0;
}
ty::Char | ty::Int(_) | ty::Uint(_) => {
ctor = match value.try_eval_bits(cx.tcx, cx.param_env) {
@@ -542,6 +541,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
None => Opaque(OpaqueId::new()),
};
fields = vec![];
arity = 0;
}
ty::Float(ty::FloatTy::F32) => {
ctor = match value.try_eval_bits(cx.tcx, cx.param_env) {
@@ -553,6 +553,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
None => Opaque(OpaqueId::new()),
};
fields = vec![];
arity = 0;
}
ty::Float(ty::FloatTy::F64) => {
ctor = match value.try_eval_bits(cx.tcx, cx.param_env) {
@@ -564,6 +565,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
None => Opaque(OpaqueId::new()),
};
fields = vec![];
arity = 0;
}
ty::Ref(_, t, _) if t.is_str() => {
// We want a `&str` constant to behave like a `Deref` pattern, to be compatible
@@ -574,16 +576,18 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
// subfields.
// Note: `t` is `str`, not `&str`.
let ty = self.reveal_opaque_ty(*t);
let subpattern = DeconstructedPat::new(Str(*value), Vec::new(), ty, pat);
let subpattern = DeconstructedPat::new(Str(*value), Vec::new(), 0, ty, pat);
ctor = Ref;
fields = vec![subpattern]
fields = vec![subpattern.at_index(0)];
arity = 1;
}
// All constants that can be structurally matched have already been expanded
// into the corresponding `Pat`s by `const_to_pat`. Constants that remain are
// opaque.
_ => {
ctor = Opaque(OpaqueId::new());
fields = vec![];
arity = 0;
}
}
}
@@ -623,6 +627,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
_ => bug!("invalid type for range pattern: {}", ty.inner()),
};
fields = vec![];
arity = 0;
}
PatKind::Array { prefix, slice, suffix } | PatKind::Slice { prefix, slice, suffix } => {
let array_len = match ty.kind() {
@@ -638,26 +643,41 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
SliceKind::FixedLen(prefix.len() + suffix.len())
};
ctor = Slice(Slice::new(array_len, kind));
fields = prefix.iter().chain(suffix.iter()).map(|p| self.lower_pat(&*p)).collect();
fields = prefix
.iter()
.chain(suffix.iter())
.map(|p| self.lower_pat(&*p))
.enumerate()
.map(|(i, p)| p.at_index(i))
.collect();
arity = kind.arity();
}
PatKind::Or { .. } => {
ctor = Or;
let pats = expand_or_pat(pat);
fields = pats.into_iter().map(|p| self.lower_pat(p)).collect();
fields = pats
.into_iter()
.map(|p| self.lower_pat(p))
.enumerate()
.map(|(i, p)| p.at_index(i))
.collect();
arity = fields.len();
}
PatKind::Never => {
// A never pattern matches all the values of its type (namely none). Moreover it
// must be compatible with other constructors, since we can use `!` on a type like
// `Result<!, !>` which has other constructors. Hence we lower it as a wildcard.
ctor = Wildcard;
fields = vec![];
arity = 0;
}
PatKind::Error(_) => {
ctor = Opaque(OpaqueId::new());
fields = vec![];
arity = 0;
}
}
DeconstructedPat::new(ctor, fields, ty, pat)
DeconstructedPat::new(ctor, fields, arity, ty, pat)
}

/// Convert back to a `thir::PatRangeBoundary` for diagnostic purposes.
@@ -884,10 +904,10 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
let overlap_as_pat = self.hoist_pat_range(&overlaps_on, *pat.ty());
let overlaps: Vec<_> = overlaps_with
.iter()
.map(|pat| pat.data().unwrap().span)
.map(|pat| pat.data().span)
.map(|span| errors::Overlap { range: overlap_as_pat.clone(), span })
.collect();
let pat_span = pat.data().unwrap().span;
let pat_span = pat.data().span;
self.tcx.emit_node_span_lint(
lint::builtin::OVERLAPPING_RANGE_ENDPOINTS,
self.match_lint_level,
@@ -907,7 +927,7 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
gap: IntRange,
gapped_with: &[&crate::pat::DeconstructedPat<Self>],
) {
let Some(&thir_pat) = pat.data() else { return };
let &thir_pat = pat.data();
let thir::PatKind::Range(range) = &thir_pat.kind else { return };
// Only lint when the left range is an exclusive range.
if range.end != rustc_hir::RangeEnd::Excluded {
@@ -955,7 +975,7 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
gap_with: gapped_with
.iter()
.map(|pat| errors::GappedRange {
span: pat.data().unwrap().span,
span: pat.data().span,
gap: gap_as_pat.clone(),
first_range: thir_pat.clone(),
})
17 changes: 10 additions & 7 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
View file
Original file line number Diff line number Diff line change
@@ -1006,15 +1006,17 @@ impl<'p, Cx: TypeCx> PatStack<'p, Cx> {
ctor_arity: usize,
ctor_is_relevant: bool,
) -> Result<PatStack<'p, Cx>, Cx::Error> {
// We pop the head pattern and push the new fields extracted from the arguments of
// `self.head()`.
let mut new_pats = self.head().specialize(ctor, ctor_arity);
if new_pats.len() != ctor_arity {
let head_pat = self.head();
if head_pat.as_pat().is_some_and(|pat| pat.arity() > ctor_arity) {
// Arity can be smaller in case of variable-length slices, but mustn't be larger.
return Err(cx.bug(format_args!(
"uncaught type error: pattern {:?} has inconsistent arity (expected arity {ctor_arity})",
self.head().as_pat().unwrap()
"uncaught type error: pattern {:?} has inconsistent arity (expected arity <= {ctor_arity})",
head_pat.as_pat().unwrap()
)));
}
// We pop the head pattern and push the new fields extracted from the arguments of
// `self.head()`.
let mut new_pats = head_pat.specialize(ctor, ctor_arity);
new_pats.extend_from_slice(&self.pats[1..]);
// `ctor` is relevant for this row if it is the actual constructor of this row, or if the
// row has a wildcard and `ctor` is relevant for wildcards.
@@ -1706,7 +1708,8 @@ fn collect_pattern_usefulness<'p, Cx: TypeCx>(
) -> bool {
if useful_subpatterns.contains(&pat.uid) {
true
} else if pat.is_or_pat() && pat.iter_fields().any(|f| pat_is_useful(useful_subpatterns, f))
} else if pat.is_or_pat()
&& pat.iter_fields().any(|f| pat_is_useful(useful_subpatterns, &f.pat))
{
// We always expand or patterns in the matrix, so we will never see the actual
// or-pattern (the one with constructor `Or`) in the column. As such, it will not be