Auto merge of #45283 - alexcrichton:used-mut-nodes, r=arielb1

rustc: Remove `used_mut_nodes` from `TyCtxt`

This updates the borrowck query to return a result, and this result is then used
to incrementally check for unused mutable nodes given sets of all the used
mutable nodes.

Closes #42384
This commit is contained in:
bors 2017-10-16 03:02:05 +00:00
commit fdec805d03
16 changed files with 220 additions and 124 deletions

2
src/Cargo.lock generated
View File

@ -1504,6 +1504,7 @@ dependencies = [
"graphviz 0.0.0",
"log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc 0.0.0",
"rustc_back 0.0.0",
"rustc_errors 0.0.0",
"rustc_mir 0.0.0",
"syntax 0.0.0",
@ -1611,7 +1612,6 @@ version = "0.0.0"
dependencies = [
"log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc 0.0.0",
"rustc_back 0.0.0",
"rustc_const_eval 0.0.0",
"syntax 0.0.0",
"syntax_pos 0.0.0",

View File

@ -106,6 +106,7 @@ pub mod lint;
pub mod middle {
pub mod allocator;
pub mod borrowck;
pub mod expr_use_visitor;
pub mod const_val;
pub mod cstore;

View File

@ -222,6 +222,12 @@ declare_lint! {
"unnecessary use of an `unsafe` block"
}
declare_lint! {
pub UNUSED_MUT,
Warn,
"detect mut variables which don't need to be mutable"
}
/// Does nothing as a lint pass, but registers some `Lint`s
/// which are used by other parts of the compiler.
#[derive(Copy, Clone)]
@ -263,7 +269,8 @@ impl LintPass for HardwiredLints {
PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
LATE_BOUND_LIFETIME_ARGUMENTS,
DEPRECATED,
UNUSED_UNSAFE
UNUSED_UNSAFE,
UNUSED_MUT
)
}
}

View File

@ -0,0 +1,31 @@
// 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.
use ich::StableHashingContext;
use hir::HirId;
use util::nodemap::FxHashSet;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher,
StableHasherResult};
pub struct BorrowCheckResult {
pub used_mut_nodes: FxHashSet<HirId>,
}
impl<'gcx> HashStable<StableHashingContext<'gcx>> for BorrowCheckResult {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'gcx>,
hasher: &mut StableHasher<W>) {
let BorrowCheckResult {
ref used_mut_nodes,
} = *self;
used_mut_nodes.hash_stable(hcx, hasher);
}
}

View File

@ -898,11 +898,6 @@ pub struct GlobalCtxt<'tcx> {
pub inhabitedness_cache: RefCell<FxHashMap<Ty<'tcx>, DefIdForest>>,
/// Set of nodes which mark locals as mutable which end up getting used at
/// some point. Local variable definitions not in this set can be warned
/// about.
pub used_mut_nodes: RefCell<NodeSet>,
/// Caches the results of trait selection. This cache is used
/// for things that do not have to do with the parameters in scope.
pub selection_cache: traits::SelectionCache<'tcx>,
@ -1185,7 +1180,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
rcache: RefCell::new(FxHashMap()),
normalized_cache: RefCell::new(FxHashMap()),
inhabitedness_cache: RefCell::new(FxHashMap()),
used_mut_nodes: RefCell::new(NodeSet()),
selection_cache: traits::SelectionCache::new(),
evaluation_cache: traits::EvaluationCache::new(),
rvalue_promotable_to_static: RefCell::new(NodeMap()),

View File

@ -15,6 +15,7 @@ use hir::def::{Def, Export};
use hir::{self, TraitCandidate, ItemLocalId};
use hir::svh::Svh;
use lint;
use middle::borrowck::BorrowCheckResult;
use middle::const_val;
use middle::cstore::{ExternCrate, LinkagePreference, NativeLibrary,
ExternBodyNestedBodies};
@ -183,7 +184,7 @@ define_maps! { <'tcx>
[] fn coherent_trait: coherent_trait_dep_node((CrateNum, DefId)) -> (),
[] fn borrowck: BorrowCheck(DefId) -> (),
[] fn borrowck: BorrowCheck(DefId) -> Rc<BorrowCheckResult>,
// FIXME: shouldn't this return a `Result<(), BorrowckErrors>` instead?
[] fn mir_borrowck: MirBorrowCheck(DefId) -> (),

View File

@ -15,5 +15,6 @@ syntax = { path = "../libsyntax" }
syntax_pos = { path = "../libsyntax_pos" }
graphviz = { path = "../libgraphviz" }
rustc = { path = "../librustc" }
rustc_back = { path = "../librustc_back" }
rustc_mir = { path = "../librustc_mir" }
rustc_errors = { path = "../librustc_errors" }

View File

@ -770,7 +770,8 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
let lp = opt_loan_path(&assignee_cmt).unwrap();
self.move_data.each_assignment_of(assignment_id, &lp, |assign| {
if assignee_cmt.mutbl.is_mutable() {
self.tcx().used_mut_nodes.borrow_mut().insert(local_id);
let hir_id = self.bccx.tcx.hir.node_to_hir_id(local_id);
self.bccx.used_mut_nodes.borrow_mut().insert(hir_id);
} else {
self.bccx.report_reassigned_immutable_variable(
assignment_span,

View File

@ -442,13 +442,13 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> {
wrapped_path = match current_path.kind {
LpVar(local_id) => {
if !through_borrow {
self.tcx().used_mut_nodes.borrow_mut().insert(local_id);
let hir_id = self.bccx.tcx.hir.node_to_hir_id(local_id);
self.bccx.used_mut_nodes.borrow_mut().insert(hir_id);
}
None
}
LpUpvar(ty::UpvarId{ var_id, closure_expr_id: _ }) => {
let local_id = self.tcx().hir.hir_to_node_id(var_id);
self.tcx().used_mut_nodes.borrow_mut().insert(local_id);
self.bccx.used_mut_nodes.borrow_mut().insert(var_id);
None
}
LpExtend(ref base, mc::McInherited, LpDeref(pointer_kind)) |

View File

@ -20,6 +20,7 @@ pub use self::MovedValueUseKind::*;
use self::InteriorKind::*;
use rustc::hir::HirId;
use rustc::hir::map as hir_map;
use rustc::hir::map::blocks::FnLikeNode;
use rustc::cfg;
@ -27,6 +28,7 @@ use rustc::middle::dataflow::DataFlowContext;
use rustc::middle::dataflow::BitwiseOperator;
use rustc::middle::dataflow::DataFlowOperator;
use rustc::middle::dataflow::KillFrom;
use rustc::middle::borrowck::BorrowCheckResult;
use rustc::hir::def_id::{DefId, DefIndex};
use rustc::middle::expr_use_visitor as euv;
use rustc::middle::mem_categorization as mc;
@ -37,7 +39,9 @@ use rustc::middle::free_region::RegionRelations;
use rustc::ty::{self, Ty, TyCtxt};
use rustc::ty::maps::Providers;
use rustc_mir::util::borrowck_errors::{BorrowckErrors, Origin};
use rustc::util::nodemap::FxHashSet;
use std::cell::RefCell;
use std::fmt;
use std::rc::Rc;
use std::hash::{Hash, Hasher};
@ -54,6 +58,8 @@ pub mod gather_loans;
pub mod move_data;
mod unused;
#[derive(Clone, Copy)]
pub struct LoanDataFlowOperator;
@ -79,7 +85,9 @@ pub struct AnalysisData<'a, 'tcx: 'a> {
pub move_data: move_data::FlowedMoveData<'a, 'tcx>,
}
fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId) {
fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId)
-> Rc<BorrowCheckResult>
{
debug!("borrowck(body_owner_def_id={:?})", owner_def_id);
let owner_id = tcx.hir.as_local_node_id(owner_def_id).unwrap();
@ -91,7 +99,9 @@ fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId) {
// those things (notably the synthesized constructors from
// tuple structs/variants) do not have an associated body
// and do not need borrowchecking.
return;
return Rc::new(BorrowCheckResult {
used_mut_nodes: FxHashSet(),
})
}
_ => { }
}
@ -100,7 +110,14 @@ fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId) {
let tables = tcx.typeck_tables_of(owner_def_id);
let region_scope_tree = tcx.region_scope_tree(owner_def_id);
let body = tcx.hir.body(body_id);
let bccx = &mut BorrowckCtxt { tcx, tables, region_scope_tree, owner_def_id, body };
let mut bccx = BorrowckCtxt {
tcx,
tables,
region_scope_tree,
owner_def_id,
body,
used_mut_nodes: RefCell::new(FxHashSet()),
};
// Eventually, borrowck will always read the MIR, but at the
// moment we do not. So, for now, we always force MIR to be
@ -118,14 +135,19 @@ fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId) {
if let Some(AnalysisData { all_loans,
loans: loan_dfcx,
move_data: flowed_moves }) =
build_borrowck_dataflow_data(bccx, false, body_id,
build_borrowck_dataflow_data(&mut bccx, false, body_id,
|bccx| {
cfg = Some(cfg::CFG::new(bccx.tcx, &body));
cfg.as_mut().unwrap()
})
{
check_loans::check_loans(bccx, &loan_dfcx, &flowed_moves, &all_loans, body);
check_loans::check_loans(&mut bccx, &loan_dfcx, &flowed_moves, &all_loans, body);
}
unused::check(&mut bccx, body);
Rc::new(BorrowCheckResult {
used_mut_nodes: bccx.used_mut_nodes.into_inner(),
})
}
fn build_borrowck_dataflow_data<'a, 'c, 'tcx, F>(this: &mut BorrowckCtxt<'a, 'tcx>,
@ -198,7 +220,14 @@ pub fn build_borrowck_dataflow_data_for_fn<'a, 'tcx>(
let tables = tcx.typeck_tables_of(owner_def_id);
let region_scope_tree = tcx.region_scope_tree(owner_def_id);
let body = tcx.hir.body(body_id);
let mut bccx = BorrowckCtxt { tcx, tables, region_scope_tree, owner_def_id, body };
let mut bccx = BorrowckCtxt {
tcx,
tables,
region_scope_tree,
owner_def_id,
body,
used_mut_nodes: RefCell::new(FxHashSet()),
};
let dataflow_data = build_borrowck_dataflow_data(&mut bccx, true, body_id, |_| cfg);
(bccx, dataflow_data.unwrap())
@ -219,6 +248,8 @@ pub struct BorrowckCtxt<'a, 'tcx: 'a> {
owner_def_id: DefId,
body: &'tcx hir::Body,
used_mut_nodes: RefCell<FxHashSet<HirId>>,
}
impl<'b, 'tcx: 'b> BorrowckErrors for BorrowckCtxt<'b, 'tcx> {

View File

@ -0,0 +1,118 @@
// 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.
use rustc::hir::intravisit::{Visitor, NestedVisitorMap};
use rustc::hir::{self, HirId};
use rustc::lint::builtin::UNUSED_MUT;
use rustc::ty;
use rustc::util::nodemap::{FxHashMap, FxHashSet};
use rustc_back::slice;
use syntax::ptr::P;
use borrowck::BorrowckCtxt;
pub fn check<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, body: &'tcx hir::Body) {
let mut used_mut = bccx.used_mut_nodes.borrow().clone();
UsedMutFinder {
bccx,
set: &mut used_mut,
}.visit_expr(&body.value);
let mut cx = UnusedMutCx { bccx, used_mut };
for arg in body.arguments.iter() {
cx.check_unused_mut_pat(slice::ref_slice(&arg.pat));
}
cx.visit_expr(&body.value);
}
struct UsedMutFinder<'a, 'tcx: 'a> {
bccx: &'a BorrowckCtxt<'a, 'tcx>,
set: &'a mut FxHashSet<HirId>,
}
struct UnusedMutCx<'a, 'tcx: 'a> {
bccx: &'a BorrowckCtxt<'a, 'tcx>,
used_mut: FxHashSet<HirId>,
}
impl<'a, 'tcx> UnusedMutCx<'a, 'tcx> {
fn check_unused_mut_pat(&self, pats: &[P<hir::Pat>]) {
let tcx = self.bccx.tcx;
let mut mutables = FxHashMap();
for p in pats {
p.each_binding(|_, id, span, path1| {
let name = path1.node;
// Skip anything that looks like `_foo`
if name.as_str().starts_with("_") {
return
}
// Skip anything that looks like `&foo` or `&mut foo`, only look
// for by-value bindings
let hir_id = tcx.hir.node_to_hir_id(id);
let bm = match self.bccx.tables.pat_binding_modes().get(hir_id) {
Some(&bm) => bm,
None => span_bug!(span, "missing binding mode"),
};
match bm {
ty::BindByValue(hir::MutMutable) => {}
_ => return,
}
mutables.entry(name).or_insert(Vec::new()).push((id, hir_id, span));
});
}
for (_name, ids) in mutables {
// If any id for this name was used mutably then consider them all
// ok, so move on to the next
if ids.iter().any(|&(_, ref id, _)| self.used_mut.contains(id)) {
continue
}
let mut_span = tcx.sess.codemap().span_until_char(ids[0].2, ' ');
// Ok, every name wasn't used mutably, so issue a warning that this
// didn't need to be mutable.
tcx.struct_span_lint_node(UNUSED_MUT,
ids[0].0,
ids[0].2,
"variable does not need to be mutable")
.span_suggestion_short(mut_span, "remove this `mut`", "".to_owned())
.emit();
}
}
}
impl<'a, 'tcx> Visitor<'tcx> for UnusedMutCx<'a, 'tcx> {
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::OnlyBodies(&self.bccx.tcx.hir)
}
fn visit_arm(&mut self, arm: &hir::Arm) {
self.check_unused_mut_pat(&arm.pats)
}
fn visit_local(&mut self, local: &hir::Local) {
self.check_unused_mut_pat(slice::ref_slice(&local.pat));
}
}
impl<'a, 'tcx> Visitor<'tcx> for UsedMutFinder<'a, 'tcx> {
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::OnlyBodies(&self.bccx.tcx.hir)
}
fn visit_nested_body(&mut self, id: hir::BodyId) {
let def_id = self.bccx.tcx.hir.body_owner_def_id(id);
self.set.extend(self.bccx.tcx.borrowck(def_id).used_mut_nodes.iter().cloned());
self.visit_body(self.bccx.tcx.hir.body(id));
}
}

View File

@ -21,6 +21,7 @@
extern crate syntax;
extern crate syntax_pos;
extern crate rustc_errors as errors;
extern crate rustc_back;
// for "clarity", rename the graphviz crate to dot; graphviz within `borrowck`
// refers to the borrowck-specific graphviz adapter traits.

View File

@ -12,7 +12,6 @@ test = false
[dependencies]
log = "0.3"
rustc = { path = "../librustc" }
rustc_back = { path = "../librustc_back" }
rustc_const_eval = { path = "../librustc_const_eval" }
syntax = { path = "../libsyntax" }
syntax_pos = { path = "../libsyntax_pos" }

View File

@ -38,7 +38,6 @@ extern crate syntax;
extern crate rustc;
#[macro_use]
extern crate log;
extern crate rustc_back;
extern crate rustc_const_eval;
extern crate syntax_pos;
@ -129,7 +128,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
NonUpperCaseGlobals,
NonShorthandFieldPatterns,
UnsafeCode,
UnusedMut,
UnusedAllocation,
MissingCopyImplementations,
UnstableFeatures,

View File

@ -11,105 +11,18 @@
use rustc::hir::def_id::DefId;
use rustc::ty;
use rustc::ty::adjustment;
use util::nodemap::FxHashMap;
use lint::{LateContext, EarlyContext, LintContext, LintArray};
use lint::{LintPass, EarlyLintPass, LateLintPass};
use std::collections::hash_map::Entry::{Occupied, Vacant};
use syntax::ast;
use syntax::attr;
use syntax::feature_gate::{BUILTIN_ATTRIBUTES, AttributeType};
use syntax::symbol::keywords;
use syntax::ptr::P;
use syntax::print::pprust;
use syntax::symbol::keywords;
use syntax::util::parser;
use syntax_pos::Span;
use rustc_back::slice;
use rustc::hir;
use rustc::hir::intravisit::FnKind;
declare_lint! {
pub UNUSED_MUT,
Warn,
"detect mut variables which don't need to be mutable"
}
#[derive(Copy, Clone)]
pub struct UnusedMut;
impl UnusedMut {
fn check_unused_mut_pat(&self, cx: &LateContext, pats: &[P<hir::Pat>]) {
// collect all mutable pattern and group their NodeIDs by their Identifier to
// avoid false warnings in match arms with multiple patterns
let mut mutables = FxHashMap();
for p in pats {
p.each_binding(|_, id, span, path1| {
let hir_id = cx.tcx.hir.node_to_hir_id(id);
let bm = match cx.tables.pat_binding_modes().get(hir_id) {
Some(&bm) => bm,
None => span_bug!(span, "missing binding mode"),
};
let name = path1.node;
if let ty::BindByValue(hir::MutMutable) = bm {
if !name.as_str().starts_with("_") {
match mutables.entry(name) {
Vacant(entry) => {
entry.insert(vec![id]);
}
Occupied(mut entry) => {
entry.get_mut().push(id);
}
}
}
}
});
}
let used_mutables = cx.tcx.used_mut_nodes.borrow();
for (_, v) in &mutables {
if !v.iter().any(|e| used_mutables.contains(e)) {
let binding_span = cx.tcx.hir.span(v[0]);
let mut_span = cx.tcx.sess.codemap().span_until_char(binding_span, ' ');
let mut err = cx.struct_span_lint(UNUSED_MUT,
binding_span,
"variable does not need to be mutable");
err.span_suggestion_short(mut_span, "remove this `mut`", "".to_owned());
err.emit();
}
}
}
}
impl LintPass for UnusedMut {
fn get_lints(&self) -> LintArray {
lint_array!(UNUSED_MUT)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedMut {
fn check_arm(&mut self, cx: &LateContext, a: &hir::Arm) {
self.check_unused_mut_pat(cx, &a.pats)
}
fn check_local(&mut self, cx: &LateContext, l: &hir::Local) {
self.check_unused_mut_pat(cx, slice::ref_slice(&l.pat));
}
fn check_fn(&mut self,
cx: &LateContext,
_: FnKind,
_: &hir::FnDecl,
body: &hir::Body,
_: Span,
_: ast::NodeId) {
for a in &body.arguments {
self.check_unused_mut_pat(cx, slice::ref_slice(&a.pat));
}
}
}
declare_lint! {
pub UNUSED_MUST_USE,

View File

@ -14,6 +14,20 @@ warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was
|
= note: #[warn(deprecated)] on by default
warning: variable does not need to be mutable
--> $DIR/suggestions.rs:17:13
|
17 | let mut a = (1); // should suggest no `mut`, no parens
| ---^^
| |
| help: remove this `mut`
|
note: lint level defined here
--> $DIR/suggestions.rs:11:9
|
11 | #![warn(unused_mut)] // UI tests pass `-A unused`—see Issue #43896
| ^^^^^^^^^^
warning: denote infinite loops with `loop { ... }`
--> $DIR/suggestions.rs:16:5
|
@ -29,17 +43,3 @@ warning: denote infinite loops with `loop { ... }`
|
= note: #[warn(while_true)] on by default
warning: variable does not need to be mutable
--> $DIR/suggestions.rs:17:13
|
17 | let mut a = (1); // should suggest no `mut`, no parens
| ---^^
| |
| help: remove this `mut`
|
note: lint level defined here
--> $DIR/suggestions.rs:11:9
|
11 | #![warn(unused_mut)] // UI tests pass `-A unused`—see Issue #43896
| ^^^^^^^^^^