diff --git a/src/librustc/middle/astencode.rs b/src/librustc/middle/astencode.rs index a22daac90b5..857579b5bf0 100644 --- a/src/librustc/middle/astencode.rs +++ b/src/librustc/middle/astencode.rs @@ -383,7 +383,7 @@ impl tr for ast::def { ast::def_method(did0.tr(xcx), did1.map(|did1| did1.tr(xcx))) } ast::def_self_ty(nid) => { ast::def_self_ty(xcx.tr_id(nid)) } - ast::def_self(nid, i) => { ast::def_self(xcx.tr_id(nid), i) } + ast::def_self(nid) => { ast::def_self(xcx.tr_id(nid)) } ast::def_mod(did) => { ast::def_mod(did.tr(xcx)) } ast::def_foreign_mod(did) => { ast::def_foreign_mod(did.tr(xcx)) } ast::def_static(did, m) => { ast::def_static(did.tr(xcx), m) } diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 0fa7750afa7..4991f75dc3e 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -367,7 +367,6 @@ impl<'self> CheckLoanCtxt<'self> { mc::cat_rvalue(*) | mc::cat_static_item | - mc::cat_implicit_self | mc::cat_copied_upvar(*) | mc::cat_deref(_, _, mc::unsafe_ptr(*)) | mc::cat_deref(_, _, mc::gc_ptr(*)) | @@ -406,15 +405,7 @@ impl<'self> CheckLoanCtxt<'self> { mc::cat_deref(b, _, mc::region_ptr(m_mutbl, _)) => { // Statically prohibit writes to `&mut` when aliasable - match b.freely_aliasable() { - None => {} - Some(cause) => { - this.bccx.report_aliasability_violation( - expr.span, - MutabilityViolation, - cause); - } - } + check_for_aliasability_violation(this, expr, b); } mc::cat_deref(_, deref_count, mc::gc_ptr(ast::m_mutbl)) => { @@ -434,6 +425,51 @@ impl<'self> CheckLoanCtxt<'self> { return true; // no errors reported } + fn check_for_aliasability_violation(this: &CheckLoanCtxt, + expr: @ast::expr, + cmt: mc::cmt) -> bool { + let mut cmt = cmt; + + loop { + match cmt.cat { + mc::cat_deref(b, _, mc::region_ptr(m_mutbl, _)) | + mc::cat_downcast(b) | + mc::cat_stack_upvar(b) | + mc::cat_deref(b, _, mc::uniq_ptr) | + mc::cat_interior(b, _) | + mc::cat_discr(b, _) => { + // Aliasability depends on base cmt + cmt = b; + } + + mc::cat_copied_upvar(_) | + mc::cat_rvalue(*) | + mc::cat_local(*) | + mc::cat_arg(_) | + mc::cat_self(*) | + mc::cat_deref(_, _, mc::unsafe_ptr(*)) | + mc::cat_static_item(*) | + mc::cat_deref(_, _, mc::gc_ptr(_)) | + mc::cat_deref(_, _, mc::region_ptr(m_const, _)) | + mc::cat_deref(_, _, mc::region_ptr(m_imm, _)) => { + // Aliasability is independent of base cmt + match cmt.freely_aliasable() { + None => { + return true; + } + Some(cause) => { + this.bccx.report_aliasability_violation( + expr.span, + MutabilityViolation, + cause); + return false; + } + } + } + } + } + } + fn check_for_assignment_to_restricted_or_frozen_location( this: &CheckLoanCtxt, expr: @ast::expr, @@ -511,6 +547,12 @@ impl<'self> CheckLoanCtxt<'self> { // path, and check that the super path was not lent out as // mutable or immutable (a const loan is ok). // + // Mutability of a path can be dependent on the super path + // in two ways. First, it might be inherited mutability. + // Second, the pointee of an `&mut` pointer can only be + // mutated if it is found in an unaliased location, so we + // have to check that the owner location is not borrowed. + // // Note that we are *not* checking for any and all // restrictions. We are only interested in the pointers // that the user created, whereas we add restrictions for @@ -528,9 +570,12 @@ impl<'self> CheckLoanCtxt<'self> { let mut loan_path = loan_path; loop { match *loan_path { - // Peel back one layer if `loan_path` has - // inherited mutability - LpExtend(lp_base, mc::McInherited, _) => { + // Peel back one layer if, for `loan_path` to be + // mutable, `lp_base` must be mutable. This occurs + // with inherited mutability and with `&mut` + // pointers. + LpExtend(lp_base, mc::McInherited, _) | + LpExtend(lp_base, _, LpDeref(mc::region_ptr(ast::m_mutbl, _))) => { loan_path = lp_base; } diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index 24a6e5b6e0b..d6f9d1db7df 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -100,7 +100,6 @@ fn check_is_legal_to_move_from(bccx: @BorrowckCtxt, cmt0: mc::cmt, cmt: mc::cmt) -> bool { match cmt.cat { - mc::cat_implicit_self(*) | mc::cat_deref(_, _, mc::region_ptr(*)) | mc::cat_deref(_, _, mc::gc_ptr(*)) | mc::cat_deref(_, _, mc::unsafe_ptr(*)) => { diff --git a/src/librustc/middle/borrowck/gather_loans/lifetime.rs b/src/librustc/middle/borrowck/gather_loans/lifetime.rs index b315a7a2e72..1dcfc515139 100644 --- a/src/librustc/middle/borrowck/gather_loans/lifetime.rs +++ b/src/librustc/middle/borrowck/gather_loans/lifetime.rs @@ -68,7 +68,6 @@ impl GuaranteeLifetimeContext { match cmt.cat { mc::cat_rvalue(*) | - mc::cat_implicit_self | mc::cat_copied_upvar(*) | // L-Local mc::cat_local(*) | // L-Local mc::cat_arg(*) | // L-Local @@ -301,7 +300,6 @@ impl GuaranteeLifetimeContext { } mc::cat_rvalue(*) | mc::cat_static_item | - mc::cat_implicit_self | mc::cat_copied_upvar(*) | mc::cat_deref(*) => { false @@ -328,7 +326,6 @@ impl GuaranteeLifetimeContext { mc::cat_rvalue(cleanup_scope_id) => { ty::re_scope(cleanup_scope_id) } - mc::cat_implicit_self | mc::cat_copied_upvar(_) => { ty::re_scope(self.item_scope_id) } diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 46bb23e400e..ccfef63d9bc 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -101,7 +101,7 @@ impl RestrictionsContext { self.extend(result, cmt.mutbl, LpInterior(i), restrictions) } - mc::cat_deref(cmt_base, _, mc::uniq_ptr) => { + mc::cat_deref(cmt_base, _, pk @ mc::uniq_ptr) => { // R-Deref-Send-Pointer // // When we borrow the interior of an owned pointer, we @@ -110,12 +110,11 @@ impl RestrictionsContext { let result = self.restrict( cmt_base, restrictions | RESTR_MUTATE | RESTR_CLAIM); - self.extend(result, cmt.mutbl, LpDeref, restrictions) + self.extend(result, cmt.mutbl, LpDeref(pk), restrictions) } mc::cat_copied_upvar(*) | // FIXME(#2152) allow mutation of upvars mc::cat_static_item(*) | - mc::cat_implicit_self(*) | mc::cat_deref(_, _, mc::region_ptr(m_imm, _)) | mc::cat_deref(_, _, mc::gc_ptr(m_imm)) => { // R-Deref-Imm-Borrowed @@ -129,7 +128,7 @@ impl RestrictionsContext { Safe } - mc::cat_deref(cmt_base, _, mc::gc_ptr(m_mutbl)) => { + mc::cat_deref(cmt_base, _, pk @ mc::gc_ptr(m_mutbl)) => { // R-Deref-Managed-Borrowed // // Technically, no restrictions are *necessary* here. @@ -170,14 +169,14 @@ impl RestrictionsContext { match opt_loan_path(cmt_base) { None => Safe, Some(lp_base) => { - let lp = @LpExtend(lp_base, cmt.mutbl, LpDeref); + let lp = @LpExtend(lp_base, cmt.mutbl, LpDeref(pk)); SafeIf(lp, ~[Restriction {loan_path: lp, set: restrictions}]) } } } - mc::cat_deref(cmt_base, _, mc::region_ptr(m_mutbl, _)) => { + mc::cat_deref(cmt_base, _, pk @ mc::region_ptr(m_mutbl, _)) => { // Because an `&mut` pointer does not inherit its // mutability, we can only prevent mutation or prevent // freezing if it is not aliased. Therefore, in such @@ -187,7 +186,7 @@ impl RestrictionsContext { let result = self.restrict( cmt_base, RESTR_ALIAS | RESTR_MUTATE | RESTR_CLAIM); - self.extend(result, cmt.mutbl, LpDeref, restrictions) + self.extend(result, cmt.mutbl, LpDeref(pk), restrictions) } else { // R-Deref-Mut-Borrowed-2 Safe diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 95eae32922b..4490fe65d91 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -261,7 +261,7 @@ pub enum LoanPath { #[deriving(Eq, IterBytes)] pub enum LoanPathElem { - LpDeref, // `*LV` in doc.rs + LpDeref(mc::PointerKind), // `*LV` in doc.rs LpInterior(mc::InteriorKind) // `LV.f` in doc.rs } @@ -284,8 +284,7 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> { match cmt.cat { mc::cat_rvalue(*) | mc::cat_static_item | - mc::cat_copied_upvar(_) | - mc::cat_implicit_self => { + mc::cat_copied_upvar(_) => { None } @@ -295,9 +294,9 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> { Some(@LpVar(id)) } - mc::cat_deref(cmt_base, _, _) => { + mc::cat_deref(cmt_base, _, pk) => { do opt_loan_path(cmt_base).map_move |lp| { - @LpExtend(lp, cmt.mutbl, LpDeref) + @LpExtend(lp, cmt.mutbl, LpDeref(pk)) } } @@ -728,7 +727,7 @@ impl BorrowckCtxt { loan_path: &LoanPath, out: &mut ~str) { match *loan_path { - LpExtend(_, _, LpDeref) => { + LpExtend(_, _, LpDeref(_)) => { out.push_char('('); self.append_loan_path_to_str(loan_path, out); out.push_char(')'); @@ -776,7 +775,7 @@ impl BorrowckCtxt { out.push_str("[]"); } - LpExtend(lp_base, _, LpDeref) => { + LpExtend(lp_base, _, LpDeref(_)) => { out.push_char('*'); self.append_loan_path_to_str(lp_base, out); } @@ -854,7 +853,7 @@ impl Repr for LoanPath { fmt!("$(%?)", id) } - &LpExtend(lp, _, LpDeref) => { + &LpExtend(lp, _, LpDeref(_)) => { fmt!("%s.*", lp.repr(tcx)) } diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 283724447f8..8be9a5db8bb 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -59,18 +59,17 @@ use syntax::print::pprust; #[deriving(Eq)] pub enum categorization { - cat_rvalue(ast::NodeId), // temporary val, argument is its scope + cat_rvalue(ast::NodeId), // temporary val, argument is its scope cat_static_item, - cat_implicit_self, cat_copied_upvar(CopiedUpvar), // upvar copied into @fn or ~fn env cat_stack_upvar(cmt), // by ref upvar from &fn - cat_local(ast::NodeId), // local variable - cat_arg(ast::NodeId), // formal argument - cat_deref(cmt, uint, ptr_kind), // deref of a ptr + cat_local(ast::NodeId), // local variable + cat_arg(ast::NodeId), // formal argument + cat_deref(cmt, uint, PointerKind), // deref of a ptr cat_interior(cmt, InteriorKind), // something interior: field, tuple, etc cat_downcast(cmt), // selects a particular enum variant (*) - cat_discr(cmt, ast::NodeId), // match discriminant (see preserve()) - cat_self(ast::NodeId), // explicit `self` + cat_discr(cmt, ast::NodeId), // match discriminant (see preserve()) + cat_self(ast::NodeId), // explicit `self` // (*) downcast is only required if the enum has more than one variant } @@ -82,8 +81,8 @@ pub struct CopiedUpvar { } // different kinds of pointers: -#[deriving(Eq)] -pub enum ptr_kind { +#[deriving(Eq, IterBytes)] +pub enum PointerKind { uniq_ptr, gc_ptr(ast::mutability), region_ptr(ast::mutability, ty::Region), @@ -147,7 +146,7 @@ pub type cmt = @cmt_; // We pun on *T to mean both actual deref of a ptr as well // as accessing of components: pub enum deref_kind { - deref_ptr(ptr_kind), + deref_ptr(PointerKind), deref_interior(InteriorKind), } @@ -493,17 +492,11 @@ impl mem_categorization_ctxt { } } - ast::def_self(self_id, is_implicit) => { - let cat = if is_implicit { - cat_implicit_self - } else { - cat_self(self_id) - }; - + ast::def_self(self_id) => { @cmt_ { id:id, span:span, - cat:cat, + cat:cat_self(self_id), mutbl: McImmutable, ty:expr_ty } @@ -1016,9 +1009,6 @@ impl mem_categorization_ctxt { cat_static_item => { ~"static item" } - cat_implicit_self => { - ~"self reference" - } cat_copied_upvar(_) => { ~"captured outer variable in a heap closure" } @@ -1121,7 +1111,6 @@ impl cmt_ { match self.cat { cat_rvalue(*) | cat_static_item | - cat_implicit_self | cat_copied_upvar(*) | cat_local(*) | cat_self(*) | @@ -1146,9 +1135,10 @@ impl cmt_ { } pub fn freely_aliasable(&self) -> Option { - //! True if this lvalue resides in an area that is - //! freely aliasable, meaning that rustc cannot track - //! the alias//es with precision. + /*! + * Returns `Some(_)` if this lvalue represents a freely aliasable + * pointer type. + */ // Maybe non-obvious: copied upvars can only be considered // non-aliasable in once closures, since any other kind can be @@ -1166,8 +1156,7 @@ impl cmt_ { } cat_copied_upvar(CopiedUpvar {onceness: ast::Many, _}) | - cat_static_item(*) | - cat_implicit_self(*) => { + cat_static_item(*) => { Some(AliasableOther) } @@ -1180,12 +1169,12 @@ impl cmt_ { Some(AliasableBorrowed(m)) } - cat_downcast(b) | - cat_stack_upvar(b) | - cat_deref(b, _, uniq_ptr) | - cat_interior(b, _) | - cat_discr(b, _) => { - b.freely_aliasable() + cat_downcast(*) | + cat_stack_upvar(*) | + cat_deref(_, _, uniq_ptr) | + cat_interior(*) | + cat_discr(*) => { + None } } } @@ -1205,7 +1194,6 @@ impl Repr for categorization { fn repr(&self, tcx: ty::ctxt) -> ~str { match *self { cat_static_item | - cat_implicit_self | cat_rvalue(*) | cat_copied_upvar(*) | cat_local(*) | @@ -1233,7 +1221,7 @@ impl Repr for categorization { } } -pub fn ptr_sigil(ptr: ptr_kind) -> ~str { +pub fn ptr_sigil(ptr: PointerKind) -> ~str { match ptr { uniq_ptr => ~"~", gc_ptr(_) => ~"@", diff --git a/src/librustc/middle/moves.rs b/src/librustc/middle/moves.rs index 7b3fad0b36b..9f55e5e3509 100644 --- a/src/librustc/middle/moves.rs +++ b/src/librustc/middle/moves.rs @@ -228,7 +228,7 @@ pub fn moved_variable_node_id_from_def(def: def) -> Option { def_binding(nid, _) | def_arg(nid, _) | def_local(nid, _) | - def_self(nid, _) => Some(nid), + def_self(nid) => Some(nid), _ => None } diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index 0dee86e2a19..d60d1e2b845 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -125,7 +125,7 @@ pub enum Mutability { pub enum SelfBinding { NoSelfBinding, - HasSelfBinding(NodeId, bool /* is implicit */) + HasSelfBinding(NodeId) } struct ResolveVisitor { @@ -3771,9 +3771,8 @@ impl Resolver { NoSelfBinding => { // Nothing to do. } - HasSelfBinding(self_node_id, is_implicit) => { - let def_like = dl_def(def_self(self_node_id, - is_implicit)); + HasSelfBinding(self_node_id) => { + let def_like = dl_def(def_self(self_node_id)); *function_value_rib.self_binding = Some(def_like); } } @@ -3917,7 +3916,7 @@ impl Resolver { // we only have self ty if it is a non static method let self_binding = match method.explicit_self.node { sty_static => { NoSelfBinding } - _ => { HasSelfBinding(method.self_id, false) } + _ => { HasSelfBinding(method.self_id) } }; self.resolve_function(rib_kind, diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs index 06c45e563c0..6f981123f5f 100644 --- a/src/librustc/middle/trans/expr.rs +++ b/src/librustc/middle/trans/expr.rs @@ -1066,7 +1066,7 @@ pub fn trans_local_var(bcx: @mut Block, def: ast::def) -> Datum { ast::def_local(nid, _) | ast::def_binding(nid, _) => { take_local(bcx, bcx.fcx.lllocals, nid) } - ast::def_self(nid, _) => { + ast::def_self(nid) => { let self_info: ValSelfData = match bcx.fcx.llself { Some(ref self_info) => *self_info, None => { diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index a839ea976e6..0c8f2229424 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -3093,7 +3093,7 @@ pub fn ty_param_bounds_and_ty_for_def(fcx: @mut FnCtxt, defn: ast::def) -> ty_param_bounds_and_ty { match defn { - ast::def_arg(nid, _) | ast::def_local(nid, _) | ast::def_self(nid, _) | + ast::def_arg(nid, _) | ast::def_local(nid, _) | ast::def_self(nid) | ast::def_binding(nid, _) => { let typ = fcx.local_ty(sp, nid); return no_params(typ); diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index 05f1323805b..b25d81056b0 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -58,7 +58,7 @@ fn encl_region_of_def(fcx: @mut FnCtxt, def: ast::def) -> ty::Region { let tcx = fcx.tcx(); match def { def_local(node_id, _) | def_arg(node_id, _) | - def_self(node_id, _) | def_binding(node_id, _) => { + def_self(node_id) | def_binding(node_id, _) => { tcx.region_maps.encl_region(node_id) } def_upvar(_, subdef, closure_id, body_id) => { diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index b01c19a59c1..f09c7427ecc 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -171,7 +171,7 @@ pub enum def { def_static_method(/* method */ def_id, /* trait */ Option, purity), - def_self(NodeId, bool /* is_implicit */), + def_self(NodeId), def_self_ty(/* trait id */ NodeId), def_mod(def_id), def_foreign_mod(def_id), diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index cfbe61ca65e..a39da4301ba 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -64,7 +64,7 @@ pub fn def_id_of_def(d: def) -> def_id { def_use(id) | def_struct(id) | def_trait(id) | def_method(id, _) => { id } - def_arg(id, _) | def_local(id, _) | def_self(id, _) | def_self_ty(id) + def_arg(id, _) | def_local(id, _) | def_self(id) | def_self_ty(id) | def_upvar(id, _, _, _) | def_binding(id, _) | def_region(id) | def_typaram_binder(id) | def_label(id) => { local_def(id) diff --git a/src/test/compile-fail/borrowck-assign-to-andmut-in-aliasable-loc.rs b/src/test/compile-fail/borrowck-assign-to-andmut-in-aliasable-loc.rs new file mode 100644 index 00000000000..e4a23e74a12 --- /dev/null +++ b/src/test/compile-fail/borrowck-assign-to-andmut-in-aliasable-loc.rs @@ -0,0 +1,30 @@ +// Copyright 2012 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that assignments to an `&mut` pointer which is found in a +// borrowed (but otherwise non-aliasable) location is illegal. + +struct S<'self> { + pointer: &'self mut int +} + +fn a(s: &S) { + *s.pointer += 1; //~ ERROR cannot assign +} + +fn b(s: &mut S) { + *s.pointer += 1; +} + +fn c(s: & &mut S) { + *s.pointer += 1; //~ ERROR cannot assign +} + +fn main() {} diff --git a/src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs b/src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs new file mode 100644 index 00000000000..dcef74b6c2b --- /dev/null +++ b/src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs @@ -0,0 +1,31 @@ +// Copyright 2012 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that assignments to an `&mut` pointer which is found in a +// borrowed (but otherwise non-aliasable) location is illegal. + +struct S<'self> { + pointer: &'self mut int +} + +fn copy_borrowed_ptr<'a>(p: &'a mut S<'a>) -> S<'a> { + S { pointer: &mut *p.pointer } +} + +fn main() { + let mut x = 1; + + { + let mut y = S { pointer: &mut x }; + let z = copy_borrowed_ptr(&mut y); + *y.pointer += 1; //~ ERROR cannot assign + *z.pointer += 1; + } +}