Skip to content

Commit

Permalink
rustc: Remove used_mut_nodes from TyCtxt
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton committed Oct 14, 2017
1 parent af7de7b commit 4df1278
Show file tree
Hide file tree
Showing 16 changed files with 220 additions and 124 deletions.
2 changes: 1 addition & 1 deletion src/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions src/librustc/lib.rs
Expand Up @@ -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;
Expand Down
9 changes: 8 additions & 1 deletion src/librustc/lint/builtin.rs
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
)
}
}
Expand Down
31 changes: 31 additions & 0 deletions src/librustc/middle/borrowck.rs
@@ -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);
}
}
6 changes: 0 additions & 6 deletions src/librustc/ty/context.rs
Expand Up @@ -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>,
Expand Down Expand Up @@ -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()),
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/ty/maps/mod.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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) -> (),

Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/Cargo.toml
Expand Up @@ -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" }
3 changes: 2 additions & 1 deletion src/librustc_borrowck/borrowck/check_loans.rs
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_borrowck/borrowck/gather_loans/mod.rs
Expand Up @@ -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)) |
Expand Down
43 changes: 37 additions & 6 deletions src/librustc_borrowck/borrowck/mod.rs
Expand Up @@ -20,13 +20,15 @@ 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;
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;
Expand All @@ -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};
Expand All @@ -54,6 +58,8 @@ pub mod gather_loans;

pub mod move_data;

mod unused;

#[derive(Clone, Copy)]
pub struct LoanDataFlowOperator;

Expand All @@ -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();
Expand All @@ -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(),
})
}
_ => { }
}
Expand All @@ -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
Expand All @@ -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>,
Expand Down Expand Up @@ -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())
Expand All @@ -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> {
Expand Down
118 changes: 118 additions & 0 deletions src/librustc_borrowck/borrowck/unused.rs
@@ -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));
}
}
1 change: 1 addition & 0 deletions src/librustc_borrowck/lib.rs
Expand Up @@ -22,6 +22,7 @@
#[macro_use] 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.
Expand Down

0 comments on commit 4df1278

Please sign in to comment.