Skip to content

Commit 0a1cb9b

Browse files
KiChjangKiChjang
committed
Add the actual used mutable var to the set
1 parent ded0697 commit 0a1cb9b

File tree

1 file changed

+102
-65
lines changed
  • src/librustc_mir/borrow_check

1 file changed

+102
-65
lines changed

Diff for: ‎src/librustc_mir/borrow_check/mod.rs

+102-65
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,31 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
259259

260260
mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer
261261

262+
// For each non-user used mutable variable, check if it's been assigned from
263+
// a user-declared local. If so, then put that local into the used_mut set.
264+
// Note that this set is expected to be small - only upvars from closures
265+
// would have a chance of erroneously adding non-user-defined mutable vars
266+
// to the set.
267+
let temporary_used_locals: FxHashSet<Local> =
268+
mbcx.used_mut.iter()
269+
.filter(|&local| !mbcx.mir.local_decls[*local].is_user_variable)
270+
.cloned()
271+
.collect();
272+
273+
for local in temporary_used_locals {
274+
for location in mbcx.mir.find_assignments(local) {
275+
for moi in &mbcx.move_data.loc_map[location] {
276+
let mpi = &mbcx.move_data.moves[*moi].path;
277+
let path = &mbcx.move_data.move_paths[*mpi];
278+
debug!("assignment of {:?} to {:?}, adding {:?} to used mutable set",
279+
path.place, local, path.place);
280+
if let Place::Local(user_local) = path.place {
281+
mbcx.used_mut.insert(user_local);
282+
}
283+
}
284+
}
285+
}
286+
262287
debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
263288

264289
for local in mbcx.mir.mut_vars_and_args_iter().filter(|local| !mbcx.used_mut.contains(local)) {
@@ -731,6 +756,11 @@ enum InitializationRequiringAction {
731756
Assignment,
732757
}
733758

759+
struct RootPlace<'d, 'tcx: 'd> {
760+
place: &'d Place<'tcx>,
761+
is_local_mutation_allowed: LocalMutationIsAllowed,
762+
}
763+
734764
impl InitializationRequiringAction {
735765
fn as_noun(self) -> &'static str {
736766
match self {
@@ -1687,23 +1717,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
16871717
Reservation(WriteKind::MutableBorrow(BorrowKind::Mut { .. }))
16881718
| Write(WriteKind::MutableBorrow(BorrowKind::Mut { .. })) => {
16891719
match self.is_mutable(place, is_local_mutation_allowed) {
1690-
Ok((Place::Local(local), mut_allowed)) => {
1691-
if mut_allowed != LocalMutationIsAllowed::Yes {
1692-
// If the local may be initialized, and it is now currently being
1693-
// mutated, then it is justified to be annotated with the `mut`
1694-
// keyword, since the mutation may be a possible reassignment.
1695-
let mpi = self.move_data.rev_lookup.find_local(*local);
1696-
if flow_state.inits.contains(&mpi) {
1697-
self.used_mut.insert(*local);
1698-
}
1699-
}
1700-
}
1701-
Ok((Place::Projection(_), _mut_allowed)) => {
1702-
if let Some(field) = self.is_upvar_field_projection(&place) {
1703-
self.used_mut_upvars.push(field);
1704-
}
1705-
}
1706-
Ok((Place::Static(..), _mut_allowed)) => {}
1720+
Ok(root_place) => self.add_used_mut(root_place, flow_state),
17071721
Err(place_err) => {
17081722
error_reported = true;
17091723
let item_msg = self.get_default_err_msg(place);
@@ -1724,55 +1738,35 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
17241738
}
17251739
Reservation(WriteKind::Mutate) | Write(WriteKind::Mutate) => {
17261740
match self.is_mutable(place, is_local_mutation_allowed) {
1727-
Ok((Place::Local(local), mut_allowed)) => {
1728-
if mut_allowed != LocalMutationIsAllowed::Yes {
1729-
// If the local may be initialized, and it is now currently being
1730-
// mutated, then it is justified to be annotated with the `mut`
1731-
// keyword, since the mutation may be a possible reassignment.
1732-
let mpi = self.move_data.rev_lookup.find_local(*local);
1733-
if flow_state.inits.contains(&mpi) {
1734-
self.used_mut.insert(*local);
1735-
}
1736-
}
1737-
}
1738-
Ok((Place::Projection(_), _mut_allowed)) => {
1739-
if let Some(field) = self.is_upvar_field_projection(&place) {
1740-
self.used_mut_upvars.push(field);
1741-
}
1742-
}
1743-
Ok((Place::Static(..), _mut_allowed)) => {}
1741+
Ok(root_place) => self.add_used_mut(root_place, flow_state),
17441742
Err(place_err) => {
17451743
error_reported = true;
17461744

17471745
let err_info = if let Place::Projection(
17481746
box Projection {
1749-
ref base,
1747+
base: Place::Local(local),
17501748
elem: ProjectionElem::Deref
17511749
}
17521750
) = *place_err {
1753-
if let Place::Local(local) = *base {
1754-
let locations = self.mir.find_assignments(local);
1755-
if locations.len() > 0 {
1756-
let item_msg = if error_reported {
1757-
self.get_secondary_err_msg(base)
1758-
} else {
1759-
self.get_default_err_msg(place)
1760-
};
1761-
let sp = self.mir.source_info(locations[0]).span;
1762-
let mut to_suggest_span = String::new();
1763-
if let Ok(src) =
1764-
self.tcx.sess.codemap().span_to_snippet(sp) {
1765-
to_suggest_span = src[1..].to_string();
1766-
};
1767-
Some((sp,
1768-
"consider changing this to be a \
1769-
mutable reference",
1770-
to_suggest_span,
1771-
item_msg,
1772-
self.get_primary_err_msg(base)))
1751+
let locations = self.mir.find_assignments(local);
1752+
if locations.len() > 0 {
1753+
let item_msg = if error_reported {
1754+
self.get_secondary_err_msg(&Place::Local(local))
17731755
} else {
1774-
None
1775-
}
1756+
self.get_default_err_msg(place)
1757+
};
1758+
let sp = self.mir.source_info(locations[0]).span;
1759+
let mut to_suggest_span = String::new();
1760+
if let Ok(src) =
1761+
self.tcx.sess.codemap().span_to_snippet(sp) {
1762+
to_suggest_span = src[1..].to_string();
1763+
};
1764+
Some((sp,
1765+
"consider changing this to be a \
1766+
mutable reference",
1767+
to_suggest_span,
1768+
item_msg,
1769+
self.get_primary_err_msg(&Place::Local(local))))
17761770
} else {
17771771
None
17781772
}
@@ -1834,33 +1828,76 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
18341828
error_reported
18351829
}
18361830

1837-
/// Can this value be written or borrowed mutably
1831+
/// Adds the place into the used mutable variables set
1832+
fn add_used_mut<'d>(
1833+
&mut self,
1834+
root_place: RootPlace<'d, 'tcx>,
1835+
flow_state: &Flows<'cx, 'gcx, 'tcx>
1836+
) {
1837+
match root_place {
1838+
RootPlace {
1839+
place: Place::Local(local),
1840+
is_local_mutation_allowed,
1841+
} => {
1842+
if is_local_mutation_allowed != LocalMutationIsAllowed::Yes {
1843+
// If the local may be initialized, and it is now currently being
1844+
// mutated, then it is justified to be annotated with the `mut`
1845+
// keyword, since the mutation may be a possible reassignment.
1846+
let mpi = self.move_data.rev_lookup.find_local(*local);
1847+
if flow_state.inits.contains(&mpi) {
1848+
self.used_mut.insert(*local);
1849+
}
1850+
}
1851+
}
1852+
RootPlace {
1853+
place: place @ Place::Projection(_),
1854+
is_local_mutation_allowed: _,
1855+
} => {
1856+
if let Some(field) = self.is_upvar_field_projection(&place) {
1857+
self.used_mut_upvars.push(field);
1858+
}
1859+
}
1860+
RootPlace {
1861+
place: Place::Static(..),
1862+
is_local_mutation_allowed: _,
1863+
} => {}
1864+
}
1865+
}
1866+
1867+
/// Whether this value be written or borrowed mutably.
1868+
/// Returns the root place if the place passed in is a projection.
18381869
fn is_mutable<'d>(
18391870
&self,
18401871
place: &'d Place<'tcx>,
18411872
is_local_mutation_allowed: LocalMutationIsAllowed,
1842-
) -> Result<(&'d Place<'tcx>, LocalMutationIsAllowed), &'d Place<'tcx>> {
1873+
) -> Result<RootPlace<'d, 'tcx>, &'d Place<'tcx>> {
18431874
match *place {
18441875
Place::Local(local) => {
18451876
let local = &self.mir.local_decls[local];
18461877
match local.mutability {
18471878
Mutability::Not => match is_local_mutation_allowed {
18481879
LocalMutationIsAllowed::Yes => {
1849-
Ok((place, LocalMutationIsAllowed::Yes))
1880+
Ok(RootPlace {
1881+
place,
1882+
is_local_mutation_allowed: LocalMutationIsAllowed::Yes
1883+
})
18501884
}
18511885
LocalMutationIsAllowed::ExceptUpvars => {
1852-
Ok((place, LocalMutationIsAllowed::ExceptUpvars))
1886+
Ok(RootPlace {
1887+
place,
1888+
is_local_mutation_allowed: LocalMutationIsAllowed::ExceptUpvars
1889+
})
18531890
}
18541891
LocalMutationIsAllowed::No => Err(place),
18551892
},
1856-
Mutability::Mut => Ok((place, is_local_mutation_allowed)),
1893+
Mutability::Mut => Ok(RootPlace { place, is_local_mutation_allowed }),
18571894
}
18581895
}
18591896
Place::Static(ref static_) =>
18601897
if self.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) {
18611898
Err(place)
18621899
} else {
1863-
Ok((place, is_local_mutation_allowed))
1900+
Ok(RootPlace { place, is_local_mutation_allowed })
18641901
},
18651902
Place::Projection(ref proj) => {
18661903
match proj.elem {
@@ -1899,7 +1936,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
18991936
// `*mut` raw pointers are always mutable, regardless of
19001937
// context. The users have to check by themselves.
19011938
hir::MutMutable => {
1902-
return Ok((place, is_local_mutation_allowed));
1939+
return Ok(RootPlace { place, is_local_mutation_allowed });
19031940
}
19041941
}
19051942
}
@@ -1958,7 +1995,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
19581995
// }
19591996
// ```
19601997
let _ = self.is_mutable(&proj.base, is_local_mutation_allowed)?;
1961-
Ok((place, is_local_mutation_allowed))
1998+
Ok(RootPlace { place, is_local_mutation_allowed })
19621999
}
19632000
}
19642001
} else {

0 commit comments

Comments
 (0)