Rollup merge of rust-lang#55758 - davidtwco:issue-55344, r=pnkfelix

[regression - rust2018]: unused_mut lint false positives on nightly

Fixes rust-lang#55344.

This commit filters out locals that have never been initialized for
consideration in the `unused_mut` lint.

This is intended to detect when the statement that would have
initialized the local was removed as unreachable code. In these cases,
we would not want to lint. This is the same behaviour as the AST borrow
checker.

This is achieved by taking advantage of an existing pass over the MIR
for the `unused_mut` lint and creating a set of those locals that were
never initialized.

r? @pnkfelix

Author: kennytm

Diff for: src/librustc/mir/mod.rs

@@ -304,6 +304,20 @@ impl<'tcx> Mir<'tcx> {
         })
     }
 
+    /// Returns an iterator over all user-declared mutable locals.
+    #[inline]
+    pub fn mut_vars_iter<'a>(&'a self) -> impl Iterator<Item = Local> + 'a {
+        (self.arg_count + 1..self.local_decls.len()).filter_map(move |index| {
+            let local = Local::new(index);
+            let decl = &self.local_decls[local];
+            if decl.is_user_variable.is_some() && decl.mutability == Mutability::Mut {
+                Some(local)
+            } else {
+                None
+            }
+        })
+    }
+
     /// Returns an iterator over all user-declared mutable arguments and locals.
     #[inline]
     pub fn mut_vars_and_args_iter<'a>(&'a self) -> impl Iterator<Item = Local> + 'a {

Diff for: src/librustc_mir/borrow_check/mod.rs

@@ -281,23 +281,21 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
     // Note that this set is expected to be small - only upvars from closures
     // would have a chance of erroneously adding non-user-defined mutable vars
     // to the set.
-    let temporary_used_locals: FxHashSet<Local> = mbcx
-        .used_mut
-        .iter()
+    let temporary_used_locals: FxHashSet<Local> = mbcx.used_mut.iter()
         .filter(|&local| mbcx.mir.local_decls[*local].is_user_variable.is_none())
         .cloned()
         .collect();
-    mbcx.gather_used_muts(temporary_used_locals);
+    // For the remaining unused locals that are marked as mutable, we avoid linting any that
+    // were never initialized. These locals may have been removed as unreachable code; or will be
+    // linted as unused variables.
+    let unused_mut_locals = mbcx.mir.mut_vars_iter()
+        .filter(|local| !mbcx.used_mut.contains(local))
+        .collect();
+    mbcx.gather_used_muts(temporary_used_locals, unused_mut_locals);
 
     debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
-
     let used_mut = mbcx.used_mut;
-
-    for local in mbcx
-        .mir
-        .mut_vars_and_args_iter()
-        .filter(|local| !used_mut.contains(local))
-    {
+    for local in mbcx.mir.mut_vars_and_args_iter().filter(|local| !used_mut.contains(local)) {
         if let ClearCrossCrate::Set(ref vsi) = mbcx.mir.source_scope_local_data {
             let local_decl = &mbcx.mir.local_decls[local];
 

Diff for: src/librustc_mir/borrow_check/used_muts.rs

@@ -9,43 +9,113 @@
 // except according to those terms.
 
 use rustc::mir::visit::{PlaceContext, Visitor};
-use rustc::mir::{Local, Location, Place};
+use rustc::mir::{BasicBlock, Local, Location, Place, Statement, StatementKind, TerminatorKind};
 
 use rustc_data_structures::fx::FxHashSet;
 
 use borrow_check::MirBorrowckCtxt;
 
 impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
-    /// Walks the MIR looking for assignments to a set of locals, as part of the unused mutable
-    /// local variables lint, to update the context's `used_mut` in a single walk.
-    crate fn gather_used_muts(&mut self, locals: FxHashSet<Local>) {
-        let mut visitor = GatherUsedMutsVisitor {
-            needles: locals,
-            mbcx: self,
-        };
-        visitor.visit_mir(visitor.mbcx.mir);
+    /// Walks the MIR adding to the set of `used_mut` locals that will be ignored for the purposes
+    /// of the `unused_mut` lint.
+    ///
+    /// `temporary_used_locals` should contain locals that were found to be temporary, mutable and
+    ///  used from borrow checking. This function looks for assignments into these locals from
+    ///  user-declared locals and adds those user-defined locals to the `used_mut` set. This can
+    ///  occur due to a rare case involving upvars in closures.
+    ///
+    /// `never_initialized_mut_locals` should contain the set of user-declared mutable locals
+    ///  (not arguments) that have not already been marked as being used.
+    ///  This function then looks for assignments from statements or the terminator into the locals
+    ///  from this set and removes them from the set. This leaves only those locals that have not
+    ///  been assigned to - this set is used as a proxy for locals that were not initialized due to
+    ///  unreachable code. These locals are then considered "used" to silence the lint for them.
+    ///  See #55344 for context.
+    crate fn gather_used_muts(
+        &mut self,
+        temporary_used_locals: FxHashSet<Local>,
+        mut never_initialized_mut_locals: FxHashSet<Local>,
+    ) {
+        {
+            let mut visitor = GatherUsedMutsVisitor {
+                temporary_used_locals,
+                never_initialized_mut_locals: &mut never_initialized_mut_locals,
+                mbcx: self,
+            };
+            visitor.visit_mir(visitor.mbcx.mir);
+        }
+
+        // Take the union of the existed `used_mut` set with those variables we've found were
+        // never initialized.
+        debug!("gather_used_muts: never_initialized_mut_locals={:?}", never_initialized_mut_locals);
+        self.used_mut = self.used_mut.union(&never_initialized_mut_locals).cloned().collect();
     }
 }
 
-/// MIR visitor gathering the assignments to a set of locals, in a single walk.
-/// 'visit = the duration of the MIR walk
+/// MIR visitor for collecting used mutable variables.
+/// The 'visit lifetime represents the duration of the MIR walk.
 struct GatherUsedMutsVisitor<'visit, 'cx: 'visit, 'gcx: 'tcx, 'tcx: 'cx> {
-    needles: FxHashSet<Local>,
+    temporary_used_locals: FxHashSet<Local>,
+    never_initialized_mut_locals: &'visit mut FxHashSet<Local>,
     mbcx: &'visit mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>,
 }
 
 impl<'visit, 'cx, 'gcx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'cx, 'gcx, 'tcx> {
+    fn visit_terminator_kind(
+        &mut self,
+        _block: BasicBlock,
+        kind: &TerminatorKind<'tcx>,
+        _location: Location,
+    ) {
+        debug!("visit_terminator_kind: kind={:?}", kind);
+        match &kind {
+            TerminatorKind::Call { destination: Some((into, _)), .. } => {
+                if let Some(local) = into.base_local() {
+                    debug!(
+                        "visit_terminator_kind: kind={:?} local={:?} \
+                         never_initialized_mut_locals={:?}",
+                        kind, local, self.never_initialized_mut_locals
+                    );
+                    let _ = self.never_initialized_mut_locals.remove(&local);
+                }
+            },
+            _ => {},
+        }
+    }
+
+    fn visit_statement(
+        &mut self,
+        _block: BasicBlock,
+        statement: &Statement<'tcx>,
+        _location: Location,
+    ) {
+        match &statement.kind {
+            StatementKind::Assign(into, _) => {
+                // Remove any locals that we found were initialized from the
+                // `never_initialized_mut_locals` set. At the end, the only remaining locals will
+                // be those that were never initialized - we will consider those as being used as
+                // they will either have been removed by unreachable code optimizations; or linted
+                // as unused variables.
+                if let Some(local) = into.base_local() {
+                    debug!(
+                        "visit_statement: statement={:?} local={:?} \
+                         never_initialized_mut_locals={:?}",
+                        statement, local, self.never_initialized_mut_locals
+                    );
+                    let _ = self.never_initialized_mut_locals.remove(&local);
+                }
+            },
+            _ => {},
+        }
+    }
+
     fn visit_local(
         &mut self,
         local: &Local,
         place_context: PlaceContext<'tcx>,
         location: Location,
     ) {
-        if !self.needles.contains(local) {
-            return;
-        }
-
-        if place_context.is_place_assignment() {
+        if place_context.is_place_assignment() && self.temporary_used_locals.contains(local) {
             // Propagate the Local assigned at this Location as a used mutable local variable
             for moi in &self.mbcx.move_data.loc_map[location] {
                 let mpi = &self.mbcx.move_data.moves[*moi].path;

Diff for: src/test/ui/nll/issue-55344.rs

@@ -0,0 +1,26 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-pass
+
+#![feature(nll)]
+#![allow(unreachable_code)]
+#![deny(unused_mut)]
+
+pub fn foo() {
+    return;
+
+    let mut v = 0;
+    assert_eq!(v, 0);
+    v = 1;
+    assert_eq!(v, 1);
+}
+
+fn main() {}