Skip to content

Commit e70ff68

Browse files
committed
Auto merge of #51660 - lqd:the-MIRnistry-of-walks, r=nikomatsakis
NLL: Walk the MIR only once for the "unused mut" lint Turns the quadratic loop gathering local variable assignments into a single MIR walk, and brings down the number of `super_mir` calls generated from `do_mir_borrowck` to the expected levels seen in `nll::replace_regions_in_mir` and `nll::compute_regions`, i.e. on clap: 1883 `super_mir` calls instead of 8011. The limited perf numbers I could gather on my machines look to be what we expected: `clap-check` seems to be gaining back a lot of the 7% we previously saw in `visit_mir`. Fixes #51641. r? @nikomatsakis
2 parents 7dae5c0 + 63a4e72 commit e70ff68

File tree

3 files changed

+84
-33
lines changed

3 files changed

+84
-33
lines changed

src/librustc_mir/borrow_check/mod.rs

+2-14
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ mod location;
6161
crate mod place_ext;
6262
mod prefixes;
6363
mod path_utils;
64+
mod used_muts;
6465

6566
pub(crate) mod nll;
6667

@@ -281,20 +282,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
281282
.filter(|&local| !mbcx.mir.local_decls[*local].is_user_variable.is_some())
282283
.cloned()
283284
.collect();
284-
285-
for local in temporary_used_locals {
286-
for location in mbcx.mir.find_assignments(local) {
287-
for moi in &mbcx.move_data.loc_map[location] {
288-
let mpi = &mbcx.move_data.moves[*moi].path;
289-
let path = &mbcx.move_data.move_paths[*mpi];
290-
debug!("assignment of {:?} to {:?}, adding {:?} to used mutable set",
291-
path.place, local, path.place);
292-
if let Place::Local(user_local) = path.place {
293-
mbcx.used_mut.insert(user_local);
294-
}
295-
}
296-
}
297-
}
285+
mbcx.gather_used_muts(temporary_used_locals);
298286

299287
debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
300288

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use rustc::mir::visit::{PlaceContext, Visitor};
12+
use rustc::mir::{Local, Location, Place};
13+
14+
use rustc_data_structures::fx::FxHashSet;
15+
16+
use borrow_check::MirBorrowckCtxt;
17+
use util::collect_writes::is_place_assignment;
18+
19+
impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
20+
/// Walks the MIR looking for assignments to a set of locals, as part of the unused mutable
21+
/// local variables lint, to update the context's `used_mut` in a single walk.
22+
crate fn gather_used_muts(&mut self, locals: FxHashSet<Local>) {
23+
let mut visitor = GatherUsedMutsVisitor {
24+
needles: locals,
25+
mbcx: self,
26+
};
27+
visitor.visit_mir(visitor.mbcx.mir);
28+
}
29+
}
30+
31+
/// MIR visitor gathering the assignments to a set of locals, in a single walk.
32+
/// 'visit = the duration of the MIR walk
33+
struct GatherUsedMutsVisitor<'visit, 'cx: 'visit, 'gcx: 'tcx, 'tcx: 'cx> {
34+
needles: FxHashSet<Local>,
35+
mbcx: &'visit mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>,
36+
}
37+
38+
impl<'visit, 'cx, 'gcx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'cx, 'gcx, 'tcx> {
39+
fn visit_local(
40+
&mut self,
41+
local: &Local,
42+
place_context: PlaceContext<'tcx>,
43+
location: Location,
44+
) {
45+
if !self.needles.contains(local) {
46+
return;
47+
}
48+
49+
if is_place_assignment(&place_context) {
50+
// Propagate the Local assigned at this Location as a used mutable local variable
51+
for moi in &self.mbcx.move_data.loc_map[location] {
52+
let mpi = &self.mbcx.move_data.moves[*moi].path;
53+
let path = &self.mbcx.move_data.move_paths[*mpi];
54+
debug!(
55+
"assignment of {:?} to {:?}, adding {:?} to used mutable set",
56+
path.place, local, path.place
57+
);
58+
if let Place::Local(user_local) = path.place {
59+
self.mbcx.used_mut.insert(user_local);
60+
}
61+
}
62+
}
63+
}
64+
}

src/librustc_mir/util/collect_writes.rs

+18-19
Original file line numberDiff line numberDiff line change
@@ -43,25 +43,24 @@ impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor {
4343
return;
4444
}
4545

46-
match place_context {
47-
PlaceContext::Store | PlaceContext::Call => {
48-
self.locations.push(location);
49-
}
50-
PlaceContext::AsmOutput |
51-
PlaceContext::Drop |
52-
PlaceContext::Inspect |
53-
PlaceContext::Borrow { .. } |
54-
PlaceContext::Projection(..) |
55-
PlaceContext::Copy |
56-
PlaceContext::Move |
57-
PlaceContext::StorageLive |
58-
PlaceContext::StorageDead |
59-
PlaceContext::Validate => {
60-
// TO-DO
61-
// self.super_local(local)
62-
}
46+
if is_place_assignment(&place_context) {
47+
self.locations.push(location);
6348
}
6449
}
65-
// TO-DO
66-
// fn super_local()
50+
}
51+
52+
/// Returns true if this place context represents an assignment statement
53+
crate fn is_place_assignment(place_context: &PlaceContext) -> bool {
54+
match *place_context {
55+
PlaceContext::Store | PlaceContext::Call | PlaceContext::AsmOutput => true,
56+
PlaceContext::Drop
57+
| PlaceContext::Inspect
58+
| PlaceContext::Borrow { .. }
59+
| PlaceContext::Projection(..)
60+
| PlaceContext::Copy
61+
| PlaceContext::Move
62+
| PlaceContext::StorageLive
63+
| PlaceContext::StorageDead
64+
| PlaceContext::Validate => false,
65+
}
6766
}

0 commit comments

Comments
 (0)