[WIP] Run the unused variables lint on the MIR

Cargo.lock

@@ -4164,6 +4164,7 @@ dependencies = [
  "rustc_infer",
  "rustc_macros",
  "rustc_middle",
+ "rustc_mir",
  "rustc_session",
  "rustc_span",
  "rustc_target",

src/librustc_interface/passes.rs

@@ -823,8 +823,6 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> {
                         //
                         // maybe move the check to a MIR pass?
                         let local_def_id = tcx.hir().local_def_id(module);
-
-                        tcx.ensure().check_mod_liveness(local_def_id);
                         tcx.ensure().check_mod_intrinsics(local_def_id);
                     });
                 });

src/librustc_middle/lint.rs

@@ -12,7 +12,7 @@ use rustc_span::source_map::{DesugaringKind, ExpnKind, MultiSpan};
 use rustc_span::{Span, Symbol};
 
 /// How a lint level was set.
-#[derive(Clone, Copy, PartialEq, Eq, HashStable)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, HashStable, RustcEncodable, RustcDecodable)]
 pub enum LintSource {
     /// Lint is at the default level as declared
     /// in rustc or a plugin.

src/librustc_middle/mir/mod.rs

@@ -2,6 +2,7 @@
 //!
 //! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/mir/index.html
 
+use crate::lint;
 use crate::mir::interpret::{GlobalAlloc, Scalar};
 use crate::mir::visit::MirVisitable;
 use crate::ty::adjustment::PointerCast;
@@ -14,7 +15,7 @@ use crate::ty::{
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, Namespace};
 use rustc_hir::def_id::DefId;
-use rustc_hir::{self, GeneratorKind};
+use rustc_hir::GeneratorKind;
 use rustc_target::abi::VariantIdx;
 
 use polonius_engine::Atom;
@@ -596,6 +597,22 @@ pub enum LocalKind {
 
 #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
 pub struct VarBindingForm<'tcx> {
+    /// Bindings in match arms with guards are lowered to two distinct locals, one for inside the
+    /// guard and one for inside the match arm. If this local is the one used in the match arm,
+    /// `counterpart_in_guard` will be the local representing the same binding in the guard.
+    pub counterpart_in_guard: Option<Local>,
+
+    /// `true` if this variable was declared as a binding to a struct field with the same name
+    /// (e.g., `x` in `Foo { x }`).
+    pub is_shorthand_field_binding: bool,
+
+    /// The name of this variable.
+    pub name: Symbol,
+
+    pub unused_variables_lint_level: lint::LevelSource,
+
+    pub unused_assignments_lint_level: lint::LevelSource,
+
     /// Is variable bound via `x`, `mut x`, `ref x`, or `ref mut x`?
     pub binding_mode: ty::BindingMode,
     /// If an explicit type was provided for this variable binding,
@@ -845,9 +862,7 @@ impl<'tcx> LocalDecl<'tcx> {
         match self.local_info {
             Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
                 binding_mode: ty::BindingMode::BindByValue(_),
-                opt_ty_info: _,
-                opt_match_place: _,
-                pat_span: _,
+                ..
             })))) => true,
 
             Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::ImplicitSelf(
@@ -865,9 +880,7 @@ impl<'tcx> LocalDecl<'tcx> {
         match self.local_info {
             Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
                 binding_mode: ty::BindingMode::BindByValue(_),
-                opt_ty_info: _,
-                opt_match_place: _,
-                pat_span: _,
+                ..
             })))) => true,
 
             Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::ImplicitSelf(_)))) => true,
@@ -1717,6 +1730,14 @@ pub enum FakeReadCause {
     /// appropriate errors.
     ForLet,
 
+    /// This denotes the existence of a statement like
+    ///
+    /// `let _ = <expr>;`
+    ///
+    /// which would not otherwise be recorded in the MIR. This is required for to
+    /// implement the unused variable lint on the MIR. It should be ignored elsewhere.
+    ForLetUnderscore,
+
     /// If we have an index expression like
     ///
     /// (*x)[1][{ x = y; 4}]

src/librustc_middle/mir/visit.rs

@@ -1184,9 +1184,34 @@ impl PlaceContext {
             PlaceContext::MutatingUse(
                 MutatingUseContext::Store
                 | MutatingUseContext::Call
+                | MutatingUseContext::Yield
                 | MutatingUseContext::AsmOutput,
             ) => true,
             _ => false,
         }
     }
+
+    /// Returns `true` if this context could read the contents of the place.
+    pub fn is_read(&self) -> bool {
+        match *self {
+            PlaceContext::MutatingUse(
+                MutatingUseContext::Store | MutatingUseContext::Call | MutatingUseContext::Yield,
+            )
+            | PlaceContext::NonUse(_) => false,
+
+            PlaceContext::MutatingUse(_) | PlaceContext::NonMutatingUse(_) => true,
+        }
+    }
+
+    /// Returns `true` if this context does not read from this place but does write to it.
+    ///
+    /// This is relevant for inline assembly, which has read/write parameters.
+    pub fn is_write_only(&self) -> bool {
+        matches!(
+            self,
+            PlaceContext::MutatingUse(
+                MutatingUseContext::Store | MutatingUseContext::Call | MutatingUseContext::Yield
+            )
+        )
+    }
 }

src/librustc_middle/query/mod.rs

@@ -430,10 +430,6 @@ rustc_queries! {
             desc { |tcx| "checking intrinsics in {}", describe_as_module(key, tcx) }
         }
 
-        query check_mod_liveness(key: DefId) -> () {
-            desc { |tcx| "checking liveness of variables in {}", describe_as_module(key, tcx) }
-        }
-
         query check_mod_impl_wf(key: DefId) -> () {
             desc { |tcx| "checking that impls are well-formed in {}", describe_as_module(key, tcx) }
         }

src/librustc_mir/borrow_check/diagnostics/move_errors.rs

@@ -106,9 +106,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                         if let Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(
                             VarBindingForm {
                                 opt_match_place: Some((opt_match_place, match_span)),
-                                binding_mode: _,
-                                opt_ty_info: _,
-                                pat_span: _,
+                                ..
                             },
                         )))) = local_decl.local_info
                         {

src/librustc_mir/borrow_check/mod.rs

@@ -12,7 +12,7 @@ use rustc_index::bit_set::BitSet;
 use rustc_index::vec::IndexVec;
 use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
 use rustc_middle::mir::{
-    traversal, Body, ClearCrossCrate, Local, Location, Mutability, Operand, Place, PlaceElem,
+    self, traversal, Body, ClearCrossCrate, Local, Location, Mutability, Operand, Place, PlaceElem,
     PlaceRef,
 };
 use rustc_middle::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind};
@@ -571,6 +571,10 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc
 
                 self.mutate_place(location, (*lhs, span), Shallow(None), JustWrite, flow_state);
             }
+
+            // `ForLetUnderscore` is only intended for the unused variables lint.
+            StatementKind::FakeRead(mir::FakeReadCause::ForLetUnderscore, _) => {}
+
             StatementKind::FakeRead(_, box ref place) => {
                 // Read for match doesn't access any memory and is used to
                 // assert that a place is safe and live. So we don't have to

src/librustc_mir/dataflow/impls/liveness.rs

@@ -49,6 +49,9 @@ impl GenKillAnalysis<'tcx> for MaybeLiveLocals {
         statement: &mir::Statement<'tcx>,
         location: Location,
     ) {
+        if let mir::StatementKind::FakeRead(mir::FakeReadCause::ForLet, _) = statement.kind {
+            return;
+        }
         self.transfer_function(trans).visit_statement(statement, location);
     }
 

src/librustc_mir/transform/check_consts/validation.rs

@@ -514,7 +514,10 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
                 self.check_op(ops::InlineAsm);
             }
 
-            StatementKind::FakeRead(FakeReadCause::ForLet | FakeReadCause::ForIndex, _)
+            StatementKind::FakeRead(
+                FakeReadCause::ForLet | FakeReadCause::ForIndex | FakeReadCause::ForLetUnderscore,
+                _,
+            )
             | StatementKind::StorageLive(_)
             | StatementKind::StorageDead(_)
             | StatementKind::Retag { .. }

src/librustc_mir_build/Cargo.toml

@@ -21,6 +21,7 @@ rustc_errors = { path = "../librustc_errors" }
 rustc_hir = { path = "../librustc_hir" }
 rustc_infer = { path = "../librustc_infer" }
 rustc_macros = { path = "../librustc_macros" }
+rustc_mir = { path = "../librustc_mir" } # Just for dataflow
 rustc_serialize = { path = "../libserialize", package = "serialize" }
 rustc_session = { path = "../librustc_session" }
 rustc_span = { path = "../librustc_span" }

src/librustc_mir_build/build/block.rs

@@ -78,6 +78,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
         let source_info = this.source_info(span);
         for stmt in stmts {
+            debug!("ast_block_stmt({:?})", stmt);
+
             let Stmt { kind, opt_destruction_scope } = this.hir.mirror(stmt);
             match kind {
                 StmtKind::Expr { scope, expr } => {
@@ -96,8 +98,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     );
                 }
                 StmtKind::Let { remainder_scope, init_scope, pattern, initializer, lint_level } => {
-                    let ignores_expr_result =
-                        if let PatKind::Wild = *pattern.kind { true } else { false };
+                    let ignores_expr_result = matches!(*pattern.kind, PatKind::Wild);
                     this.block_context.push(BlockFrame::Statement { ignores_expr_result });
 
                     // Enter the remainder scope, i.e., the bindings' destruction scope.

src/librustc_mir_build/build/matches/mod.rs

@@ -16,6 +16,7 @@ use rustc_index::bit_set::BitSet;
 use rustc_middle::middle::region;
 use rustc_middle::mir::*;
 use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty};
+use rustc_session::lint;
 use rustc_span::Span;
 use rustc_span::symbol::Symbol;
 use rustc_target::abi::VariantIdx;
@@ -439,6 +440,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 block.unit()
             }
 
+            // Handle bindings like `let _ = var;` by lowering them to a fake read of the RHS. This
+            // is needed by the unused variables lint pass.
+            PatKind::Wild => {
+                let place = unpack!(block = self.as_place(block, initializer));
+                debug!("expr_into_pattern(place = {:?}, PatKind::Wild)", place);
+
+                if place.as_local().is_some() {
+                    let source_info = self.source_info(irrefutable_pat.span);
+                    self.cfg.push_fake_read(block, source_info, FakeReadCause::ForLetUnderscore, place);
+                }
+
+                block.unit()
+            }
+
             _ => {
                 let place = unpack!(block = self.as_place(block, initializer));
                 self.place_into_pattern(block, irrefutable_pat, place, true)
@@ -1966,6 +1981,31 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             BindingMode::ByRef(_) => ty::BindingMode::BindByReference(mutability),
         };
         debug!("declare_binding: user_ty={:?}", user_ty);
+
+        let ref_for_guard = has_guard.0.then(|| {
+            let local = self.local_decls.push(LocalDecl::<'tcx> {
+                // This variable isn't mutated but has a name, so has to be
+                // immutable to avoid the unused mut lint.
+                mutability: Mutability::Not,
+                ty: tcx.mk_imm_ref(tcx.lifetimes.re_erased, var_ty),
+                user_ty: None,
+                source_info,
+                internal: false,
+                is_block_tail: None,
+                local_info: Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::RefForGuard))),
+            });
+            self.var_debug_info.push(VarDebugInfo {
+                name,
+                source_info: debug_source_info,
+                place: local.into(),
+            });
+            local
+        });
+
+        let name = tcx.hir().name(var_id);
+        let is_shorthand_field_binding = false;
+        let unused_variables_lint_level = tcx.lint_level_at_node(lint::builtin::UNUSED_VARIABLES, var_id);
+        let unused_assignments_lint_level = tcx.lint_level_at_node(lint::builtin::UNUSED_ASSIGNMENTS, var_id);
         let local = LocalDecl::<'tcx> {
             mutability,
             ty: var_ty,
@@ -1974,6 +2014,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             internal: false,
             is_block_tail: None,
             local_info: Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
+                unused_assignments_lint_level,
+                unused_variables_lint_level,
+                is_shorthand_field_binding,
+                counterpart_in_guard: ref_for_guard,
+                name,
                 binding_mode,
                 // hypothetically, `visit_primary_bindings` could try to unzip
                 // an outermost hir::Ty as we descend, matching up
@@ -1990,27 +2035,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             source_info: debug_source_info,
             place: for_arm_body.into(),
         });
-        let locals = if has_guard.0 {
-            let ref_for_guard = self.local_decls.push(LocalDecl::<'tcx> {
-                // This variable isn't mutated but has a name, so has to be
-                // immutable to avoid the unused mut lint.
-                mutability: Mutability::Not,
-                ty: tcx.mk_imm_ref(tcx.lifetimes.re_erased, var_ty),
-                user_ty: None,
-                source_info,
-                internal: false,
-                is_block_tail: None,
-                local_info: Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::RefForGuard))),
-            });
-            self.var_debug_info.push(VarDebugInfo {
-                name,
-                source_info: debug_source_info,
-                place: ref_for_guard.into(),
-            });
-            LocalsForNode::ForGuard { ref_for_guard, for_arm_body }
-        } else {
-            LocalsForNode::One(for_arm_body)
+
+        let locals = match ref_for_guard {
+            Some(ref_for_guard) => LocalsForNode::ForGuard { ref_for_guard, for_arm_body },
+            None => LocalsForNode::One(for_arm_body),
         };
+
         debug!("declare_binding: vars={:?}", locals);
         self.var_indices.insert(var_id, locals);
     }

src/librustc_mir_build/build/mod.rs

@@ -14,6 +14,7 @@ use rustc_middle::middle::region;
 use rustc_middle::mir::*;
 use rustc_middle::ty::subst::Subst;
 use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
+use rustc_session::lint;
 use rustc_span::symbol::kw;
 use rustc_span::Span;
 use rustc_target::spec::abi::Abi;
@@ -181,7 +182,8 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Body<'_> {
             build::construct_const(cx, body_id, return_ty, return_ty_span)
         };
 
-        lints::check(tcx, &body, def_id);
+        lints::unconditional_recursion::check(tcx, &body, def_id);
+        lints::unused_variables::check(tcx, &body, def_id);
 
         // The borrow checker will replace all the regions here with its own
         // inference variables. There's no point having non-erased regions here.
@@ -888,8 +890,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                             Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::ImplicitSelf(*kind))))
                         } else {
                             let binding_mode = ty::BindingMode::BindByValue(mutability);
+                            let unused_variables_lint_level = tcx.lint_level_at_node(lint::builtin::UNUSED_VARIABLES, var);
+                            let unused_assignments_lint_level = tcx.lint_level_at_node(lint::builtin::UNUSED_ASSIGNMENTS, var);
+                            let is_shorthand_field_binding = false;
+                            let name = tcx_hir.name(var);
                             Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(
                                 VarBindingForm {
+                                    is_shorthand_field_binding,
+                                    counterpart_in_guard: None,
+                                    name,
+                                    unused_variables_lint_level,
+                                    unused_assignments_lint_level,
                                     binding_mode,
                                     opt_ty_info,
                                     opt_match_place: Some((Some(place), span)),

src/librustc_mir_build/lib.rs

@@ -9,6 +9,7 @@
 #![feature(const_panic)]
 #![feature(crate_visibility_modifier)]
 #![feature(bool_to_option)]
+#![feature(in_band_lifetimes)]
 #![feature(or_patterns)]
 #![recursion_limit = "256"]
 

src/librustc_mir_build/lints/mod.rs

@@ -0,0 +1,7 @@
+//! Lints that run immediately after MIR building.
+//!
+//! By running these before any MIR transformations, we ensure that the MIR is as close to the
+//! user's code as possible.
+
+pub mod unconditional_recursion;
+pub mod unused_variables;