1
Fork 0

Ignore never-initialized locals for unused_mut.

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.
This commit is contained in:
David Wood 2018-11-07 13:40:55 +01:00
parent 24e66c2898
commit 299a452a75
No known key found for this signature in database
GPG key ID: 01760B4F9F53F154
4 changed files with 136 additions and 28 deletions

View file

@ -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. /// Returns an iterator over all user-declared mutable arguments and locals.
#[inline] #[inline]
pub fn mut_vars_and_args_iter<'a>(&'a self) -> impl Iterator<Item = Local> + 'a { pub fn mut_vars_and_args_iter<'a>(&'a self) -> impl Iterator<Item = Local> + 'a {

View file

@ -281,23 +281,21 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
// Note that this set is expected to be small - only upvars from closures // 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 // would have a chance of erroneously adding non-user-defined mutable vars
// to the set. // to the set.
let temporary_used_locals: FxHashSet<Local> = mbcx let temporary_used_locals: FxHashSet<Local> = mbcx.used_mut.iter()
.used_mut
.iter()
.filter(|&local| mbcx.mir.local_decls[*local].is_user_variable.is_none()) .filter(|&local| mbcx.mir.local_decls[*local].is_user_variable.is_none())
.cloned() .cloned()
.collect(); .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); debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
let 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 { if let ClearCrossCrate::Set(ref vsi) = mbcx.mir.source_scope_local_data {
let local_decl = &mbcx.mir.local_decls[local]; let local_decl = &mbcx.mir.local_decls[local];

View file

@ -9,43 +9,113 @@
// except according to those terms. // except according to those terms.
use rustc::mir::visit::{PlaceContext, Visitor}; 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 rustc_data_structures::fx::FxHashSet;
use borrow_check::MirBorrowckCtxt; use borrow_check::MirBorrowckCtxt;
impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { 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 /// Walks the MIR adding to the set of `used_mut` locals that will be ignored for the purposes
/// local variables lint, to update the context's `used_mut` in a single walk. /// of the `unused_mut` lint.
crate fn gather_used_muts(&mut self, locals: FxHashSet<Local>) { ///
let mut visitor = GatherUsedMutsVisitor { /// `temporary_used_locals` should contain locals that were found to be temporary, mutable and
needles: locals, /// used from borrow checking. This function looks for assignments into these locals from
mbcx: self, /// 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.
visitor.visit_mir(visitor.mbcx.mir); ///
/// `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. /// MIR visitor for collecting used mutable variables.
/// 'visit = the duration of the MIR walk /// The 'visit lifetime represents the duration of the MIR walk.
struct GatherUsedMutsVisitor<'visit, 'cx: 'visit, 'gcx: 'tcx, 'tcx: 'cx> { 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>, mbcx: &'visit mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>,
} }
impl<'visit, 'cx, 'gcx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, '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( fn visit_local(
&mut self, &mut self,
local: &Local, local: &Local,
place_context: PlaceContext<'tcx>, place_context: PlaceContext<'tcx>,
location: Location, location: Location,
) { ) {
if !self.needles.contains(local) { if place_context.is_place_assignment() && self.temporary_used_locals.contains(local) {
return;
}
if place_context.is_place_assignment() {
// Propagate the Local assigned at this Location as a used mutable local variable // Propagate the Local assigned at this Location as a used mutable local variable
for moi in &self.mbcx.move_data.loc_map[location] { for moi in &self.mbcx.move_data.loc_map[location] {
let mpi = &self.mbcx.move_data.moves[*moi].path; let mpi = &self.mbcx.move_data.moves[*moi].path;

View file

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