Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
42 changes: 18 additions & 24 deletions compiler/rustc_pattern_analysis/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::errors::{
NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Overlap,
OverlappingRangeEndpoints, Uncovered,
};
use crate::pat::PatOrWild;
use crate::rustc::{
Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy, RustcMatchCheckCtxt,
SplitConstructorSet, WitnessPat,
Expand All @@ -33,20 +34,24 @@ pub(crate) struct PatternColumn<'p, 'tcx> {

impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
pub(crate) fn new(arms: &[MatchArm<'p, 'tcx>]) -> Self {
let mut patterns = Vec::with_capacity(arms.len());
let patterns = Vec::with_capacity(arms.len());
let mut column = PatternColumn { patterns };
for arm in arms {
if arm.pat.is_or_pat() {
patterns.extend(arm.pat.flatten_or_pat())
} else {
patterns.push(arm.pat)
}
column.expand_and_push(PatOrWild::Pat(arm.pat));
}
Self { patterns }
column
}

fn is_empty(&self) -> bool {
self.patterns.is_empty()
fn expand_and_push(&mut self, pat: PatOrWild<'p, RustcMatchCheckCtxt<'p, 'tcx>>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kind of a shame this is duplicated w/ fn expand_and_push on Matrix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do have a common shape, but I don't see how to unify them more...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(this one pushes individual patterns and skips wildcards, the Matrix one pushes rows and doesn't skip wildcards)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh; then it extra sucks that they are named identically. it would be nice if these methods had more doc comments, lol

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

// We flatten or-patterns and skip wildcards
if pat.is_or_pat() {
self.patterns.extend(
pat.flatten_or_pat().into_iter().filter_map(|pat_or_wild| pat_or_wild.as_pat()),
)
} else if let Some(pat) = pat.as_pat() {
self.patterns.push(pat)
}
}

fn head_ty(&self) -> Option<RevealedTy<'tcx>> {
self.patterns.first().map(|pat| pat.ty())
}
Expand Down Expand Up @@ -83,23 +88,12 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
(0..arity).map(|_| Self { patterns: Vec::new() }).collect();
let relevant_patterns =
self.patterns.iter().filter(|pat| ctor.is_covered_by(pcx, pat.ctor()));
let ctor_sub_tys = pcx.ctor_sub_tys(ctor);
for pat in relevant_patterns {
let specialized = pat.specialize(pcx, ctor, ctor_sub_tys);
for (subpat, column) in specialized.iter().zip(&mut specialized_columns) {
if subpat.is_or_pat() {
column.patterns.extend(subpat.flatten_or_pat())
} else {
column.patterns.push(subpat)
}
let specialized = pat.specialize(ctor, arity);
for (subpat, column) in specialized.into_iter().zip(&mut specialized_columns) {
column.expand_and_push(subpat);
}
}

assert!(
!specialized_columns[0].is_empty(),
"ctor {ctor:?} was listed as present but isn't;
there is an inconsistency between `Constructor::is_covered_by` and `ConstructorSet::split`"
);
specialized_columns
}
}
Expand Down
104 changes: 81 additions & 23 deletions compiler/rustc_pattern_analysis/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::usefulness::PlaceCtxt;
use crate::{Captures, TypeCx};

use self::Constructor::*;
use self::PatOrWild::*;

/// Values and patterns can be represented as a constructor applied to some fields. This represents
/// a pattern in this form.
Expand Down Expand Up @@ -50,14 +51,6 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> {
pub(crate) fn is_or_pat(&self) -> bool {
matches!(self.ctor, Or)
}
/// Expand this (possibly-nested) or-pattern into its alternatives.
pub(crate) fn flatten_or_pat(&self) -> SmallVec<[&Self; 1]> {
if self.is_or_pat() {
self.iter_fields().flat_map(|p| p.flatten_or_pat()).collect()
} else {
smallvec![self]
}
}

pub fn ctor(&self) -> &Constructor<Cx> {
&self.ctor
Expand All @@ -79,17 +72,10 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> {
/// `other_ctor` can be different from `self.ctor`, but must be covered by it.
pub(crate) fn specialize(
&self,
pcx: &PlaceCtxt<'_, 'p, Cx>,
other_ctor: &Constructor<Cx>,
ctor_sub_tys: &[Cx::Ty],
) -> SmallVec<[&'p DeconstructedPat<'p, Cx>; 2]> {
let wildcard_sub_tys = || {
ctor_sub_tys
.iter()
.map(|ty| DeconstructedPat::wildcard(*ty))
.map(|pat| pcx.mcx.wildcard_arena.alloc(pat) as &_)
.collect()
};
ctor_arity: usize,
) -> SmallVec<[PatOrWild<'p, Cx>; 2]> {
let wildcard_sub_tys = || (0..ctor_arity).map(|_| Wild).collect();
match (&self.ctor, other_ctor) {
// Return a wildcard for each field of `other_ctor`.
(Wildcard, _) => wildcard_sub_tys(),
Expand All @@ -105,14 +91,14 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> {
// Fill in the fields from both ends.
let new_arity = fields.len();
for i in 0..prefix {
fields[i] = &self.fields[i];
fields[i] = Pat(&self.fields[i]);
}
for i in 0..suffix {
fields[new_arity - 1 - i] = &self.fields[self.fields.len() - 1 - i];
fields[new_arity - 1 - i] = Pat(&self.fields[self.fields.len() - 1 - i]);
}
fields
}
_ => self.fields.iter().collect(),
_ => self.fields.iter().map(Pat).collect(),
}
}

Expand Down Expand Up @@ -153,14 +139,86 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> {
}
}

/// This is mostly copied from the `Pat` impl. This is best effort and not good enough for a
/// `Display` impl.
/// This is best effort and not good enough for a `Display` impl.
impl<'p, Cx: TypeCx> fmt::Debug for DeconstructedPat<'p, Cx> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Cx::debug_pat(f, self)
}
}

/// Represents either a pattern obtained from user input or a wildcard constructed during the
/// algorithm. Do not use `Wild` to represent a wildcard pattern comping from user input.
///
/// This is morally `Option<&'p DeconstructedPat>` where `None` is interpreted as a wildcard.
#[derive(derivative::Derivative)]
#[derivative(Clone(bound = ""), Copy(bound = ""))]
pub(crate) enum PatOrWild<'p, Cx: TypeCx> {
/// A non-user-provided wildcard, created during specialization.
Wild,
/// A user-provided pattern.
Pat(&'p DeconstructedPat<'p, Cx>),
}

impl<'p, Cx: TypeCx> PatOrWild<'p, Cx> {
pub(crate) fn as_pat(&self) -> Option<&'p DeconstructedPat<'p, Cx>> {
match self {
Wild => None,
Pat(pat) => Some(pat),
}
}
pub(crate) fn ctor(self) -> &'p Constructor<Cx> {
match self {
Wild => &Wildcard,
Pat(pat) => pat.ctor(),
}
}

pub(crate) fn is_or_pat(&self) -> bool {
match self {
Wild => false,
Pat(pat) => pat.is_or_pat(),
}
}

/// Expand this (possibly-nested) or-pattern into its alternatives.
pub(crate) fn flatten_or_pat(self) -> SmallVec<[Self; 1]> {
match self {
Pat(pat) if pat.is_or_pat() => {
pat.iter_fields().flat_map(|p| Pat(p).flatten_or_pat()).collect()
}
_ => smallvec![self],
}
}

/// Specialize this pattern with a constructor.
/// `other_ctor` can be different from `self.ctor`, but must be covered by it.
pub(crate) fn specialize(
&self,
other_ctor: &Constructor<Cx>,
ctor_arity: usize,
) -> SmallVec<[PatOrWild<'p, Cx>; 2]> {
match self {
Wild => (0..ctor_arity).map(|_| Wild).collect(),
Pat(pat) => pat.specialize(other_ctor, ctor_arity),
}
}

pub(crate) fn set_useful(&self) {
if let Pat(pat) = self {
pat.set_useful()
}
}
}

impl<'p, Cx: TypeCx> fmt::Debug for PatOrWild<'p, Cx> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Wild => write!(f, "_"),
Pat(pat) => pat.fmt(f),
}
}
}

/// Same idea as `DeconstructedPat`, except this is a fictitious pattern built up for diagnostics
/// purposes. As such they don't use interning and can be cloned.
#[derive(derivative::Derivative)]
Expand Down
37 changes: 16 additions & 21 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ use smallvec::{smallvec, SmallVec};
use std::fmt;

use crate::constructor::{Constructor, ConstructorSet};
use crate::pat::{DeconstructedPat, WitnessPat};
use crate::pat::{DeconstructedPat, PatOrWild, WitnessPat};
use crate::{Captures, MatchArm, MatchCtxt, TypeCx};

use self::ValidityConstraint::*;
Expand Down Expand Up @@ -827,7 +827,7 @@ impl fmt::Display for ValidityConstraint {
#[derivative(Clone(bound = ""))]
struct PatStack<'p, Cx: TypeCx> {
// Rows of len 1 are very common, which is why `SmallVec[_; 2]` works well.
pats: SmallVec<[&'p DeconstructedPat<'p, Cx>; 2]>,
pats: SmallVec<[PatOrWild<'p, Cx>; 2]>,
/// Sometimes we know that as far as this row is concerned, the current case is already handled
/// by a different, more general, case. When the case is irrelevant for all rows this allows us
/// to skip a case entirely. This is purely an optimization. See at the top for details.
Expand All @@ -836,7 +836,7 @@ struct PatStack<'p, Cx: TypeCx> {

impl<'p, Cx: TypeCx> PatStack<'p, Cx> {
fn from_pattern(pat: &'p DeconstructedPat<'p, Cx>) -> Self {
PatStack { pats: smallvec![pat], relevant: true }
PatStack { pats: smallvec![PatOrWild::Pat(pat)], relevant: true }
}

fn is_empty(&self) -> bool {
Expand All @@ -847,14 +847,11 @@ impl<'p, Cx: TypeCx> PatStack<'p, Cx> {
self.pats.len()
}

fn head_opt(&self) -> Option<&'p DeconstructedPat<'p, Cx>> {
self.pats.first().copied()
}
fn head(&self) -> &'p DeconstructedPat<'p, Cx> {
self.head_opt().unwrap()
fn head(&self) -> PatOrWild<'p, Cx> {
self.pats[0]
}

fn iter(&self) -> impl Iterator<Item = &'p DeconstructedPat<'p, Cx>> + Captures<'_> {
fn iter(&self) -> impl Iterator<Item = PatOrWild<'p, Cx>> + Captures<'_> {
self.pats.iter().copied()
}

Expand All @@ -872,14 +869,13 @@ impl<'p, Cx: TypeCx> PatStack<'p, Cx> {
/// Only call if `ctor.is_covered_by(self.head().ctor())` is true.
fn pop_head_constructor(
&self,
pcx: &PlaceCtxt<'_, 'p, Cx>,
ctor: &Constructor<Cx>,
ctor_sub_tys: &[Cx::Ty],
ctor_arity: usize,
ctor_is_relevant: bool,
) -> PatStack<'p, Cx> {
// We pop the head pattern and push the new fields extracted from the arguments of
// `self.head()`.
let mut new_pats = self.head().specialize(pcx, ctor, ctor_sub_tys);
let mut new_pats = self.head().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.
Expand Down Expand Up @@ -926,11 +922,11 @@ impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> {
self.pats.len()
}

fn head(&self) -> &'p DeconstructedPat<'p, Cx> {
fn head(&self) -> PatOrWild<'p, Cx> {
self.pats.head()
}

fn iter(&self) -> impl Iterator<Item = &'p DeconstructedPat<'p, Cx>> + Captures<'_> {
fn iter(&self) -> impl Iterator<Item = PatOrWild<'p, Cx>> + Captures<'_> {
self.pats.iter()
}

Expand All @@ -949,14 +945,13 @@ impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> {
/// Only call if `ctor.is_covered_by(self.head().ctor())` is true.
fn pop_head_constructor(
&self,
pcx: &PlaceCtxt<'_, 'p, Cx>,
ctor: &Constructor<Cx>,
ctor_sub_tys: &[Cx::Ty],
ctor_arity: usize,
ctor_is_relevant: bool,
parent_row: usize,
) -> MatrixRow<'p, Cx> {
MatrixRow {
pats: self.pats.pop_head_constructor(pcx, ctor, ctor_sub_tys, ctor_is_relevant),
pats: self.pats.pop_head_constructor(ctor, ctor_arity, ctor_is_relevant),
parent_row,
is_under_guard: self.is_under_guard,
useful: false,
Expand Down Expand Up @@ -1054,7 +1049,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
}

/// Iterate over the first pattern of each row.
fn heads(&self) -> impl Iterator<Item = &'p DeconstructedPat<'p, Cx>> + Clone + Captures<'_> {
fn heads(&self) -> impl Iterator<Item = PatOrWild<'p, Cx>> + Clone + Captures<'_> {
self.rows().map(|r| r.head())
}

Expand All @@ -1066,11 +1061,12 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
ctor_is_relevant: bool,
) -> Matrix<'p, Cx> {
let ctor_sub_tys = pcx.ctor_sub_tys(ctor);
let arity = ctor_sub_tys.len();
let specialized_place_ty =
ctor_sub_tys.iter().chain(self.place_ty[1..].iter()).copied().collect();
let ctor_sub_validity = self.place_validity[0].specialize(ctor);
let specialized_place_validity = std::iter::repeat(ctor_sub_validity)
.take(ctor.arity(pcx))
.take(arity)
.chain(self.place_validity[1..].iter().copied())
.collect();
let mut matrix = Matrix {
Expand All @@ -1081,8 +1077,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
};
for (i, row) in self.rows().enumerate() {
if ctor.is_covered_by(pcx, row.head().ctor()) {
let new_row =
row.pop_head_constructor(pcx, ctor, ctor_sub_tys, ctor_is_relevant, i);
let new_row = row.pop_head_constructor(ctor, arity, ctor_is_relevant, i);
matrix.expand_and_push(new_row);
}
}
Expand Down