Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Jul 2, 2020
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