Skip to content

Commit

Permalink
Auto merge of #73743 - eddyb:lint-on-demand-typeck-tables, r=Manishearth
Browse files Browse the repository at this point in the history
rustc_lint: only query `typeck_tables_of` when a lint needs it.

This was prompted by @jyn514 running into a situation where `rustdoc` wants to run the `unused_doc` lint without triggering type-checking (as an alternative to the "everybody loops" approach - type-checking may error/ICE because of the `rustdoc` feature of allowing multi-platform docs where the actual bodies of functions may refer to APIs for different platforms).

There was also this comment in the source:
```rust
// FIXME: Make this lazy to avoid running the TypeckTables query?
```

The main effect of this is for lint authors, who now need to use `cx.tables()` to get `&TypeckTables`, as opposed to having them always available in `cx.tables`.

r? @oli-obk or @Manishearth
  • Loading branch information
bors committed Jun 26, 2020
2 parents e093b65 + 0cf4b9d commit 14e65d5
Show file tree
Hide file tree
Showing 106 changed files with 441 additions and 401 deletions.
8 changes: 4 additions & 4 deletions src/librustc_lint/array_into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIntoIter {

// Check if the method call actually calls the libcore
// `IntoIterator::into_iter`.
let def_id = cx.tables.type_dependent_def_id(expr.hir_id).unwrap();
let def_id = cx.tables().type_dependent_def_id(expr.hir_id).unwrap();
match cx.tcx.trait_of_item(def_id) {
Some(trait_id) if cx.tcx.is_diagnostic_item(sym::IntoIterator, trait_id) => {}
_ => return,
Expand All @@ -45,7 +45,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIntoIter {
// `Box` is the only thing that values can be moved out of via
// method call. `Box::new([1]).into_iter()` should trigger this
// lint.
let mut recv_ty = cx.tables.expr_ty(receiver_arg);
let mut recv_ty = cx.tables().expr_ty(receiver_arg);
let mut num_box_derefs = 0;
while recv_ty.is_box() {
num_box_derefs += 1;
Expand All @@ -60,13 +60,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIntoIter {
// Make sure that there is an autoref coercion at the expected
// position. The first `num_box_derefs` adjustments are the derefs
// of the box.
match cx.tables.expr_adjustments(receiver_arg).get(num_box_derefs) {
match cx.tables().expr_adjustments(receiver_arg).get(num_box_derefs) {
Some(Adjustment { kind: Adjust::Borrow(_), .. }) => {}
_ => return,
}

// Emit lint diagnostic.
let target = match cx.tables.expr_ty_adjusted(receiver_arg).kind {
let target = match cx.tables().expr_ty_adjusted(receiver_arg).kind {
ty::Ref(_, ty::TyS { kind: ty::Array(..), .. }, _) => "[T; N]",
ty::Ref(_, ty::TyS { kind: ty::Slice(..), .. }, _) => "[T]",

Expand Down
20 changes: 10 additions & 10 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxPointers {
}

fn check_expr(&mut self, cx: &LateContext<'_, '_>, e: &hir::Expr<'_>) {
let ty = cx.tables.node_type(e.hir_id);
let ty = cx.tables().node_type(e.hir_id);
self.check_heap_type(cx, e.span, ty);
}
}
Expand All @@ -161,11 +161,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonShorthandFieldPatterns {
fn check_pat(&mut self, cx: &LateContext<'_, '_>, pat: &hir::Pat<'_>) {
if let PatKind::Struct(ref qpath, field_pats, _) = pat.kind {
let variant = cx
.tables
.tables()
.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.tables().qpath_res(qpath, pat.hir_id));
for fieldpat in field_pats {
if fieldpat.is_shorthand {
continue;
Expand All @@ -178,7 +178,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonShorthandFieldPatterns {
}
if let PatKind::Binding(binding_annot, _, ident, None) = fieldpat.pat.kind {
if cx.tcx.find_field_index(ident, &variant)
== Some(cx.tcx.field_index(fieldpat.hir_id, cx.tables))
== Some(cx.tcx.field_index(fieldpat.hir_id, cx.tables()))
{
cx.struct_span_lint(NON_SHORTHAND_FIELD_PATTERNS, fieldpat.span, |lint| {
let mut err = lint
Expand Down Expand Up @@ -901,15 +901,15 @@ 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.tables().qpath_res(qpath, expr.hir_id)
} else {
return None;
};
if let Res::Def(DefKind::Fn, did) = def {
if !def_id_is_transmute(cx, did) {
return None;
}
let sig = cx.tables.node_type(expr.hir_id).fn_sig(cx.tcx);
let sig = cx.tables().node_type(expr.hir_id).fn_sig(cx.tcx);
let from = sig.inputs().skip_binder()[0];
let to = *sig.output().skip_binder();
return Some((from, to));
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.tables().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 @@ -1905,14 +1905,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
}
} else if let hir::ExprKind::MethodCall(_, _, ref args, _) = expr.kind {
// Find problematic calls to `MaybeUninit::assume_init`.
let def_id = cx.tables.type_dependent_def_id(expr.hir_id)?;
let def_id = cx.tables().type_dependent_def_id(expr.hir_id)?;
if cx.tcx.is_diagnostic_item(sym::assume_init, def_id) {
// This is a call to *some* method named `assume_init`.
// 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()?;
cx.tables().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 Expand Up @@ -2025,7 +2025,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
// This conjures an instance of a type out of nothing,
// using zeroed or uninitialized memory.
// We are extremely conservative with what we warn about.
let conjured_ty = cx.tables.expr_ty(expr);
let conjured_ty = cx.tables().expr_ty(expr);
if let Some((msg, span)) = ty_find_init_error(cx.tcx, conjured_ty, init) {
cx.struct_span_lint(INVALID_VALUE, expr.span, |lint| {
let mut err = lint.build(&format!(
Expand Down
31 changes: 28 additions & 3 deletions src/librustc_lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use rustc_session::Session;
use rustc_span::{symbol::Symbol, MultiSpan, Span, DUMMY_SP};
use rustc_target::abi::LayoutOf;

use std::cell::Cell;
use std::slice;

/// Information about the registered lints.
Expand Down Expand Up @@ -423,9 +424,17 @@ pub struct LateContext<'a, 'tcx> {
/// Type context we're checking in.
pub tcx: TyCtxt<'tcx>,

/// Side-tables for the body we are in.
// FIXME: Make this lazy to avoid running the TypeckTables query?
pub tables: &'a ty::TypeckTables<'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.
// 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 @@ -667,6 +676,22 @@ 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(|| {
let tables = self.tcx.body_tables(body);
self.cached_typeck_tables.set(Some(tables));
tables
})
} else {
self.empty_typeck_tables
}
}

pub fn current_lint_root(&self) -> hir::HirId {
self.last_node_with_lint_attrs
}
Expand Down
39 changes: 29 additions & 10 deletions src/librustc_lint/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use rustc_span::Span;

use log::debug;
use std::any::Any;
use std::cell::Cell;
use std::slice;

/// Extract the `LintStore` from the query context.
Expand Down Expand Up @@ -104,12 +105,25 @@ impl<'a, 'tcx, T: LateLintPass<'a, 'tcx>> hir_visit::Visitor<'tcx>
hir_visit::NestedVisitorMap::All(self.context.tcx.hir())
}

fn visit_nested_body(&mut self, body: hir::BodyId) {
let old_tables = self.context.tables;
self.context.tables = self.context.tcx.body_tables(body);
let body = self.context.tcx.hir().body(body);
fn visit_nested_body(&mut self, body_id: hir::BodyId) {
let old_enclosing_body = self.context.enclosing_body.replace(body_id);
let old_cached_typeck_tables = self.context.cached_typeck_tables.get();

// HACK(eddyb) avoid trashing `cached_typeck_tables` when we're
// nested in `visit_fn`, which may have already resulted in them
// being queried.
if old_enclosing_body != Some(body_id) {
self.context.cached_typeck_tables.set(None);
}

let body = self.context.tcx.hir().body(body_id);
self.visit_body(body);
self.context.tables = old_tables;
self.context.enclosing_body = old_enclosing_body;

// See HACK comment above.
if old_enclosing_body != Some(body_id) {
self.context.cached_typeck_tables.set(old_cached_typeck_tables);
}
}

fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
Expand Down Expand Up @@ -181,13 +195,14 @@ impl<'a, 'tcx, T: LateLintPass<'a, 'tcx>> hir_visit::Visitor<'tcx>
) {
// Wrap in tables here, not just in visit_nested_body,
// in order for `check_fn` to be able to use them.
let old_tables = self.context.tables;
self.context.tables = self.context.tcx.body_tables(body_id);
let old_enclosing_body = self.context.enclosing_body.replace(body_id);
let old_cached_typeck_tables = self.context.cached_typeck_tables.take();
let body = self.context.tcx.hir().body(body_id);
lint_callback!(self, check_fn, fk, decl, body, span, id);
hir_visit::walk_fn(self, fk, decl, body_id, span, id);
lint_callback!(self, check_fn_post, fk, decl, body, span, id);
self.context.tables = old_tables;
self.context.enclosing_body = old_enclosing_body;
self.context.cached_typeck_tables.set(old_cached_typeck_tables);
}

fn visit_variant_data(
Expand Down Expand Up @@ -361,7 +376,9 @@ fn late_lint_mod_pass<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(

let context = LateContext {
tcx,
tables: &ty::TypeckTables::empty(None),
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 @@ -408,7 +425,9 @@ fn late_lint_pass_crate<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>(tcx: TyCtxt<'tc

let context = LateContext {
tcx,
tables: &ty::TypeckTables::empty(None),
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
9 changes: 5 additions & 4 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ fn report_bin_hex_error(
the type `{}` and will become `{}{}`",
repr_str, val, t, actually, t
));
if let Some(sugg_ty) = get_type_suggestion(&cx.tables.node_type(expr.hir_id), val, negative)
if let Some(sugg_ty) =
get_type_suggestion(&cx.tables().node_type(expr.hir_id), val, negative)
{
if let Some(pos) = repr_str.chars().position(|c| c == 'i' || c == 'u') {
let (sans_suffix, _) = repr_str.split_at(pos);
Expand Down Expand Up @@ -301,7 +302,7 @@ fn lint_uint_literal<'a, 'tcx>(
if let Node::Expr(par_e) = cx.tcx.hir().get(parent_id) {
match par_e.kind {
hir::ExprKind::Cast(..) => {
if let ty::Char = cx.tables.expr_ty(par_e).kind {
if let ty::Char = cx.tables().expr_ty(par_e).kind {
cx.struct_span_lint(OVERFLOWING_LITERALS, par_e.span, |lint| {
lint.build("only `u8` can be cast into `char`")
.span_suggestion(
Expand Down Expand Up @@ -352,7 +353,7 @@ fn lint_literal<'a, 'tcx>(
e: &'tcx hir::Expr<'tcx>,
lit: &hir::Lit,
) {
match cx.tables.node_type(e.hir_id).kind {
match cx.tables().node_type(e.hir_id).kind {
ty::Int(t) => {
match lit.node {
ast::LitKind::Int(v, ast::LitIntType::Signed(_) | ast::LitIntType::Unsuffixed) => {
Expand Down Expand Up @@ -448,7 +449,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypeLimits {
// Normalize the binop so that the literal is always on the RHS in
// the comparison
let norm_binop = if swap { rev_binop(binop) } else { binop };
match cx.tables.node_type(expr.hir_id).kind {
match cx.tables().node_type(expr.hir_id).kind {
ty::Int(int_ty) => {
let (min, max) = int_ty_range(int_ty);
let lit_val: i128 = match lit.kind {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
return;
}

let ty = cx.tables.expr_ty(&expr);
let ty = cx.tables().expr_ty(&expr);
let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, s.span, "", "", 1);

let mut fn_warned = false;
Expand All @@ -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.tables().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 All @@ -65,7 +65,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
_ => None,
}
}
hir::ExprKind::MethodCall(..) => cx.tables.type_dependent_def_id(expr.hir_id),
hir::ExprKind::MethodCall(..) => cx.tables().type_dependent_def_id(expr.hir_id),
_ => None,
};
if let Some(def_id) = maybe_def_id {
Expand Down Expand Up @@ -950,7 +950,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAllocation {
_ => return,
}

for adj in cx.tables.expr_adjustments(e) {
for adj in cx.tables().expr_adjustments(e) {
if let adjustment::Adjust::Borrow(adjustment::AutoBorrow::Ref(_, m)) = adj.kind {
cx.struct_span_lint(UNUSED_ALLOCATION, e.span, |lint| {
let msg = match m {
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
_ => (),
}

let (l_ty, r_ty) = (cx.tables.expr_ty(l), cx.tables.expr_ty(r));
let (l_ty, r_ty) = (cx.tables().expr_ty(l), cx.tables().expr_ty(r));
if l_ty.peel_refs().is_integral() && r_ty.peel_refs().is_integral() {
span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
self.expr_span = Some(expr.span);
Expand All @@ -96,8 +96,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
}
},
hir::ExprKind::Unary(hir::UnOp::UnNeg, arg) => {
let ty = cx.tables.expr_ty(arg);
if constant_simple(cx, cx.tables, expr).is_none() {
let ty = cx.tables().expr_ty(arg);
if constant_simple(cx, cx.tables(), expr).is_none() {
if ty.is_integral() {
span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
self.expr_span = Some(expr.span);
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
}
if_chain! {
if let ExprKind::Unary(_, ref lit) = e.kind;
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables, lit);
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables(), lit);
if is_true;
then {
lint_true(true);
Expand Down Expand Up @@ -121,7 +121,7 @@ fn match_assert_with_message<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx E
if let ExprKind::DropTemps(ref expr) = expr.kind;
if let ExprKind::Unary(UnOp::UnNot, ref expr) = expr.kind;
// bind the first argument of the `assert!` macro
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables, expr);
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables(), expr);
// arm 1 pattern
if let PatKind::Lit(ref lit_expr) = arms[0].pat.kind;
if let ExprKind::Lit(ref lit) = lit_expr.kind;
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/assign_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
hir::ExprKind::Assign(assignee, e, _) => {
if let hir::ExprKind::Binary(op, l, r) = &e.kind {
let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| {
let ty = cx.tables.expr_ty(assignee);
let rty = cx.tables.expr_ty(rhs);
let ty = cx.tables().expr_ty(assignee);
let rty = cx.tables().expr_ty(rhs);
macro_rules! ops {
($op:expr,
$cx:expr,
Expand Down Expand Up @@ -167,7 +167,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
// a = b commutative_op a
// Limited to primitive type as these ops are know to be commutative
if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, r)
&& cx.tables.expr_ty(assignee).is_primitive_ty()
&& cx.tables().expr_ty(assignee).is_primitive_ty()
{
match op.node {
hir::BinOpKind::Add
Expand Down
Loading

0 comments on commit 14e65d5

Please sign in to comment.