From a8babed8a3a806066f2bf2ae3fcd21fd35703ed8 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 21 Dec 2018 11:44:05 +0100 Subject: [PATCH] Remove "visited" set from inhabitedness checking Now that references are no longer recursively checked, this should no longer be necessary, and it's a major performance bottleneck. --- src/librustc/ty/inhabitedness/mod.rs | 54 ++++++---------------------- 1 file changed, 10 insertions(+), 44 deletions(-) diff --git a/src/librustc/ty/inhabitedness/mod.rs b/src/librustc/ty/inhabitedness/mod.rs index 721d5e14ccc..c64811e32f4 100644 --- a/src/librustc/ty/inhabitedness/mod.rs +++ b/src/librustc/ty/inhabitedness/mod.rs @@ -8,7 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use util::nodemap::{FxHashMap, FxHashSet}; use ty::context::TyCtxt; use ty::{AdtDef, VariantDef, FieldDef, Ty, TyS}; use ty::{DefId, Substs}; @@ -113,7 +112,7 @@ pub fn is_ty_uninhabited_from_all_modules(self, ty: Ty<'tcx>) -> bool { } fn ty_inhabitedness_forest(self, ty: Ty<'tcx>) -> DefIdForest { - ty.uninhabited_from(&mut FxHashMap::default(), self) + ty.uninhabited_from(self) } pub fn is_enum_variant_uninhabited_from(self, @@ -140,7 +139,7 @@ fn variant_inhabitedness_forest(self, variant: &'tcx VariantDef, substs: &'tcx S let adt_kind = self.adt_def(adt_def_id).adt_kind(); // Compute inhabitedness forest: - variant.uninhabited_from(&mut FxHashMap::default(), self, substs, adt_kind) + variant.uninhabited_from(self, substs, adt_kind) } } @@ -148,12 +147,11 @@ impl<'a, 'gcx, 'tcx> AdtDef { /// Calculate the forest of DefIds from which this adt is visibly uninhabited. fn uninhabited_from( &self, - visited: &mut FxHashMap>>, tcx: TyCtxt<'a, 'gcx, 'tcx>, substs: &'tcx Substs<'tcx>) -> DefIdForest { DefIdForest::intersection(tcx, self.variants.iter().map(|v| { - v.uninhabited_from(visited, tcx, substs, self.adt_kind()) + v.uninhabited_from(tcx, substs, self.adt_kind()) })) } } @@ -162,7 +160,6 @@ impl<'a, 'gcx, 'tcx> VariantDef { /// Calculate the forest of DefIds from which this variant is visibly uninhabited. fn uninhabited_from( &self, - visited: &mut FxHashMap>>, tcx: TyCtxt<'a, 'gcx, 'tcx>, substs: &'tcx Substs<'tcx>, adt_kind: AdtKind) -> DefIdForest @@ -175,7 +172,7 @@ fn uninhabited_from( AdtKind::Struct => false, }; DefIdForest::union(tcx, self.fields.iter().map(|f| { - f.uninhabited_from(visited, tcx, substs, is_enum) + f.uninhabited_from(tcx, substs, is_enum) })) } } @@ -184,13 +181,12 @@ impl<'a, 'gcx, 'tcx> FieldDef { /// Calculate the forest of DefIds from which this field is visibly uninhabited. fn uninhabited_from( &self, - visited: &mut FxHashMap>>, tcx: TyCtxt<'a, 'gcx, 'tcx>, substs: &'tcx Substs<'tcx>, is_enum: bool, ) -> DefIdForest { - let mut data_uninhabitedness = move || { - self.ty(tcx, substs).uninhabited_from(visited, tcx) + let data_uninhabitedness = move || { + self.ty(tcx, substs).uninhabited_from(tcx) }; // FIXME(canndrew): Currently enum fields are (incorrectly) stored with // Visibility::Invisible so we need to override self.vis if we're @@ -213,46 +209,16 @@ fn uninhabited_from( impl<'a, 'gcx, 'tcx> TyS<'tcx> { /// Calculate the forest of DefIds from which this type is visibly uninhabited. - fn uninhabited_from( - &self, - visited: &mut FxHashMap>>, - tcx: TyCtxt<'a, 'gcx, 'tcx>) -> DefIdForest + fn uninhabited_from(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> DefIdForest { match self.sty { - Adt(def, substs) => { - { - let substs_set = visited.entry(def.did).or_default(); - if !substs_set.insert(substs) { - // We are already calculating the inhabitedness of this type. - // The type must contain a reference to itself. Break the - // infinite loop. - return DefIdForest::empty(); - } - if substs_set.len() >= tcx.sess.recursion_limit.get() / 4 { - // We have gone very deep, reinstantiating this ADT inside - // itself with different type arguments. We are probably - // hitting an infinite loop. For example, it's possible to write: - // a type Foo - // which contains a Foo<(T, T)> - // which contains a Foo<((T, T), (T, T))> - // which contains a Foo<(((T, T), (T, T)), ((T, T), (T, T)))> - // etc. - let error = format!("reached recursion limit while checking \ - inhabitedness of `{}`", self); - tcx.sess.fatal(&error); - } - } - let ret = def.uninhabited_from(visited, tcx, substs); - let substs_set = visited.get_mut(&def.did).unwrap(); - substs_set.remove(substs); - ret - } + Adt(def, substs) => def.uninhabited_from(tcx, substs), Never => DefIdForest::full(tcx), Tuple(ref tys) => { DefIdForest::union(tcx, tys.iter().map(|ty| { - ty.uninhabited_from(visited, tcx) + ty.uninhabited_from(tcx) })) } @@ -260,7 +226,7 @@ fn uninhabited_from( match len.assert_usize(tcx) { // If the array is definitely non-empty, it's uninhabited if // the type of its elements is uninhabited. - Some(n) if n != 0 => ty.uninhabited_from(visited, tcx), + Some(n) if n != 0 => ty.uninhabited_from(tcx), _ => DefIdForest::empty() } }