Skip to content

Commit

Permalink
Auto merge of #73751 - eddyb:no-empty-tables, r=nikomatsakis
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Jul 2, 2020
2 parents 8a6d434 + 4b2d9e6 commit 3503f56
Show file tree
Hide file tree
Showing 47 changed files with 355 additions and 371 deletions.
1 change: 1 addition & 0 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/")]
#![feature(nll)]
#![cfg_attr(bootstrap, feature(track_caller))]
#![recursion_limit = "256"]

#[macro_use]
Expand Down
29 changes: 19 additions & 10 deletions src/librustc_driver/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
9 changes: 4 additions & 5 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonShorthandFieldPatterns {
.pat_ty(pat)
.ty_adt_def()
.expect("struct pattern type is not an ADT")
.variant_of_res(cx.tables().qpath_res(qpath, pat.hir_id));
.variant_of_res(cx.qpath_res(qpath, pat.hir_id));
for fieldpat in field_pats {
if fieldpat.is_shorthand {
continue;
Expand Down Expand Up @@ -901,7 +901,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MutableTransmutes {
expr: &hir::Expr<'_>,
) -> Option<(Ty<'tcx>, Ty<'tcx>)> {
let def = if let hir::ExprKind::Path(ref qpath) = expr.kind {
cx.tables().qpath_res(qpath, expr.hir_id)
cx.qpath_res(qpath, expr.hir_id)
} else {
return None;
};
Expand Down Expand Up @@ -1891,7 +1891,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
if let hir::ExprKind::Call(ref path_expr, ref args) = expr.kind {
// Find calls to `mem::{uninitialized,zeroed}` methods.
if let hir::ExprKind::Path(ref qpath) = path_expr.kind {
let def_id = cx.tables().qpath_res(qpath, path_expr.hir_id).opt_def_id()?;
let def_id = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id()?;

if cx.tcx.is_diagnostic_item(sym::mem_zeroed, def_id) {
return Some(InitKind::Zeroed);
Expand All @@ -1911,8 +1911,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
// See if the `self` parameter is one of the dangerous constructors.
if let hir::ExprKind::Call(ref path_expr, _) = args[0].kind {
if let hir::ExprKind::Path(ref qpath) = path_expr.kind {
let def_id =
cx.tables().qpath_res(qpath, path_expr.hir_id).opt_def_id()?;
let def_id = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id()?;

if cx.tcx.is_diagnostic_item(sym::maybe_uninit_zeroed, def_id) {
return Some(InitKind::Zeroed);
Expand Down
41 changes: 28 additions & 13 deletions src/librustc_lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync;
use rustc_errors::{struct_span_err, Applicability};
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::def_id::{CrateNum, DefId};
use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
use rustc_middle::lint::LintDiagnosticBuilder;
Expand Down Expand Up @@ -427,15 +428,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>,

Expand Down Expand Up @@ -677,18 +675,35 @@ 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_typeck_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_typeck_tables().expect("`LateContext::tables` called outside of body")
}

/// Returns the final resolution of a `QPath`, or `Res::Err` if unavailable.
/// Unlike `.tables().qpath_res(qpath, id)`, this can be used even outside
/// bodies (e.g. for paths in `hir::Ty`), without any risk of ICE-ing.
pub fn qpath_res(&self, qpath: &hir::QPath<'_>, id: hir::HirId) -> Res {
match *qpath {
hir::QPath::Resolved(_, ref path) => path.res,
hir::QPath::TypeRelative(..) => self
.maybe_typeck_tables()
.and_then(|tables| tables.type_dependent_def(id))
.map_or(Res::Err, |(kind, def_id)| Res::Def(kind, def_id)),
}
}

Expand Down
2 changes: 0 additions & 2 deletions src/librustc_lint/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#![feature(never_type)]
#![feature(nll)]
#![feature(or_patterns)]
#![cfg_attr(bootstrap, feature(track_caller))]
#![recursion_limit = "256"]

#[macro_use]
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
hir::ExprKind::Call(ref callee, _) => {
match callee.kind {
hir::ExprKind::Path(ref qpath) => {
match cx.tables().qpath_res(qpath, callee.hir_id) {
match cx.qpath_res(qpath, callee.hir_id) {
Res::Def(DefKind::Fn | DefKind::AssocFn, def_id) => Some(def_id),
// `Res::Local` if it was a closure, for which we
// do not currently support must-use linting
Expand Down
Loading

0 comments on commit 3503f56

Please sign in to comment.