Skip to content

Commit

Permalink
Auto merge of rust-lang#99843 - oli-obk:is_useful_perf, r=compiler-er…
Browse files Browse the repository at this point in the history 
…rors

Some `is_useful` cleanups

rust-lang#98582 was reverted because it was a perf regression.

rust-lang#99806 reintroduces the changes, but this PR picks individual ones that have no regressions.
  • Loading branch information
@bors
bors committed Aug 4, 2022
2 parents caee496 + 803d374 commit 2f2243c
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 46 deletions.
63 changes: 30 additions & 33 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ impl IntRange {
/// Lint on likely incorrect range patterns (#63987)
pub(super) fn lint_overlapping_range_endpoints<'a, 'p: 'a, 'tcx: 'a>(
&self,
pcx: PatCtxt<'_, 'p, 'tcx>,
pcx: &PatCtxt<'_, 'p, 'tcx>,
pats: impl Iterator<Item = &'a DeconstructedPat<'p, 'tcx>>,
column_count: usize,
hir_id: HirId,
Expand Down Expand Up @@ -696,7 +696,7 @@ impl<'tcx> Constructor<'tcx> {
/// `EvalResult::Deny { .. }`.
///
/// This means that the variant has a stdlib unstable feature marking it.
pub(super) fn is_unstable_variant(&self, pcx: PatCtxt<'_, '_, 'tcx>) -> bool {
pub(super) fn is_unstable_variant(&self, pcx: &PatCtxt<'_, '_, 'tcx>) -> bool {
if let Constructor::Variant(idx) = self && let ty::Adt(adt, _) = pcx.ty.kind() {
let variant_def_id = adt.variant(*idx).def_id;
// Filter variants that depend on a disabled unstable feature.
Expand All @@ -710,7 +710,7 @@ impl<'tcx> Constructor<'tcx> {

/// Checks if the `Constructor` is a `Constructor::Variant` with a `#[doc(hidden)]`
/// attribute from a type not local to the current crate.
pub(super) fn is_doc_hidden_variant(&self, pcx: PatCtxt<'_, '_, 'tcx>) -> bool {
pub(super) fn is_doc_hidden_variant(&self, pcx: &PatCtxt<'_, '_, 'tcx>) -> bool {
if let Constructor::Variant(idx) = self && let ty::Adt(adt, _) = pcx.ty.kind() {
let variant_def_id = adt.variants()[*idx].def_id;
return pcx.cx.tcx.is_doc_hidden(variant_def_id) && !variant_def_id.is_local();
Expand All @@ -731,7 +731,7 @@ impl<'tcx> Constructor<'tcx> {

/// The number of fields for this constructor. This must be kept in sync with
/// `Fields::wildcards`.
pub(super) fn arity(&self, pcx: PatCtxt<'_, '_, 'tcx>) -> usize {
pub(super) fn arity(&self, pcx: &PatCtxt<'_, '_, 'tcx>) -> usize {
match self {
Single | Variant(_) => match pcx.ty.kind() {
ty::Tuple(fs) => fs.len(),
Expand Down Expand Up @@ -775,7 +775,7 @@ impl<'tcx> Constructor<'tcx> {
/// matrix, unless all of them are.
pub(super) fn split<'a>(
&self,
pcx: PatCtxt<'_, '_, 'tcx>,
pcx: &PatCtxt<'_, '_, 'tcx>,
ctors: impl Iterator<Item = &'a Constructor<'tcx>> + Clone,
) -> SmallVec<[Self; 1]>
where
Expand Down Expand Up @@ -811,7 +811,7 @@ impl<'tcx> Constructor<'tcx> {
/// this checks for inclusion.
// We inline because this has a single call site in `Matrix::specialize_constructor`.
#[inline]
pub(super) fn is_covered_by<'p>(&self, pcx: PatCtxt<'_, 'p, 'tcx>, other: &Self) -> bool {
pub(super) fn is_covered_by<'p>(&self, pcx: &PatCtxt<'_, 'p, 'tcx>, other: &Self) -> bool {
// This must be kept in sync with `is_covered_by_any`.
match (self, other) {
// Wildcards cover anything
Expand Down Expand Up @@ -865,7 +865,7 @@ impl<'tcx> Constructor<'tcx> {
/// assumed to have been split from a wildcard.
fn is_covered_by_any<'p>(
&self,
pcx: PatCtxt<'_, 'p, 'tcx>,
pcx: &PatCtxt<'_, 'p, 'tcx>,
used_ctors: &[Constructor<'tcx>],
) -> bool {
if used_ctors.is_empty() {
Expand Down Expand Up @@ -918,7 +918,7 @@ pub(super) struct SplitWildcard<'tcx> {
}

impl<'tcx> SplitWildcard<'tcx> {
pub(super) fn new<'p>(pcx: PatCtxt<'_, 'p, 'tcx>) -> Self {
pub(super) fn new<'p>(pcx: &PatCtxt<'_, 'p, 'tcx>) -> Self {
debug!("SplitWildcard::new({:?})", pcx.ty);
let cx = pcx.cx;
let make_range = |start, end| {
Expand Down Expand Up @@ -1044,7 +1044,7 @@ impl<'tcx> SplitWildcard<'tcx> {
/// do what you want.
pub(super) fn split<'a>(
&mut self,
pcx: PatCtxt<'_, '_, 'tcx>,
pcx: &PatCtxt<'_, '_, 'tcx>,
ctors: impl Iterator<Item = &'a Constructor<'tcx>> + Clone,
) where
'tcx: 'a,
Expand All @@ -1056,21 +1056,21 @@ impl<'tcx> SplitWildcard<'tcx> {
}

/// Whether there are any value constructors for this type that are not present in the matrix.
fn any_missing(&self, pcx: PatCtxt<'_, '_, 'tcx>) -> bool {
fn any_missing(&self, pcx: &PatCtxt<'_, '_, 'tcx>) -> bool {
self.iter_missing(pcx).next().is_some()
}

/// Iterate over the constructors for this type that are not present in the matrix.
pub(super) fn iter_missing<'a, 'p>(
&'a self,
pcx: PatCtxt<'a, 'p, 'tcx>,
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]> {
fn into_ctors(self, pcx: &PatCtxt<'_, '_, 'tcx>) -> SmallVec<[Constructor<'tcx>; 1]> {
if self.any_missing(pcx) {
// Some constructors are missing, thus we can specialize with the special `Missing`
// constructor, which stands for those constructors that are not seen in the matrix,
Expand Down Expand Up @@ -1202,35 +1202,32 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {

/// Creates a new list of wildcard fields for a given constructor. The result must have a
/// length of `constructor.arity()`.
pub(super) fn wildcards(
cx: &MatchCheckCtxt<'p, 'tcx>,
ty: Ty<'tcx>,
constructor: &Constructor<'tcx>,
) -> Self {
#[instrument(level = "trace")]
pub(super) fn wildcards(pcx: &PatCtxt<'_, 'p, 'tcx>, constructor: &Constructor<'tcx>) -> Self {
let ret = match constructor {
Single | Variant(_) => match ty.kind() {
ty::Tuple(fs) => Fields::wildcards_from_tys(cx, fs.iter()),
ty::Ref(_, rty, _) => Fields::wildcards_from_tys(cx, once(*rty)),
Single | Variant(_) => match pcx.ty.kind() {
ty::Tuple(fs) => Fields::wildcards_from_tys(pcx.cx, fs.iter()),
ty::Ref(_, rty, _) => Fields::wildcards_from_tys(pcx.cx, once(*rty)),
ty::Adt(adt, substs) => {
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.
Fields::wildcards_from_tys(cx, once(substs.type_at(0)))
Fields::wildcards_from_tys(pcx.cx, once(substs.type_at(0)))
} else {
let variant = &adt.variant(constructor.variant_index_for_adt(*adt));
let tys = Fields::list_variant_nonhidden_fields(cx, ty, variant)
let tys = Fields::list_variant_nonhidden_fields(pcx.cx, pcx.ty, variant)
.map(|(_, ty)| ty);
Fields::wildcards_from_tys(cx, tys)
Fields::wildcards_from_tys(pcx.cx, tys)
}
}
_ => bug!("Unexpected type for `Single` constructor: {:?}", ty),
_ => bug!("Unexpected type for `Single` constructor: {:?}", pcx),
},
Slice(slice) => match *ty.kind() {
Slice(slice) => match *pcx.ty.kind() {
ty::Slice(ty) | ty::Array(ty, _) => {
let arity = slice.arity();
Fields::wildcards_from_tys(cx, (0..arity).map(|_| ty))
Fields::wildcards_from_tys(pcx.cx, (0..arity).map(|_| ty))
}
_ => bug!("bad slice pattern {:?} {:?}", constructor, ty),
_ => bug!("bad slice pattern {:?} {:?}", constructor, pcx),
},
Str(..)
| FloatRange(..)
Expand All @@ -1243,7 +1240,7 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
bug!("called `Fields::wildcards` on an `Or` ctor")
}
};
debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret);
debug!(?ret);
ret
}

Expand Down Expand Up @@ -1285,8 +1282,8 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
/// Construct a pattern that matches everything that starts with this constructor.
/// For example, if `ctor` is a `Constructor::Variant` for `Option::Some`, we get the pattern
/// `Some(_)`.
pub(super) fn wild_from_ctor(pcx: PatCtxt<'_, 'p, 'tcx>, ctor: Constructor<'tcx>) -> Self {
let fields = Fields::wildcards(pcx.cx, pcx.ty, &ctor);
pub(super) fn wild_from_ctor(pcx: &PatCtxt<'_, 'p, 'tcx>, ctor: Constructor<'tcx>) -> Self {
let fields = Fields::wildcards(pcx, &ctor);
DeconstructedPat::new(ctor, fields, pcx.ty, DUMMY_SP)
}

Expand Down Expand Up @@ -1553,13 +1550,13 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
/// `other_ctor` can be different from `self.ctor`, but must be covered by it.
pub(super) fn specialize<'a>(
&'a self,
cx: &MatchCheckCtxt<'p, 'tcx>,
pcx: &PatCtxt<'_, 'p, 'tcx>,
other_ctor: &Constructor<'tcx>,
) -> SmallVec<[&'p DeconstructedPat<'p, 'tcx>; 2]> {
match (&self.ctor, other_ctor) {
(Wildcard, _) => {
// We return a wildcard for each field of `other_ctor`.
Fields::wildcards(cx, self.ty, other_ctor).iter_patterns().collect()
Fields::wildcards(pcx, other_ctor).iter_patterns().collect()
}
(Slice(self_slice), Slice(other_slice))
if self_slice.arity() != other_slice.arity() =>
Expand All @@ -1578,7 +1575,7 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
let prefix = &self.fields.fields[..prefix];
let suffix = &self.fields.fields[self_slice.arity() - suffix..];
let wildcard: &_ =
cx.pattern_arena.alloc(DeconstructedPat::wildcard(inner_ty));
pcx.cx.pattern_arena.alloc(DeconstructedPat::wildcard(inner_ty));
let extra_wildcards = other_slice.arity() - self_slice.arity();
let extra_wildcards = (0..extra_wildcards).map(|_| wildcard);
prefix.iter().chain(extra_wildcards).chain(suffix).collect()
Expand Down
26 changes: 13 additions & 13 deletions compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,12 +411,12 @@ impl<'p, 'tcx> PatStack<'p, 'tcx> {
/// This is roughly the inverse of `Constructor::apply`.
fn pop_head_constructor(
&self,
cx: &MatchCheckCtxt<'p, 'tcx>,
pcx: &PatCtxt<'_, 'p, 'tcx>,
ctor: &Constructor<'tcx>,
) -> PatStack<'p, 'tcx> {
// We pop the head pattern and push the new fields extracted from the arguments of
// `self.head()`.
let mut new_fields: SmallVec<[_; 2]> = self.head().specialize(cx, ctor);
let mut new_fields: SmallVec<[_; 2]> = self.head().specialize(pcx, ctor);
new_fields.extend_from_slice(&self.pats[1..]);
PatStack::from_vec(new_fields)
}
Expand Down Expand Up @@ -469,13 +469,13 @@ impl<'p, 'tcx> Matrix<'p, 'tcx> {
/// This computes `S(constructor, self)`. See top of the file for explanations.
fn specialize_constructor(
&self,
pcx: PatCtxt<'_, 'p, 'tcx>,
pcx: &PatCtxt<'_, 'p, 'tcx>,
ctor: &Constructor<'tcx>,
) -> Matrix<'p, 'tcx> {
let mut matrix = Matrix::empty();
for row in &self.patterns {
if ctor.is_covered_by(pcx, row.head().ctor()) {
let new_row = row.pop_head_constructor(pcx.cx, ctor);
let new_row = row.pop_head_constructor(pcx, ctor);
matrix.push(new_row);
}
}
Expand Down Expand Up @@ -575,7 +575,7 @@ impl<'p, 'tcx> Usefulness<'p, 'tcx> {
/// with the results of specializing with the other constructors.
fn apply_constructor(
self,
pcx: PatCtxt<'_, 'p, 'tcx>,
pcx: &PatCtxt<'_, 'p, 'tcx>,
matrix: &Matrix<'p, 'tcx>, // used to compute missing ctors
ctor: &Constructor<'tcx>,
) -> Self {
Expand Down Expand Up @@ -713,7 +713,7 @@ impl<'p, 'tcx> Witness<'p, 'tcx> {
///
/// left_ty: struct X { a: (bool, &'static str), b: usize}
/// pats: [(false, "foo"), 42] => X { a: (false, "foo"), b: 42 }
fn apply_constructor(mut self, pcx: PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'tcx>) -> Self {
fn apply_constructor(mut self, pcx: &PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'tcx>) -> Self {
let pat = {
let len = self.0.len();
let arity = ctor.arity(pcx);
Expand Down Expand Up @@ -786,7 +786,7 @@ fn is_useful<'p, 'tcx>(
is_under_guard: bool,
is_top_level: bool,
) -> Usefulness<'p, 'tcx> {
debug!("matrix,v={:?}{:?}", matrix, v);
debug!(?matrix, ?v);
let Matrix { patterns: rows, .. } = matrix;

// The base case. We are pattern-matching on () and the return value is
Expand All @@ -806,11 +806,6 @@ fn is_useful<'p, 'tcx>(

debug_assert!(rows.iter().all(|r| r.len() == v.len()));

let ty = v.head().ty();
let is_non_exhaustive = cx.is_foreign_non_exhaustive_enum(ty);
debug!("v.head: {:?}, v.span: {:?}", v.head(), v.head().span());
let pcx = PatCtxt { cx, ty, span: v.head().span(), is_top_level, is_non_exhaustive };

// If the first pattern is an or-pattern, expand it.
let mut ret = Usefulness::new_not_useful(witness_preference);
if v.head().is_or_pat() {
Expand All @@ -832,6 +827,11 @@ fn is_useful<'p, 'tcx>(
}
}
} else {
let ty = v.head().ty();
let is_non_exhaustive = cx.is_foreign_non_exhaustive_enum(ty);
debug!("v.head: {:?}, v.span: {:?}", v.head(), v.head().span());
let pcx = &PatCtxt { cx, ty, span: v.head().span(), is_top_level, is_non_exhaustive };

let v_ctor = v.head().ctor();
debug!(?v_ctor);
if let Constructor::IntRange(ctor_range) = &v_ctor {
Expand All @@ -853,7 +853,7 @@ fn is_useful<'p, 'tcx>(
debug!("specialize({:?})", ctor);
// We cache the result of `Fields::wildcards` because it is used a lot.
let spec_matrix = start_matrix.specialize_constructor(pcx, &ctor);
let v = v.pop_head_constructor(cx, &ctor);
let v = v.pop_head_constructor(pcx, &ctor);
let usefulness = ensure_sufficient_stack(|| {
is_useful(cx, &spec_matrix, &v, witness_preference, hir_id, is_under_guard, false)
});
Expand Down

0 comments on commit 2f2243c

Please sign in to comment.