From dedbb4edd1c3056208d1892c7c4045240d96b15a Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 30 Nov 2017 23:18:38 +0000 Subject: [PATCH] Immutable unique closure upvars cannot be mutated. --- src/librustc/ich/impls_mir.rs | 2 +- src/librustc/mir/mod.rs | 4 +- src/librustc_mir/borrow_check.rs | 97 ++++++++++++++++---------------- src/librustc_mir/build/mod.rs | 10 ++++ 4 files changed, 64 insertions(+), 49 deletions(-) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index e40d07d936b..1c554dde232 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -31,7 +31,7 @@ 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 }); diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index d5df90a5ea0..3e59de33563 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -554,7 +554,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, } /////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index ec3954f7349..642df2e179f 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/src/librustc_mir/borrow_check.rs @@ -761,47 +761,34 @@ fn check_if_reassignment_to_immutable_state(&mut self, 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 @@ fn is_mutable<'d>(&self, ProjectionElem::Deref => { let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx); - // `Box` 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 @@ fn is_mutable<'d>(&self, 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 @@ fn is_mutable<'d>(&self, hir::MutMutable => Ok(()), } }, + // `Box` 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 @@ fn is_mutable<'d>(&self, 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) + } } } } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 080cf4b47cf..d8011e151d8 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -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 -- GitLab