Skip to content

Commit

Permalink
Auto merge of rust-lang#79670 - Nadrieril:uninhabited-query, r=estebank
Browse files Browse the repository at this point in the history
Turn type inhabitedness into a query to fix `exhaustive_patterns` perf

We measured in rust-lang#79394 that enabling the [`exhaustive_patterns` feature](rust-lang#51085) causes significant perf degradation. It was conjectured that the culprit is type inhabitedness checking, and [I hypothesized](rust-lang#79394 (comment)) that turning this computation into a query would solve most of the problem.

This PR turns `tcx.is_ty_uninhabited_from` into a query, and I measured a 25% perf gain on the benchmark that stress-tests `exhaustiveness_patterns`. This more than compensates for the 30% perf hit I measured [when creating it](rust-lang/rustc-perf#801). We'll have to measure enabling the feature again, but I suspect this fixes the perf regression entirely.
I'd like a perf run on this PR obviously.
I made small atomic commits to help reviewing. The first one is just me discovering the "revisions" feature of the testing framework.

I believe there's a push to move things out of `rustc_middle` because it's huge. I guess `inhabitedness/mod.rs` could be moved out, but it's quite small. `DefIdForest` might be movable somewhere too. I don't know what the policy is for that.

Ping `@camelid` since you were interested in following along
`@rustbot` modify labels: +A-exhaustiveness-checking
  • Loading branch information
bors committed Jan 12, 2021
2 parents 7a9b552 + e608d8f commit 058a710
Show file tree
Hide file tree
Showing 15 changed files with 557 additions and 516 deletions.
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,15 @@ rustc_queries! {
eval_always
desc { |tcx| "computing visibility of `{}`", tcx.def_path_str(def_id) }
}

/// Computes the set of modules from which this type is visibly uninhabited.
/// To check whether a type is uninhabited at all (not just from a given module), you could
/// check whether the forest is empty.
query type_uninhabited_from(
key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>
) -> ty::inhabitedness::DefIdForest {
desc { "computing the inhabitedness of `{:?}`", key }
}
}

Other {
Expand Down
120 changes: 77 additions & 43 deletions compiler/rustc_middle/src/ty/inhabitedness/def_id_forest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ use crate::ty::{DefId, DefIdTree};
use rustc_hir::CRATE_HIR_ID;
use smallvec::SmallVec;
use std::mem;
use std::sync::Arc;

use DefIdForest::*;

/// Represents a forest of `DefId`s closed under the ancestor relation. That is,
/// if a `DefId` representing a module is contained in the forest then all
Expand All @@ -11,45 +14,77 @@ use std::mem;
///
/// This is used to represent a set of modules in which a type is visibly
/// uninhabited.
#[derive(Clone)]
pub struct DefIdForest {
/// The minimal set of `DefId`s required to represent the whole set.
/// If A and B are DefIds in the `DefIdForest`, and A is a descendant
/// of B, then only B will be in `root_ids`.
/// We use a `SmallVec` here because (for its use for caching inhabitedness)
/// it's rare that this will contain even two IDs.
root_ids: SmallVec<[DefId; 1]>,
///
/// We store the minimal set of `DefId`s required to represent the whole set. If A and B are
/// `DefId`s in the `DefIdForest`, and A is a parent of B, then only A will be stored. When this is
/// used with `type_uninhabited_from`, there will very rarely be more than one `DefId` stored.
#[derive(Clone, HashStable)]
pub enum DefIdForest {
Empty,
Single(DefId),
/// This variant is very rare.
/// Invariant: >1 elements
/// We use `Arc` because this is used in the output of a query.
Multiple(Arc<[DefId]>),
}

/// Tests whether a slice of roots contains a given DefId.
#[inline]
fn slice_contains(tcx: TyCtxt<'tcx>, slice: &[DefId], id: DefId) -> bool {
slice.iter().any(|root_id| tcx.is_descendant_of(id, *root_id))
}

impl<'tcx> DefIdForest {
/// Creates an empty forest.
pub fn empty() -> DefIdForest {
DefIdForest { root_ids: SmallVec::new() }
DefIdForest::Empty
}

/// Creates a forest consisting of a single tree representing the entire
/// crate.
#[inline]
pub fn full(tcx: TyCtxt<'tcx>) -> DefIdForest {
let crate_id = tcx.hir().local_def_id(CRATE_HIR_ID);
DefIdForest::from_id(crate_id.to_def_id())
DefIdForest::from_id(tcx.hir().local_def_id(CRATE_HIR_ID).to_def_id())
}

/// Creates a forest containing a `DefId` and all its descendants.
pub fn from_id(id: DefId) -> DefIdForest {
let mut root_ids = SmallVec::new();
root_ids.push(id);
DefIdForest { root_ids }
DefIdForest::Single(id)
}

fn as_slice(&self) -> &[DefId] {
match self {
Empty => &[],
Single(id) => std::slice::from_ref(id),
Multiple(root_ids) => root_ids,
}
}

// Only allocates in the rare `Multiple` case.
fn from_slice(root_ids: &[DefId]) -> DefIdForest {
match root_ids {
[] => Empty,
[id] => Single(*id),
_ => DefIdForest::Multiple(root_ids.into()),
}
}

/// Tests whether the forest is empty.
pub fn is_empty(&self) -> bool {
self.root_ids.is_empty()
match self {
Empty => true,
Single(..) | Multiple(..) => false,
}
}

/// Iterate over the set of roots.
fn iter(&self) -> impl Iterator<Item = DefId> + '_ {
self.as_slice().iter().copied()
}

/// Tests whether the forest contains a given DefId.
pub fn contains(&self, tcx: TyCtxt<'tcx>, id: DefId) -> bool {
self.root_ids.iter().any(|root_id| tcx.is_descendant_of(id, *root_id))
slice_contains(tcx, self.as_slice(), id)
}

/// Calculate the intersection of a collection of forests.
Expand All @@ -58,56 +93,55 @@ impl<'tcx> DefIdForest {
I: IntoIterator<Item = DefIdForest>,
{
let mut iter = iter.into_iter();
let mut ret = if let Some(first) = iter.next() {
first
let mut ret: SmallVec<[_; 1]> = if let Some(first) = iter.next() {
SmallVec::from_slice(first.as_slice())
} else {
return DefIdForest::full(tcx);
};

let mut next_ret = SmallVec::new();
let mut old_ret: SmallVec<[DefId; 1]> = SmallVec::new();
let mut next_ret: SmallVec<[_; 1]> = SmallVec::new();
for next_forest in iter {
// No need to continue if the intersection is already empty.
if ret.is_empty() {
break;
if ret.is_empty() || next_forest.is_empty() {
return DefIdForest::empty();
}

for id in ret.root_ids.drain(..) {
if next_forest.contains(tcx, id) {
next_ret.push(id);
} else {
old_ret.push(id);
}
}
ret.root_ids.extend(old_ret.drain(..));
// We keep the elements in `ret` that are also in `next_forest`.
next_ret.extend(ret.iter().copied().filter(|&id| next_forest.contains(tcx, id)));
// We keep the elements in `next_forest` that are also in `ret`.
next_ret.extend(next_forest.iter().filter(|&id| slice_contains(tcx, &ret, id)));

next_ret.extend(next_forest.root_ids.into_iter().filter(|&id| ret.contains(tcx, id)));

mem::swap(&mut next_ret, &mut ret.root_ids);
next_ret.drain(..);
mem::swap(&mut next_ret, &mut ret);
next_ret.clear();
}
ret
DefIdForest::from_slice(&ret)
}

/// Calculate the union of a collection of forests.
pub fn union<I>(tcx: TyCtxt<'tcx>, iter: I) -> DefIdForest
where
I: IntoIterator<Item = DefIdForest>,
{
let mut ret = DefIdForest::empty();
let mut next_ret = SmallVec::new();
let mut ret: SmallVec<[_; 1]> = SmallVec::new();
let mut next_ret: SmallVec<[_; 1]> = SmallVec::new();
for next_forest in iter {
next_ret.extend(ret.root_ids.drain(..).filter(|&id| !next_forest.contains(tcx, id)));
// Union with the empty set is a no-op.
if next_forest.is_empty() {
continue;
}

for id in next_forest.root_ids {
if !next_ret.contains(&id) {
// We add everything in `ret` that is not in `next_forest`.
next_ret.extend(ret.iter().copied().filter(|&id| !next_forest.contains(tcx, id)));
// We add everything in `next_forest` that we haven't added yet.
for id in next_forest.iter() {
if !slice_contains(tcx, &next_ret, id) {
next_ret.push(id);
}
}

mem::swap(&mut next_ret, &mut ret.root_ids);
next_ret.drain(..);
mem::swap(&mut next_ret, &mut ret);
next_ret.clear();
}
ret
DefIdForest::from_slice(&ret)
}
}
61 changes: 36 additions & 25 deletions compiler/rustc_middle/src/ty/inhabitedness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::ty::TyKind::*;
use crate::ty::{AdtDef, FieldDef, Ty, TyS, VariantDef};
use crate::ty::{AdtKind, Visibility};
use crate::ty::{DefId, SubstsRef};
use rustc_data_structures::stack::ensure_sufficient_stack;

mod def_id_forest;

Expand Down Expand Up @@ -187,34 +186,46 @@ impl<'tcx> FieldDef {

impl<'tcx> TyS<'tcx> {
/// Calculates the forest of `DefId`s from which this type is visibly uninhabited.
fn uninhabited_from(&self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> DefIdForest {
match *self.kind() {
Adt(def, substs) => {
ensure_sufficient_stack(|| def.uninhabited_from(tcx, substs, param_env))
}
fn uninhabited_from(
&'tcx self,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> DefIdForest {
tcx.type_uninhabited_from(param_env.and(self))
}
}

Never => DefIdForest::full(tcx),
// Query provider for `type_uninhabited_from`.
pub(crate) fn type_uninhabited_from<'tcx>(
tcx: TyCtxt<'tcx>,
key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> DefIdForest {
let ty = key.value;
let param_env = key.param_env;
match *ty.kind() {
Adt(def, substs) => def.uninhabited_from(tcx, substs, param_env),

Tuple(ref tys) => DefIdForest::union(
tcx,
tys.iter().map(|ty| ty.expect_ty().uninhabited_from(tcx, param_env)),
),
Never => DefIdForest::full(tcx),

Array(ty, len) => match len.try_eval_usize(tcx, param_env) {
Some(0) | None => DefIdForest::empty(),
// If the array is definitely non-empty, it's uninhabited if
// the type of its elements is uninhabited.
Some(1..) => ty.uninhabited_from(tcx, param_env),
},
Tuple(ref tys) => DefIdForest::union(
tcx,
tys.iter().map(|ty| ty.expect_ty().uninhabited_from(tcx, param_env)),
),

// References to uninitialised memory are valid for any type, including
// uninhabited types, in unsafe code, so we treat all references as
// inhabited.
// The precise semantics of inhabitedness with respect to references is currently
// undecided.
Ref(..) => DefIdForest::empty(),
Array(ty, len) => match len.try_eval_usize(tcx, param_env) {
Some(0) | None => DefIdForest::empty(),
// If the array is definitely non-empty, it's uninhabited if
// the type of its elements is uninhabited.
Some(1..) => ty.uninhabited_from(tcx, param_env),
},

_ => DefIdForest::empty(),
}
// References to uninitialised memory are valid for any type, including
// uninhabited types, in unsafe code, so we treat all references as
// inhabited.
// The precise semantics of inhabitedness with respect to references is currently
// undecided.
Ref(..) => DefIdForest::empty(),

_ => DefIdForest::empty(),
}
}
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3155,6 +3155,7 @@ pub fn provide(providers: &mut ty::query::Providers) {
*providers = ty::query::Providers {
trait_impls_of: trait_def::trait_impls_of_provider,
all_local_trait_impls: trait_def::all_local_trait_impls,
type_uninhabited_from: inhabitedness::type_uninhabited_from,
..*providers
};
}
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/pattern/usefulness/auxiliary/empty.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
#![crate_type = "rlib"]
pub enum EmptyForeignEnum {}

pub struct VisiblyUninhabitedForeignStruct {
pub field: EmptyForeignEnum,
}

pub struct SecretlyUninhabitedForeignStruct {
_priv: EmptyForeignEnum,
}
Loading

0 comments on commit 058a710

Please sign in to comment.