diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 97314b57ef65..1168021a93d8 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -885,6 +885,11 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> { } } + // When this returns true, it means that the expression *is* a + // method-call (i.e. via the operator-overload). This true result + // also implies that walk_overloaded_operator already took care of + // recursively processing the input arguments, and thus the caller + // should not do so. fn walk_overloaded_operator(&mut self, expr: &ast::Expr, receiver: &ast::Expr, diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index bdcfc67f92b9..b213d71a78d1 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -71,6 +71,8 @@ pub use self::Note::*; pub use self::deref_kind::*; pub use self::categorization::*; +use self::Aliasability::*; + use middle::check_const; use middle::def; use middle::region; @@ -295,23 +297,29 @@ pub trait Typer<'tcx> : ty::ClosureTyper<'tcx> { impl MutabilityCategory { pub fn from_mutbl(m: ast::Mutability) -> MutabilityCategory { - match m { + let ret = match m { MutImmutable => McImmutable, MutMutable => McDeclared - } + }; + debug!("MutabilityCategory::{}({:?}) => {:?}", + "from_mutbl", m, ret); + ret } pub fn from_borrow_kind(borrow_kind: ty::BorrowKind) -> MutabilityCategory { - match borrow_kind { + let ret = match borrow_kind { ty::ImmBorrow => McImmutable, ty::UniqueImmBorrow => McImmutable, ty::MutBorrow => McDeclared, - } + }; + debug!("MutabilityCategory::{}({:?}) => {:?}", + "from_borrow_kind", borrow_kind, ret); + ret } - pub fn from_pointer_kind(base_mutbl: MutabilityCategory, - ptr: PointerKind) -> MutabilityCategory { - match ptr { + fn from_pointer_kind(base_mutbl: MutabilityCategory, + ptr: PointerKind) -> MutabilityCategory { + let ret = match ptr { Unique => { base_mutbl.inherit() } @@ -321,11 +329,14 @@ impl MutabilityCategory { UnsafePtr(m) => { MutabilityCategory::from_mutbl(m) } - } + }; + debug!("MutabilityCategory::{}({:?}, {:?}) => {:?}", + "from_pointer_kind", base_mutbl, ptr, ret); + ret } fn from_local(tcx: &ty::ctxt, id: ast::NodeId) -> MutabilityCategory { - match tcx.map.get(id) { + let ret = match tcx.map.get(id) { ast_map::NodeLocal(p) | ast_map::NodeArg(p) => match p.node { ast::PatIdent(bind_mode, _, _) => { if bind_mode == ast::BindByValue(ast::MutMutable) { @@ -337,30 +348,39 @@ impl MutabilityCategory { _ => tcx.sess.span_bug(p.span, "expected identifier pattern") }, _ => tcx.sess.span_bug(tcx.map.span(id), "expected identifier pattern") - } + }; + debug!("MutabilityCategory::{}(tcx, id={:?}) => {:?}", + "from_local", id, ret); + ret } pub fn inherit(&self) -> MutabilityCategory { - match *self { + let ret = match *self { McImmutable => McImmutable, McDeclared => McInherited, McInherited => McInherited, - } + }; + debug!("{:?}.inherit() => {:?}", self, ret); + ret } pub fn is_mutable(&self) -> bool { - match *self { + let ret = match *self { McImmutable => false, McInherited => true, McDeclared => true, - } + }; + debug!("{:?}.is_mutable() => {:?}", self, ret); + ret } pub fn is_immutable(&self) -> bool { - match *self { + let ret = match *self { McImmutable => true, McDeclared | McInherited => false - } + }; + debug!("{:?}.is_immutable() => {:?}", self, ret); + ret } pub fn to_user_str(&self) -> &'static str { @@ -733,7 +753,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { } }; - Ok(Rc::new(cmt_result)) + let ret = Rc::new(cmt_result); + debug!("cat_upvar ret={}", ret.repr(self.tcx())); + Ok(ret) } fn env_deref(&self, @@ -794,14 +816,18 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { McDeclared | McInherited => { } } - cmt_ { + let ret = cmt_ { id: id, span: span, cat: cat_deref(Rc::new(cmt_result), 0, env_ptr), mutbl: deref_mutbl, ty: var_ty, note: NoteClosureEnv(upvar_id) - } + }; + + debug!("env_deref ret {}", ret.repr(self.tcx())); + + ret } pub fn cat_rvalue_node(&self, @@ -831,7 +857,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { } } }; - self.cat_rvalue(id, span, re, expr_ty) + let ret = self.cat_rvalue(id, span, re, expr_ty); + debug!("cat_rvalue_node ret {}", ret.repr(self.tcx())); + ret } pub fn cat_rvalue(&self, @@ -839,14 +867,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span: Span, temp_scope: ty::Region, expr_ty: Ty<'tcx>) -> cmt<'tcx> { - Rc::new(cmt_ { + let ret = Rc::new(cmt_ { id:cmt_id, span:span, cat:cat_rvalue(temp_scope), mutbl:McDeclared, ty:expr_ty, note: NoteNone - }) + }); + debug!("cat_rvalue ret {}", ret.repr(self.tcx())); + ret } pub fn cat_field(&self, @@ -855,14 +885,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { f_name: ast::Name, f_ty: Ty<'tcx>) -> cmt<'tcx> { - Rc::new(cmt_ { + let ret = Rc::new(cmt_ { id: node.id(), span: node.span(), mutbl: base_cmt.mutbl.inherit(), cat: cat_interior(base_cmt, InteriorField(NamedField(f_name))), ty: f_ty, note: NoteNone - }) + }); + debug!("cat_field ret {}", ret.repr(self.tcx())); + ret } pub fn cat_tup_field(&self, @@ -871,14 +903,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { f_idx: uint, f_ty: Ty<'tcx>) -> cmt<'tcx> { - Rc::new(cmt_ { + let ret = Rc::new(cmt_ { id: node.id(), span: node.span(), mutbl: base_cmt.mutbl.inherit(), cat: cat_interior(base_cmt, InteriorField(PositionalField(f_idx))), ty: f_ty, note: NoteNone - }) + }); + debug!("cat_tup_field ret {}", ret.repr(self.tcx())); + ret } fn cat_deref(&self, @@ -913,10 +947,14 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { }; let base_cmt_ty = base_cmt.ty; match ty::deref(base_cmt_ty, true) { - Some(mt) => self.cat_deref_common(node, base_cmt, deref_cnt, + Some(mt) => { + let ret = self.cat_deref_common(node, base_cmt, deref_cnt, mt.ty, deref_context, - /* implicit: */ false), + /* implicit: */ false); + debug!("cat_deref ret {}", ret.repr(self.tcx())); + ret + } None => { debug!("Explicit deref of non-derefable type: {}", base_cmt_ty.repr(self.tcx())); @@ -954,14 +992,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { (base_cmt.mutbl.inherit(), cat_interior(base_cmt, interior)) } }; - Ok(Rc::new(cmt_ { + let ret = Rc::new(cmt_ { id: node.id(), span: node.span(), cat: cat, mutbl: m, ty: deref_ty, note: NoteNone - })) + }); + debug!("cat_deref_common ret {}", ret.repr(self.tcx())); + Ok(ret) } pub fn cat_index(&self, @@ -1009,8 +1049,10 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { }; let m = base_cmt.mutbl.inherit(); - return Ok(interior(elt, base_cmt.clone(), base_cmt.ty, - m, context, element_ty)); + let ret = interior(elt, base_cmt.clone(), base_cmt.ty, + m, context, element_ty); + debug!("cat_index ret {}", ret.repr(self.tcx())); + return Ok(ret); fn interior<'tcx, N: ast_node>(elt: &N, of_cmt: cmt<'tcx>, @@ -1039,14 +1081,14 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { context: InteriorOffsetKind) -> McResult> { - match try!(deref_kind(base_cmt.ty, Some(context))) { + let ret = match try!(deref_kind(base_cmt.ty, Some(context))) { deref_ptr(ptr) => { // for unique ptrs, we inherit mutability from the // owning reference. let m = MutabilityCategory::from_pointer_kind(base_cmt.mutbl, ptr); // the deref is explicit in the resulting cmt - Ok(Rc::new(cmt_ { + Rc::new(cmt_ { id:elt.id(), span:elt.span(), cat:cat_deref(base_cmt.clone(), 0, ptr), @@ -1056,13 +1098,15 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { None => self.tcx().sess.bug("Found non-derefable type") }, note: NoteNone - })) + }) } deref_interior(_) => { - Ok(base_cmt) + base_cmt } - } + }; + debug!("deref_vec ret {}", ret.repr(self.tcx())); + Ok(ret) } /// Given a pattern P like: `[_, ..Q, _]`, where `vec_cmt` is the cmt for `P`, `slice_pat` is @@ -1112,14 +1156,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { interior_ty: Ty<'tcx>, interior: InteriorKind) -> cmt<'tcx> { - Rc::new(cmt_ { + let ret = Rc::new(cmt_ { id: node.id(), span: node.span(), mutbl: base_cmt.mutbl.inherit(), cat: cat_interior(base_cmt, interior), ty: interior_ty, note: NoteNone - }) + }); + debug!("cat_imm_interior ret={}", ret.repr(self.tcx())); + ret } pub fn cat_downcast(&self, @@ -1128,14 +1174,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { downcast_ty: Ty<'tcx>, variant_did: ast::DefId) -> cmt<'tcx> { - Rc::new(cmt_ { + let ret = Rc::new(cmt_ { id: node.id(), span: node.span(), mutbl: base_cmt.mutbl.inherit(), cat: cat_downcast(base_cmt, variant_did), ty: downcast_ty, note: NoteNone - }) + }); + debug!("cat_downcast ret={}", ret.repr(self.tcx())); + ret } pub fn cat_pattern(&self, cmt: cmt<'tcx>, pat: &ast::Pat, mut op: F) -> McResult<()> @@ -1341,17 +1389,25 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { } } -#[derive(Copy)] +#[derive(Copy, Clone, Debug)] pub enum InteriorSafety { InteriorUnsafe, InteriorSafe } -#[derive(Copy)] +#[derive(Clone, Debug)] +pub enum Aliasability { + FreelyAliasable(AliasableReason), + NonAliasable, + ImmutableUnique(Box), +} + +#[derive(Copy, Clone, Debug)] pub enum AliasableReason { AliasableBorrowed, AliasableClosure(ast::NodeId), // Aliasable due to capture Fn closure env AliasableOther, + UnaliasableImmutable, // Created as needed upon seeing ImmutableUnique AliasableStatic(InteriorSafety), AliasableStaticMut(InteriorSafety), } @@ -1380,9 +1436,9 @@ impl<'tcx> cmt_<'tcx> { } } - /// Returns `Some(_)` if this lvalue represents a freely aliasable pointer type. + /// Returns `FreelyAliasable(_)` if this lvalue represents a freely aliasable pointer type. pub fn freely_aliasable(&self, ctxt: &ty::ctxt<'tcx>) - -> Option { + -> Aliasability { // Maybe non-obvious: copied upvars can only be considered // non-aliasable in once closures, since any other kind can be // aliased and eventually recused. @@ -1393,17 +1449,27 @@ impl<'tcx> cmt_<'tcx> { cat_deref(ref b, _, BorrowedPtr(ty::UniqueImmBorrow, _)) | cat_deref(ref b, _, Implicit(ty::UniqueImmBorrow, _)) | cat_downcast(ref b, _) | - cat_deref(ref b, _, Unique) | cat_interior(ref b, _) => { // Aliasability depends on base cmt b.freely_aliasable(ctxt) } + cat_deref(ref b, _, Unique) => { + let sub = b.freely_aliasable(ctxt); + if b.mutbl.is_mutable() { + // Aliasability depends on base cmt alone + sub + } else { + // Do not allow mutation through an immutable box. + ImmutableUnique(Box::new(sub)) + } + } + cat_rvalue(..) | cat_local(..) | cat_upvar(..) | cat_deref(_, _, UnsafePtr(..)) => { // yes, it's aliasable, but... - None + NonAliasable } cat_static_item(..) => { @@ -1414,17 +1480,18 @@ impl<'tcx> cmt_<'tcx> { }; if self.mutbl.is_mutable() { - Some(AliasableStaticMut(int_safe)) + FreelyAliasable(AliasableStaticMut(int_safe)) } else { - Some(AliasableStatic(int_safe)) + FreelyAliasable(AliasableStatic(int_safe)) } } cat_deref(ref base, _, BorrowedPtr(ty::ImmBorrow, _)) | cat_deref(ref base, _, Implicit(ty::ImmBorrow, _)) => { match base.cat { - cat_upvar(Upvar{ id, .. }) => Some(AliasableClosure(id.closure_expr_id)), - _ => Some(AliasableBorrowed) + cat_upvar(Upvar{ id, .. }) => + FreelyAliasable(AliasableClosure(id.closure_expr_id)), + _ => FreelyAliasable(AliasableBorrowed) } } } diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index 23ca5b636815..2187494caff9 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -943,13 +943,20 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { cmt: mc::cmt<'tcx>) -> bool { match cmt.freely_aliasable(this.tcx()) { - None => { + mc::Aliasability::NonAliasable => { return true; } - Some(mc::AliasableStaticMut(..)) => { + mc::Aliasability::FreelyAliasable(mc::AliasableStaticMut(..)) => { return true; } - Some(cause) => { + mc::Aliasability::ImmutableUnique(_) => { + this.bccx.report_aliasability_violation( + span, + MutabilityViolation, + mc::AliasableReason::UnaliasableImmutable); + return false; + } + mc::Aliasability::FreelyAliasable(cause) => { this.bccx.report_aliasability_violation( span, MutabilityViolation, diff --git a/src/librustc_borrowck/borrowck/gather_loans/mod.rs b/src/librustc_borrowck/borrowck/gather_loans/mod.rs index 7d77eb23b6ed..7788a2a7471a 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/mod.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/mod.rs @@ -151,10 +151,11 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for GatherLoanCtxt<'a, 'tcx> { assignee_cmt: mc::cmt<'tcx>, mode: euv::MutateMode) { - debug!("mutate(assignment_id={}, assignee_cmt={})", - assignment_id, assignee_cmt.repr(self.tcx())); + let opt_lp = opt_loan_path(&assignee_cmt); + debug!("mutate(assignment_id={}, assignee_cmt={}) opt_lp={:?}", + assignment_id, assignee_cmt.repr(self.tcx()), opt_lp); - match opt_loan_path(&assignee_cmt) { + match opt_lp { Some(lp) => { gather_moves::gather_assignment(self.bccx, &self.move_data, assignment_id, assignment_span, @@ -181,12 +182,16 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, req_kind: ty::BorrowKind) -> Result<(),()> { - match (cmt.freely_aliasable(bccx.tcx), req_kind) { - (None, _) => { + let aliasability = cmt.freely_aliasable(bccx.tcx); + debug!("check_aliasability aliasability={:?} req_kind={:?}", + aliasability, req_kind); + + match (aliasability, req_kind) { + (mc::Aliasability::NonAliasable, _) => { /* Uniquely accessible path -- OK for `&` and `&mut` */ Ok(()) } - (Some(mc::AliasableStatic(safety)), ty::ImmBorrow) => { + (mc::Aliasability::FreelyAliasable(mc::AliasableStatic(safety)), ty::ImmBorrow) => { // Borrow of an immutable static item: match safety { mc::InteriorUnsafe => { @@ -202,13 +207,20 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, } } } - (Some(mc::AliasableStaticMut(..)), _) => { + (mc::Aliasability::FreelyAliasable(mc::AliasableStaticMut(..)), _) => { // Even touching a static mut is considered unsafe. We assume the // user knows what they're doing in these cases. Ok(()) } - (Some(alias_cause), ty::UniqueImmBorrow) | - (Some(alias_cause), ty::MutBorrow) => { + (mc::Aliasability::ImmutableUnique(_), ty::MutBorrow) => { + bccx.report_aliasability_violation( + borrow_span, + BorrowViolation(loan_cause), + mc::AliasableReason::UnaliasableImmutable); + Err(()) + } + (mc::Aliasability::FreelyAliasable(alias_cause), ty::UniqueImmBorrow) | + (mc::Aliasability::FreelyAliasable(alias_cause), ty::MutBorrow) => { bccx.report_aliasability_violation( borrow_span, BorrowViolation(loan_cause), @@ -376,7 +388,8 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> { req_kind: ty::BorrowKind) -> Result<(),()> { //! Implements the M-* rules in README.md. - + debug!("check_mutability(cause={:?} cmt={} req_kind={:?}", + cause, cmt.repr(bccx.tcx), req_kind); match req_kind { ty::UniqueImmBorrow | ty::ImmBorrow => { match cmt.mutbl { diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index b176d8d4118a..a748d94e08d6 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -844,6 +844,12 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { &format!("{} in an aliasable location", prefix)); } + mc::AliasableReason::UnaliasableImmutable => { + self.tcx.sess.span_err( + span, + &format!("{} in an immutable container", + prefix)); + } mc::AliasableClosure(id) => { self.tcx.sess.span_err(span, &format!("{} in a captured outer \ diff --git a/src/test/compile-fail/borrowck-issue-14498.rs b/src/test/compile-fail/borrowck-issue-14498.rs index 8278b4fb971c..64033623fe2d 100644 --- a/src/test/compile-fail/borrowck-issue-14498.rs +++ b/src/test/compile-fail/borrowck-issue-14498.rs @@ -9,56 +9,116 @@ // except according to those terms. // This tests that we can't modify Box<&mut T> contents while they -// are borrowed. +// are borrowed (#14498). +// +// Also includes tests of the errors reported when the Box in question +// is immutable (#14270). #![feature(box_syntax)] struct A { a: isize } struct B<'a> { a: Box<&'a mut isize> } +fn indirect_write_to_imm_box() { + let mut x: isize = 1; + let y: Box<_> = box &mut x; + let p = &y; + ***p = 2; //~ ERROR cannot assign to data in an immutable container + drop(p); +} + fn borrow_in_var_from_var() { + let mut x: isize = 1; + let mut y: Box<_> = box &mut x; + let p = &y; + let q = &***p; + **y = 2; //~ ERROR cannot assign to `**y` because it is borrowed + drop(p); + drop(q); +} + +fn borrow_in_var_from_var_via_imm_box() { let mut x: isize = 1; let y: Box<_> = box &mut x; let p = &y; let q = &***p; **y = 2; //~ ERROR cannot assign to `**y` because it is borrowed + //~^ ERROR cannot assign to data in an immutable container drop(p); drop(q); } fn borrow_in_var_from_field() { + let mut x = A { a: 1 }; + let mut y: Box<_> = box &mut x.a; + let p = &y; + let q = &***p; + **y = 2; //~ ERROR cannot assign to `**y` because it is borrowed + drop(p); + drop(q); +} + +fn borrow_in_var_from_field_via_imm_box() { let mut x = A { a: 1 }; let y: Box<_> = box &mut x.a; let p = &y; let q = &***p; **y = 2; //~ ERROR cannot assign to `**y` because it is borrowed + //~^ ERROR cannot assign to data in an immutable container drop(p); drop(q); } fn borrow_in_field_from_var() { + let mut x: isize = 1; + let mut y = B { a: box &mut x }; + let p = &y.a; + let q = &***p; + **y.a = 2; //~ ERROR cannot assign to `**y.a` because it is borrowed + drop(p); + drop(q); +} + +fn borrow_in_field_from_var_via_imm_box() { let mut x: isize = 1; let y = B { a: box &mut x }; let p = &y.a; let q = &***p; **y.a = 2; //~ ERROR cannot assign to `**y.a` because it is borrowed + //~^ ERROR cannot assign to data in an immutable container drop(p); drop(q); } fn borrow_in_field_from_field() { + let mut x = A { a: 1 }; + let mut y = B { a: box &mut x.a }; + let p = &y.a; + let q = &***p; + **y.a = 2; //~ ERROR cannot assign to `**y.a` because it is borrowed + drop(p); + drop(q); +} + +fn borrow_in_field_from_field_via_imm_box() { let mut x = A { a: 1 }; let y = B { a: box &mut x.a }; let p = &y.a; let q = &***p; **y.a = 2; //~ ERROR cannot assign to `**y.a` because it is borrowed + //~^ ERROR cannot assign to data in an immutable container drop(p); drop(q); } fn main() { + indirect_write_to_imm_box(); borrow_in_var_from_var(); + borrow_in_var_from_var_via_imm_box(); borrow_in_var_from_field(); + borrow_in_var_from_field_via_imm_box(); borrow_in_field_from_var(); + borrow_in_field_from_var_via_imm_box(); borrow_in_field_from_field(); + borrow_in_field_from_field_via_imm_box(); }