Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 9 additions & 11 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down
104 changes: 87 additions & 17 deletions src/librustc_mir/borrow_check/used_muts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
26 changes: 26 additions & 0 deletions src/test/ui/nll/issue-55344.rs
Original file line number Diff line number Diff line change
@@ -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() {}