]> git.lizzy.rs Git - rust.git/commitdiff
restructure the public inhabitedness APIs and remove the cache
authorNiko Matsakis <niko@alum.mit.edu>
Fri, 8 Sep 2017 13:58:53 +0000 (09:58 -0400)
committerNiko Matsakis <niko@alum.mit.edu>
Mon, 16 Oct 2017 21:32:21 +0000 (17:32 -0400)
The cache was broken anyhow and this computation doesn't look that
expensive. These public accessors could potentially become queries,
but we'd have to add some more complex logic around lift. I'd prefer
to have some test cases to profile with before doing that.

Fixes #44402.

src/librustc/ty/context.rs
src/librustc/ty/inhabitedness/mod.rs
src/librustc/ty/mod.rs
src/librustc_mir/build/matches/simplify.rs
src/test/run-pass/issue-44402.rs [new file with mode: 0644]

index 24ba38cf147796449be3a2883fba992892a54fff..c2e881255f24dc938eaa6c57bd07d514a6c08e99 100644 (file)
@@ -43,7 +43,6 @@
 use ty::{TyVar, TyVid, IntVar, IntVid, FloatVar, FloatVid};
 use ty::TypeVariants::*;
 use ty::layout::{Layout, TargetDataLayout};
-use ty::inhabitedness::DefIdForest;
 use ty::maps;
 use ty::steal::Steal;
 use ty::BindingMode;
@@ -896,8 +895,6 @@ pub struct GlobalCtxt<'tcx> {
     // FIXME dep tracking -- should be harmless enough
     pub normalized_cache: RefCell<FxHashMap<Ty<'tcx>, Ty<'tcx>>>,
 
-    pub inhabitedness_cache: RefCell<FxHashMap<Ty<'tcx>, DefIdForest>>,
-
     /// Caches the results of trait selection. This cache is used
     /// for things that do not have to do with the parameters in scope.
     pub selection_cache: traits::SelectionCache<'tcx>,
@@ -1179,7 +1176,6 @@ pub fn create_and_enter<F, R>(s: &'tcx Session,
             mir_passes,
             rcache: RefCell::new(FxHashMap()),
             normalized_cache: RefCell::new(FxHashMap()),
-            inhabitedness_cache: RefCell::new(FxHashMap()),
             selection_cache: traits::SelectionCache::new(),
             evaluation_cache: traits::EvaluationCache::new(),
             rvalue_promotable_to_static: RefCell::new(NodeMap()),
index 9dc1b19c0f16de6717491076cc1199f14b813b5a..34e9084662ae54c4c172e1e34994c2d7b57e3a86 100644 (file)
@@ -100,14 +100,16 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
     /// This code should only compile in modules where the uninhabitedness of Foo is
     /// visible.
     pub fn is_ty_uninhabited_from(self, module: DefId, ty: Ty<'tcx>) -> bool {
-        let forest = ty.uninhabited_from(&mut FxHashMap(), self);
-
         // To check whether this type is uninhabited at all (not just from the
         // given node) you could check whether the forest is empty.
         // ```
         // forest.is_empty()
         // ```
-        forest.contains(self, module)
+        self.ty_inhabitedness_forest(ty).contains(self, module)
+    }
+
+    fn ty_inhabitedness_forest(self, ty: Ty<'tcx>) -> DefIdForest {
+        ty.uninhabited_from(&mut FxHashMap(), self)
     }
 
     pub fn is_enum_variant_uninhabited_from(self,
@@ -116,17 +118,25 @@ pub fn is_enum_variant_uninhabited_from(self,
                                             substs: &'tcx Substs<'tcx>)
                                             -> bool
     {
-        let adt_kind = AdtKind::Enum;
-        variant.uninhabited_from(&mut FxHashMap(), self, substs, adt_kind).contains(self, module)
+        self.variant_inhabitedness_forest(variant, substs).contains(self, module)
     }
 
     pub fn is_variant_uninhabited_from_all_modules(self,
                                                    variant: &'tcx VariantDef,
-                                                   substs: &'tcx Substs<'tcx>,
-                                                   adt_kind: AdtKind)
+                                                   substs: &'tcx Substs<'tcx>)
                                                    -> bool
     {
-        !variant.uninhabited_from(&mut FxHashMap(), self, substs, adt_kind).is_empty()
+        !self.variant_inhabitedness_forest(variant, substs).is_empty()
+    }
+
+    fn variant_inhabitedness_forest(self, variant: &'tcx VariantDef, substs: &'tcx Substs<'tcx>)
+                                    -> DefIdForest {
+        // Determine the ADT kind:
+        let adt_def_id = self.adt_def_id_of_variant(variant);
+        let adt_kind = self.adt_def(adt_def_id).adt_kind();
+
+        // Compute inhabitedness forest:
+        variant.uninhabited_from(&mut FxHashMap(), self, substs, adt_kind)
     }
 }
 
@@ -210,31 +220,6 @@ fn uninhabited_from(
         &self,
         visited: &mut FxHashMap<DefId, FxHashSet<&'tcx Substs<'tcx>>>,
         tcx: TyCtxt<'a, 'gcx, 'tcx>) -> DefIdForest
-    {
-        match tcx.lift_to_global(&self) {
-            Some(global_ty) => {
-                {
-                    let cache = tcx.inhabitedness_cache.borrow();
-                    if let Some(forest) = cache.get(&global_ty) {
-                        return forest.clone();
-                    }
-                }
-                let forest = global_ty.uninhabited_from_inner(visited, tcx);
-                let mut cache = tcx.inhabitedness_cache.borrow_mut();
-                cache.insert(global_ty, forest.clone());
-                forest
-            },
-            None => {
-                let forest = self.uninhabited_from_inner(visited, tcx);
-                forest
-            },
-        }
-    }
-
-    fn uninhabited_from_inner(
-                &self,
-                visited: &mut FxHashMap<DefId, FxHashSet<&'tcx Substs<'tcx>>>,
-                tcx: TyCtxt<'a, 'gcx, 'tcx>) -> DefIdForest
     {
         match self.sty {
             TyAdt(def, substs) => {
index c4f526d80146b8868b48798ff991a5e3e004b0fc..35969361544ee1163f99a8e28c44e1084ef38599 100644 (file)
@@ -18,6 +18,7 @@
 use hir::{map as hir_map, FreevarMap, TraitMap};
 use hir::def::{Def, CtorKind, ExportMap};
 use hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
+use hir::map::DefPathData;
 use ich::StableHashingContext;
 use middle::const_val::ConstVal;
 use middle::lang_items::{FnTraitLangItem, FnMutTraitLangItem, FnOnceTraitLangItem};
@@ -2232,6 +2233,20 @@ pub fn expect_variant_def(self, def: Def) -> &'tcx VariantDef {
         }
     }
 
+    /// Given a `VariantDef`, returns the def-id of the `AdtDef` of which it is a part.
+    pub fn adt_def_id_of_variant(self, variant_def: &'tcx VariantDef) -> DefId {
+        let def_key = self.def_key(variant_def.did);
+        match def_key.disambiguated_data.data {
+            // for enum variants and tuple structs, the def-id of the ADT itself
+            // is the *parent* of the variant
+            DefPathData::EnumVariant(..) | DefPathData::StructCtor =>
+                DefId { krate: variant_def.did.krate, index: def_key.parent.unwrap() },
+
+            // otherwise, for structs and unions, they share a def-id
+            _ => variant_def.did,
+        }
+    }
+
     pub fn item_name(self, id: DefId) -> InternedString {
         if let Some(id) = self.hir.as_local_node_id(id) {
             self.hir.name(id).as_str()
index f7d15d9118887137023b2e95fc4286d4e2475434..9b3f16f1ab4326b06369a1301008912449f1bb50 100644 (file)
@@ -101,10 +101,7 @@ fn simplify_match_pair<'pat>(&mut self,
                 if self.hir.tcx().sess.features.borrow().never_type {
                     let irrefutable = adt_def.variants.iter().enumerate().all(|(i, v)| {
                         i == variant_index || {
-                            let adt_kind = adt_def.adt_kind();
-                            self.hir.tcx().is_variant_uninhabited_from_all_modules(v,
-                                                                                   substs,
-                                                                                   adt_kind)
+                            self.hir.tcx().is_variant_uninhabited_from_all_modules(v, substs)
                         }
                     });
                     if irrefutable {
diff --git a/src/test/run-pass/issue-44402.rs b/src/test/run-pass/issue-44402.rs
new file mode 100644 (file)
index 0000000..244aa65
--- /dev/null
@@ -0,0 +1,36 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(never_type)]
+
+// Regression test for inhabitedness check. The old
+// cache used to cause us to incorrectly decide
+// that `test_b` was invalid.
+
+struct Foo {
+    field1: !,
+    field2: Option<&'static Bar>,
+}
+
+struct Bar {
+    field1: &'static Foo
+}
+
+fn test_a() {
+    let x: Option<Foo> = None;
+    match x { None => () }
+}
+
+fn test_b() {
+    let x: Option<Bar> = None;
+    match x { None => () }
+}
+
+fn main() { }