Auto merge of rust-lang#73743 - eddyb:lint-on-demand-typeck-tables, r…

…=Manishearth

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

src/librustc_lint/array_into_iter.rs

@@ -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,
@@ -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;
@@ -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]",
 

src/librustc_lint/builtin.rs

@@ -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);
     }
 }
@@ -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;
@@ -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
@@ -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));
@@ -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);
@@ -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);
@@ -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!(

src/librustc_lint/context.rs

@@ -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.
@@ -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>,
@@ -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
     }

src/librustc_lint/late.rs

@@ -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.
@@ -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>) {
@@ -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(
@@ -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),
@@ -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),

src/librustc_lint/types.rs

@@ -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);
@@ -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(
@@ -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) => {
@@ -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 {

src/librustc_lint/unused.rs

@@ -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;
@@ -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
@@ -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 {
@@ -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 {

src/tools/clippy/clippy_lints/src/arithmetic.rs

@@ -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);
@@ -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);

src/tools/clippy/clippy_lints/src/assertions_on_constants.rs

@@ -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);
@@ -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;

src/tools/clippy/clippy_lints/src/assign_ops.rs

@@ -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,
@@ -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