Rollup merge of rust-lang#73751 - eddyb:no-empty-tables, r=nikomatsakis

Remove `TypeckTables::empty(None)` and make hir_owner non-optional.

Each commit before the last one removes uses of `TypeckTables::empty(None)`, replacing the empty tables with having `Option` around the `&'tcx TypeckTables<'tcx>` that HIR visitors kept track of.

The last commit removes the concept of "empty `TypeckTables`" altogether, guaranteeing that every `TypeckTables` corresponds to a HIR body owner.

r? @nikomatsakis

Author: Manishearth

src/librustc_driver/lib.rs

@@ -6,6 +6,7 @@
 
 #![doc(html_root_url = "https://doc.rust-lang.org/nightly/")]
 #![feature(nll)]
+#![feature(track_caller)]
 #![recursion_limit = "256"]
 
 #[macro_use]

src/librustc_driver/pretty.rs

@@ -80,8 +80,7 @@ where
         PpmTyped => {
             abort_on_err(tcx.analysis(LOCAL_CRATE), tcx.sess);
 
-            let empty_tables = ty::TypeckTables::empty(None);
-            let annotation = TypedAnnotation { tcx, tables: Cell::new(&empty_tables) };
+            let annotation = TypedAnnotation { tcx, maybe_typeck_tables: Cell::new(None) };
             tcx.dep_graph.with_ignore(|| f(&annotation, tcx.hir().krate()))
         }
         _ => panic!("Should use call_with_pp_support"),
@@ -304,12 +303,22 @@ impl<'a> pprust::PpAnn for HygieneAnnotation<'a> {
     }
 }
 
-struct TypedAnnotation<'a, 'tcx> {
+struct TypedAnnotation<'tcx> {
     tcx: TyCtxt<'tcx>,
-    tables: Cell<&'a ty::TypeckTables<'tcx>>,
+    maybe_typeck_tables: Cell<Option<&'tcx ty::TypeckTables<'tcx>>>,
 }
 
-impl<'b, 'tcx> HirPrinterSupport<'tcx> for TypedAnnotation<'b, 'tcx> {
+impl<'tcx> TypedAnnotation<'tcx> {
+    /// Gets the type-checking side-tables for the current body.
+    /// As this will ICE if called outside bodies, only call when working with
+    /// `Expr` or `Pat` nodes (they are guaranteed to be found only in bodies).
+    #[track_caller]
+    fn tables(&self) -> &'tcx ty::TypeckTables<'tcx> {
+        self.maybe_typeck_tables.get().expect("`TypedAnnotation::tables` called outside of body")
+    }
+}
+
+impl<'tcx> HirPrinterSupport<'tcx> for TypedAnnotation<'tcx> {
     fn sess(&self) -> &Session {
         &self.tcx.sess
     }
@@ -327,15 +336,15 @@ impl<'b, 'tcx> HirPrinterSupport<'tcx> for TypedAnnotation<'b, 'tcx> {
     }
 }
 
-impl<'a, 'tcx> pprust_hir::PpAnn for TypedAnnotation<'a, 'tcx> {
+impl<'tcx> pprust_hir::PpAnn for TypedAnnotation<'tcx> {
     fn nested(&self, state: &mut pprust_hir::State<'_>, nested: pprust_hir::Nested) {
-        let old_tables = self.tables.get();
+        let old_maybe_typeck_tables = self.maybe_typeck_tables.get();
         if let pprust_hir::Nested::Body(id) = nested {
-            self.tables.set(self.tcx.body_tables(id));
+            self.maybe_typeck_tables.set(Some(self.tcx.body_tables(id)));
         }
         let pp_ann = &(&self.tcx.hir() as &dyn hir::intravisit::Map<'_>);
         pprust_hir::PpAnn::nested(pp_ann, state, nested);
-        self.tables.set(old_tables);
+        self.maybe_typeck_tables.set(old_maybe_typeck_tables);
     }
     fn pre(&self, s: &mut pprust_hir::State<'_>, node: pprust_hir::AnnNode<'_>) {
         if let pprust_hir::AnnNode::Expr(_) = node {
@@ -347,7 +356,7 @@ impl<'a, 'tcx> pprust_hir::PpAnn for TypedAnnotation<'a, 'tcx> {
             s.s.space();
             s.s.word("as");
             s.s.space();
-            s.s.word(self.tables.get().expr_ty(expr).to_string());
+            s.s.word(self.tables().expr_ty(expr).to_string());
             s.pclose();
         }
     }

src/librustc_infer/infer/error_reporting/mod.rs

@@ -1684,7 +1684,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         // Attempt to obtain the span of the parameter so we can
         // suggest adding an explicit lifetime bound to it.
         let generics =
-            self.in_progress_tables.and_then(|table| table.borrow().hir_owner).map(|table_owner| {
+            self.in_progress_tables.map(|table| table.borrow().hir_owner).map(|table_owner| {
                 let hir_id = hir.as_local_hir_id(table_owner);
                 let parent_id = hir.get_parent_item(hir_id);
                 (

src/librustc_infer/infer/mod.rs

@@ -588,7 +588,7 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
     /// Used only by `rustc_typeck` during body type-checking/inference,
     /// will initialize `in_progress_tables` with fresh `TypeckTables`.
     pub fn with_fresh_in_progress_tables(mut self, table_owner: LocalDefId) -> Self {
-        self.fresh_tables = Some(RefCell::new(ty::TypeckTables::empty(Some(table_owner))));
+        self.fresh_tables = Some(RefCell::new(ty::TypeckTables::new(table_owner)));
         self
     }
 

src/librustc_lint/context.rs

@@ -427,15 +427,12 @@ pub struct LateContext<'a, 'tcx> {
     /// Current body, or `None` if outside a body.
     pub enclosing_body: Option<hir::BodyId>,
 
-    /// Type-checking side-tables for the current body. Access using the
-    /// `tables` method, which handles querying the tables on demand.
+    /// Type-checking side-tables for the current body. Access using the `tables`
+    /// and `maybe_tables` methods, which handle querying the tables on demand.
     // FIXME(eddyb) move all the code accessing internal fields like this,
     // to this module, to avoid exposing it to lint logic.
     pub(super) cached_typeck_tables: Cell<Option<&'tcx ty::TypeckTables<'tcx>>>,
 
-    // HACK(eddyb) replace this with having `Option` around `&TypeckTables`.
-    pub(super) empty_typeck_tables: &'a ty::TypeckTables<'tcx>,
-
     /// Parameter environment for the item we are in.
     pub param_env: ty::ParamEnv<'tcx>,
 
@@ -677,19 +674,23 @@ impl LintContext for EarlyContext<'_> {
 
 impl<'a, 'tcx> LateContext<'a, 'tcx> {
     /// Gets the type-checking side-tables for the current body,
-    /// or empty `TypeckTables` if outside a body.
-    // FIXME(eddyb) return `Option<&'tcx ty::TypeckTables<'tcx>>`,
-    // where `None` indicates we're outside a body.
-    pub fn tables(&self) -> &'a ty::TypeckTables<'tcx> {
-        if let Some(body) = self.enclosing_body {
-            self.cached_typeck_tables.get().unwrap_or_else(|| {
+    /// or `None` if outside a body.
+    pub fn maybe_tables(&self) -> Option<&'tcx ty::TypeckTables<'tcx>> {
+        self.cached_typeck_tables.get().or_else(|| {
+            self.enclosing_body.map(|body| {
                 let tables = self.tcx.body_tables(body);
                 self.cached_typeck_tables.set(Some(tables));
                 tables
             })
-        } else {
-            self.empty_typeck_tables
-        }
+        })
+    }
+
+    /// Gets the type-checking side-tables for the current body.
+    /// As this will ICE if called outside bodies, only call when working with
+    /// `Expr` or `Pat` nodes (they are guaranteed to be found only in bodies).
+    #[track_caller]
+    pub fn tables(&self) -> &'tcx ty::TypeckTables<'tcx> {
+        self.maybe_tables().expect("`LateContext::tables` called outside of body")
     }
 
     pub fn current_lint_root(&self) -> hir::HirId {

src/librustc_lint/late.rs

@@ -378,7 +378,6 @@ fn late_lint_mod_pass<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(
         tcx,
         enclosing_body: None,
         cached_typeck_tables: Cell::new(None),
-        empty_typeck_tables: &ty::TypeckTables::empty(None),
         param_env: ty::ParamEnv::empty(),
         access_levels,
         lint_store: unerased_lint_store(tcx),
@@ -427,7 +426,6 @@ fn late_lint_pass_crate<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(tcx: TyCtxt<'tc
         tcx,
         enclosing_body: None,
         cached_typeck_tables: Cell::new(None),
-        empty_typeck_tables: &ty::TypeckTables::empty(None),
         param_env: ty::ParamEnv::empty(),
         access_levels,
         lint_store: unerased_lint_store(tcx),

src/librustc_lint/lib.rs

@@ -34,6 +34,7 @@
 #![feature(never_type)]
 #![feature(nll)]
 #![feature(or_patterns)]
+#![feature(track_caller)]
 #![recursion_limit = "256"]
 
 #[macro_use]

src/librustc_middle/ty/context.rs

@@ -188,7 +188,7 @@ pub struct CommonConsts<'tcx> {
 }
 
 pub struct LocalTableInContext<'a, V> {
-    hir_owner: Option<LocalDefId>,
+    hir_owner: LocalDefId,
     data: &'a ItemLocalMap<V>,
 }
 
@@ -199,42 +199,27 @@ pub struct LocalTableInContext<'a, V> {
 /// would be in a different frame of reference and using its `local_id`
 /// would result in lookup errors, or worse, in silently wrong data being
 /// stored/returned.
-fn validate_hir_id_for_typeck_tables(
-    hir_owner: Option<LocalDefId>,
-    hir_id: hir::HirId,
-    mut_access: bool,
-) {
-    if let Some(hir_owner) = hir_owner {
-        if hir_id.owner != hir_owner {
-            ty::tls::with(|tcx| {
-                bug!(
-                    "node {} with HirId::owner {:?} cannot be placed in TypeckTables with hir_owner {:?}",
-                    tcx.hir().node_to_string(hir_id),
-                    hir_id.owner,
-                    hir_owner
-                )
-            });
-        }
-    } else {
-        // We use "Null Object" TypeckTables in some of the analysis passes.
-        // These are just expected to be empty and their `hir_owner` is
-        // `None`. Therefore we cannot verify whether a given `HirId` would
-        // be a valid key for the given table. Instead we make sure that
-        // nobody tries to write to such a Null Object table.
-        if mut_access {
-            bug!("access to invalid TypeckTables")
-        }
+fn validate_hir_id_for_typeck_tables(hir_owner: LocalDefId, hir_id: hir::HirId) {
+    if hir_id.owner != hir_owner {
+        ty::tls::with(|tcx| {
+            bug!(
+                "node {} with HirId::owner {:?} cannot be placed in TypeckTables with hir_owner {:?}",
+                tcx.hir().node_to_string(hir_id),
+                hir_id.owner,
+                hir_owner
+            )
+        });
     }
 }
 
 impl<'a, V> LocalTableInContext<'a, V> {
     pub fn contains_key(&self, id: hir::HirId) -> bool {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, false);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.data.contains_key(&id.local_id)
     }
 
     pub fn get(&self, id: hir::HirId) -> Option<&V> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, false);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.data.get(&id.local_id)
     }
 
@@ -252,28 +237,28 @@ impl<'a, V> ::std::ops::Index<hir::HirId> for LocalTableInContext<'a, V> {
 }
 
 pub struct LocalTableInContextMut<'a, V> {
-    hir_owner: Option<LocalDefId>,
+    hir_owner: LocalDefId,
     data: &'a mut ItemLocalMap<V>,
 }
 
 impl<'a, V> LocalTableInContextMut<'a, V> {
     pub fn get_mut(&mut self, id: hir::HirId) -> Option<&mut V> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, true);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.data.get_mut(&id.local_id)
     }
 
     pub fn entry(&mut self, id: hir::HirId) -> Entry<'_, hir::ItemLocalId, V> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, true);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.data.entry(id.local_id)
     }
 
     pub fn insert(&mut self, id: hir::HirId, val: V) -> Option<V> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, true);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.data.insert(id.local_id, val)
     }
 
     pub fn remove(&mut self, id: hir::HirId) -> Option<V> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, true);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.data.remove(&id.local_id)
     }
 }
@@ -324,7 +309,7 @@ pub struct GeneratorInteriorTypeCause<'tcx> {
 #[derive(RustcEncodable, RustcDecodable, Debug)]
 pub struct TypeckTables<'tcx> {
     /// The `HirId::owner` all `ItemLocalId`s in this table are relative to.
-    pub hir_owner: Option<LocalDefId>,
+    pub hir_owner: LocalDefId,
 
     /// Resolved definitions for `<T>::X` associated paths and
     /// method calls, including those of overloaded operators.
@@ -432,7 +417,7 @@ pub struct TypeckTables<'tcx> {
 }
 
 impl<'tcx> TypeckTables<'tcx> {
-    pub fn empty(hir_owner: Option<LocalDefId>) -> TypeckTables<'tcx> {
+    pub fn new(hir_owner: LocalDefId) -> TypeckTables<'tcx> {
         TypeckTables {
             hir_owner,
             type_dependent_defs: Default::default(),
@@ -474,7 +459,7 @@ impl<'tcx> TypeckTables<'tcx> {
     }
 
     pub fn type_dependent_def(&self, id: HirId) -> Option<(DefKind, DefId)> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, false);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.type_dependent_defs.get(&id.local_id).cloned().and_then(|r| r.ok())
     }
 
@@ -521,7 +506,7 @@ impl<'tcx> TypeckTables<'tcx> {
     }
 
     pub fn node_type_opt(&self, id: hir::HirId) -> Option<Ty<'tcx>> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, false);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.node_types.get(&id.local_id).cloned()
     }
 
@@ -530,12 +515,12 @@ impl<'tcx> TypeckTables<'tcx> {
     }
 
     pub fn node_substs(&self, id: hir::HirId) -> SubstsRef<'tcx> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, false);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.node_substs.get(&id.local_id).cloned().unwrap_or_else(|| InternalSubsts::empty())
     }
 
     pub fn node_substs_opt(&self, id: hir::HirId) -> Option<SubstsRef<'tcx>> {
-        validate_hir_id_for_typeck_tables(self.hir_owner, id, false);
+        validate_hir_id_for_typeck_tables(self.hir_owner, id);
         self.node_substs.get(&id.local_id).cloned()
     }
 
@@ -578,7 +563,7 @@ impl<'tcx> TypeckTables<'tcx> {
     }
 
     pub fn expr_adjustments(&self, expr: &hir::Expr<'_>) -> &[ty::adjustment::Adjustment<'tcx>] {
-        validate_hir_id_for_typeck_tables(self.hir_owner, expr.hir_id, false);
+        validate_hir_id_for_typeck_tables(self.hir_owner, expr.hir_id);
         self.adjustments.get(&expr.hir_id.local_id).map_or(&[], |a| &a[..])
     }
 
@@ -657,7 +642,7 @@ impl<'tcx> TypeckTables<'tcx> {
     }
 
     pub fn is_coercion_cast(&self, hir_id: hir::HirId) -> bool {
-        validate_hir_id_for_typeck_tables(self.hir_owner, hir_id, true);
+        validate_hir_id_for_typeck_tables(self.hir_owner, hir_id);
         self.coercion_casts.contains(&hir_id.local_id)
     }
 
@@ -710,7 +695,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckTables<'tcx> {
             hash_stable_hashmap(hcx, hasher, upvar_capture_map, |up_var_id, hcx| {
                 let ty::UpvarId { var_path, closure_expr_id } = *up_var_id;
 
-                assert_eq!(Some(var_path.hir_id.owner), hir_owner);
+                assert_eq!(var_path.hir_id.owner, hir_owner);
 
                 (
                     hcx.local_def_path_hash(var_path.hir_id.owner),