bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #46236 - davidtwco:issue-46023, r=arielb1

Browse source 
MIR-borrowck: immutable unique closure upvars can be mutated

Fixes #46023 and #46160 (see [this comment](https://github.com/rust-lang/rust/pull/46236#issuecomment-347204874)).
This commit is contained in:
bors 2017-12-01 13:26:10 +00:00
commit e3ed21272d
6 changed files with 89 additions and 50 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/librustc/ich/impls_mir.rs
View file

@ -31,7 +31,7 @@ impl_stable_hash_for!(struct mir::LocalDecl<'tcx> {
lexical_scope,
is_user_variable
});
impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, by_ref });
impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, by_ref, mutability });
impl_stable_hash_for!(struct mir::BasicBlockData<'tcx> { statements, terminator, is_cleanup });
impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, kind });
impl_stable_hash_for!(struct mir::UnsafetyCheckResult { violations, unsafe_blocks });

4
src/librustc/mir/mod.rs
View file

@ -546,7 +546,9 @@ pub struct UpvarDecl {
pub debug_name: Name,
/// If true, the capture is behind a reference.
pub by_ref: bool
pub by_ref: bool,
pub mutability: Mutability,
}
///////////////////////////////////////////////////////////////////////////

97
src/librustc_mir/borrow_check.rs
View file

@ -761,47 +761,34 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
let move_data = self.move_data;
// determine if this path has a non-mut owner (and thus needs checking).
let mut l = lvalue;
loop {
match *l {
Lvalue::Projection(ref proj) => {
l = &proj.base;
continue;
}
Lvalue::Local(local) => {
match self.mir.local_decls[local].mutability {
Mutability::Not => break, // needs check
Mutability::Mut => return,
}
}
Lvalue::Static(ref static_) => {
// mutation of non-mut static is always illegal,
// independent of dataflow. However it will be catched by
// `check_access_permissions()`, we call delay_span_bug here
// to be sure that no case has been missed
if !self.tcx.is_static_mut(static_.def_id) {
let item_msg = match self.describe_lvalue(lvalue) {
Some(name) => format!("immutable static item `{}`", name),
None => "immutable static item".to_owned()
};
self.tcx.sess.delay_span_bug(span,
&format!("cannot assign to {}, should have been caught by \
`check_access_permissions()`", item_msg));
}
return;
}
}
if let Ok(()) = self.is_mutable(lvalue, LocalMutationIsAllowed::No) {
return;
}
if let Some(mpi) = self.move_path_for_lvalue(lvalue) {
for ii in &move_data.init_path_map[mpi] {
if flow_state.ever_inits.curr_state.contains(ii) {
let first_assign_span = self.move_data.inits[*ii].span;
self.report_illegal_reassignment(
context, (lvalue, span), first_assign_span);
break;
if let Err(_) = self.is_mutable(lvalue, LocalMutationIsAllowed::Yes) {
return;
}
match self.move_path_closest_to(lvalue) {
Ok(mpi) => {
for ii in &move_data.init_path_map[mpi] {
if flow_state.ever_inits.curr_state.contains(ii) {
let first_assign_span = self.move_data.inits[*ii].span;
self.report_illegal_reassignment(
context, (lvalue, span), first_assign_span);
break;
}
}
}
},
Err(NoMovePathFound::ReachedStatic) => {
let item_msg = match self.describe_lvalue(lvalue) {
Some(name) => format!("immutable static item `{}`", name),
None => "immutable static item".to_owned()
};
self.tcx.sess.delay_span_bug(span,
&format!("cannot assign to {}, should have been caught by \
`check_access_permissions()`", item_msg));
},
}
}
@ -1108,12 +1095,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
ProjectionElem::Deref => {
let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
// `Box<T>` owns its content, so mutable if its location is mutable
if base_ty.is_box() {
return self.is_mutable(&proj.base, LocalMutationIsAllowed::No);
}
// Otherwise we check the kind of deref to decide
// Check the kind of deref to decide
match base_ty.sty {
ty::TyRef(_, tnm) => {
match tnm.mutbl {
@ -1121,7 +1103,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
hir::MutImmutable => Err(lvalue),
// Mutably borrowed data is mutable, but only if we have a
// unique path to the `&mut`
hir::MutMutable => self.is_unique(&proj.base),
hir::MutMutable => {
if self.is_upvar_field_projection(&proj.base).is_some() {
self.is_mutable(&proj.base, is_local_mutation_allowed)
} else {
self.is_unique(&proj.base)
}
},
}
},
ty::TyRawPtr(tnm) => {
@ -1133,8 +1121,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
hir::MutMutable => Ok(()),
}
},
// `Box<T>` owns its content, so mutable if its location is mutable
_ if base_ty.is_box() =>
self.is_mutable(&proj.base, LocalMutationIsAllowed::No),
// Deref should only be for reference, pointers or boxes
_ => bug!("Deref of unexpected type: {:?}", base_ty)
_ => bug!("Deref of unexpected type: {:?}", base_ty),
}
},
// All other projections are owned by their base path, so mutable if
@ -1143,8 +1134,20 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
ProjectionElem::Index(..) |
ProjectionElem::ConstantIndex{..} |
ProjectionElem::Subslice{..} |
ProjectionElem::Downcast(..) =>
ProjectionElem::Downcast(..) => {
let field_projection = self.is_upvar_field_projection(lvalue);
if let Some(field) = field_projection {
let decl = &self.mir.upvar_decls[field.index()];
return match decl.mutability {
Mutability::Mut => self.is_unique(&proj.base),
Mutability::Not => Err(lvalue),
};
}
self.is_mutable(&proj.base, LocalMutationIsAllowed::No)
}
}
}
}

12
src/librustc_mir/build/mod.rs
View file

@ -443,10 +443,20 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>,
let mut decl = UpvarDecl {
debug_name: keywords::Invalid.name(),
by_ref,
mutability: Mutability::Not,
};
if let Some(hir::map::NodeBinding(pat)) = tcx.hir.find(var_id) {
if let hir::PatKind::Binding(_, _, ref ident, _) = pat.node {
decl.debug_name = ident.node;
let bm = *hir.tables.pat_binding_modes()
.get(pat.hir_id)
.expect("missing binding mode");
if bm == ty::BindByValue(hir::MutMutable) {
decl.mutability = Mutability::Mut;
} else {
decl.mutability = Mutability::Not;
}
}
}
decl
@ -571,7 +581,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
self.local_decls.push(LocalDecl {
mutability: Mutability::Not,
mutability: Mutability::Mut,
ty,
source_info: SourceInfo {
scope: ARGUMENT_VISIBILITY_SCOPE,

22
src/test/compile-fail/issue-46023.rs Normal file
View file

@ -0,0 +1,22 @@
// Copyright 2016 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.
// revisions: ast mir
//[mir]compile-flags: -Z emit-end-regions -Z borrowck=mir
fn main() {
let x = 0;
(move || {
x = 1;
//[mir]~^ ERROR cannot assign to immutable item `x` [E0594]
//[ast]~^^ ERROR cannot assign to captured outer variable in an `FnMut` closure [E0594]
})()
}

2
src/test/run-pass/issue-16671.rs
View file

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
//compile-flags: -Z borrowck=compare -Z emit-end-regions
#![deny(warnings)]
fn foo<F: FnOnce()>(_f: F) { }