diff options
| -rw-r--r-- | src/librustc/metadata/encoder.rs | 2 | ||||
| -rw-r--r-- | src/librustc/middle/borrowck/check_loans.rs | 56 | ||||
| -rw-r--r-- | src/librustc/middle/borrowck/doc.rs | 68 | ||||
| -rw-r--r-- | src/librustc/middle/borrowck/gather_loans/mod.rs | 51 | ||||
| -rw-r--r-- | src/librustc/middle/borrowck/gather_loans/restrictions.rs | 34 | ||||
| -rw-r--r-- | src/librustc/middle/borrowck/mod.rs | 24 | ||||
| -rw-r--r-- | src/librustc/middle/mem_categorization.rs | 40 | ||||
| -rw-r--r-- | src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs | 5 |
8 files changed, 159 insertions, 121 deletions
diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index 07997577a9f..4abfedb8722 100644 --- a/src/librustc/metadata/encoder.rs +++ b/src/librustc/metadata/encoder.rs @@ -681,7 +681,7 @@ fn encode_explicit_self(ebml_w: &mut writer::Encoder, explicit_self: ast::Explic ebml_w.end_tag(); - fn encode_mutability(ebml_w: &writer::Encoder, + fn encode_mutability(ebml_w: &mut writer::Encoder, m: ast::Mutability) { match m { MutImmutable => { ebml_w.writer.write(&[ 'i' as u8 ]); } diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 306285d23fc..cfd2c0719b0 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -22,7 +22,6 @@ use mc = middle::mem_categorization; use middle::borrowck::*; use middle::moves; use middle::ty; -use syntax::ast::{MutImmutable, MutMutable}; use syntax::ast; use syntax::ast_map; use syntax::ast_util; @@ -220,8 +219,8 @@ impl<'a> CheckLoanCtxt<'a> { // Restrictions that would cause the new loan to be illegal: let illegal_if = match loan2.mutbl { - MutableMutability => RESTR_ALIAS | RESTR_FREEZE | RESTR_CLAIM, - ImmutableMutability => RESTR_ALIAS | RESTR_FREEZE, + MutableMutability => RESTR_FREEZE | RESTR_CLAIM, + ImmutableMutability => RESTR_FREEZE, }; debug!("illegal_if={:?}", illegal_if); @@ -423,7 +422,7 @@ impl<'a> CheckLoanCtxt<'a> { debug!("check_for_aliasable_mutable_writes(cmt={}, guarantor={})", cmt.repr(this.tcx()), guarantor.repr(this.tcx())); match guarantor.cat { - mc::cat_deref(b, _, mc::region_ptr(MutMutable, _)) => { + mc::cat_deref(b, _, mc::region_ptr(ast::MutMutable, _)) => { // Statically prohibit writes to `&mut` when aliasable check_for_aliasability_violation(this, expr, b); @@ -437,43 +436,18 @@ impl<'a> CheckLoanCtxt<'a> { 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(MutMutable, _)) | - 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_deref(_, _, mc::unsafe_ptr(..)) | - mc::cat_static_item(..) | - mc::cat_deref(_, _, mc::gc_ptr) | - mc::cat_deref(_, _, mc::region_ptr(MutImmutable, _)) => { - // 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; - } - } - } + cmt: mc::cmt) + -> bool { + match cmt.freely_aliasable() { + None => { + return true; + } + Some(cause) => { + this.bccx.report_aliasability_violation( + expr.span, + MutabilityViolation, + cause); + return false; } } } diff --git a/src/librustc/middle/borrowck/doc.rs b/src/librustc/middle/borrowck/doc.rs index 04ce2b5846c..ffc269f8cc8 100644 --- a/src/librustc/middle/borrowck/doc.rs +++ b/src/librustc/middle/borrowck/doc.rs @@ -151,14 +151,13 @@ that the value `(*x).f` may be mutated via the newly created reference restrictions `RS` that accompany the loan. The first restriction `((*x).f, [MUTATE, CLAIM, FREEZE])` states that -the lender may not mutate nor freeze `(*x).f`. Mutation is illegal -because `(*x).f` is only supposed to be mutated via the new reference, -not by mutating the original path `(*x).f`. Freezing is +the lender may not mutate, freeze, nor alias `(*x).f`. Mutation is +illegal because `(*x).f` is only supposed to be mutated via the new +reference, not by mutating the original path `(*x).f`. Freezing is illegal because the path now has an `&mut` alias; so even if we the lender were to consider `(*x).f` to be immutable, it might be mutated -via this alias. Both of these restrictions are temporary. They will be -enforced for the lifetime `'a` of the loan. After the loan expires, -the restrictions no longer apply. +via this alias. They will be enforced for the lifetime `'a` of the +loan. After the loan expires, the restrictions no longer apply. The second restriction on `*x` is interesting because it does not apply to the path that was lent (`(*x).f`) but rather to a prefix of @@ -188,11 +187,9 @@ The kinds of expressions which in-scope loans can render illegal are: against mutating `lv`; - *moves*: illegal if there is any in-scope restriction on `lv` at all; - *mutable borrows* (`&mut lv`): illegal there is an in-scope restriction - against mutating `lv` or aliasing `lv`; + against claiming `lv`; - *immutable borrows* (`&lv`): illegal there is an in-scope restriction - against freezing `lv` or aliasing `lv`; -- *read-only borrows* (`&const lv`): illegal there is an in-scope restriction - against aliasing `lv`. + against freezing `lv`. ## Formal rules @@ -238,19 +235,23 @@ live. (This is done via restrictions, read on.) We start with the `gather_loans` pass, which walks the AST looking for borrows. For each borrow, there are three bits of information: the lvalue `LV` being borrowed and the mutability `MQ` and lifetime `LT` -of the resulting pointer. Given those, `gather_loans` applies three +of the resulting pointer. Given those, `gather_loans` applies four validity tests: 1. `MUTABILITY(LV, MQ)`: The mutability of the reference is compatible with the mutability of `LV` (i.e., not borrowing immutable data as mutable). -2. `LIFETIME(LV, LT, MQ)`: The lifetime of the borrow does not exceed +2. `ALIASABLE(LV, MQ)`: The aliasability of the reference is +compatible with the aliasability of `LV`. The goal is to prevent +`&mut` borrows of aliasability data. + +3. `LIFETIME(LV, LT, MQ)`: The lifetime of the borrow does not exceed the lifetime of the value being borrowed. This pass is also responsible for inserting root annotations to keep managed values alive. -3. `RESTRICTIONS(LV, LT, ACTIONS) = RS`: This pass checks and computes the +4. `RESTRICTIONS(LV, LT, ACTIONS) = RS`: This pass checks and computes the restrictions to maintain memory safety. These are the restrictions that will go into the final loan. We'll discuss in more detail below. @@ -313,6 +314,47 @@ be borrowed if MQ is immutable or const: MUTABILITY(*LV, MQ) // M-Deref-Borrowed-Mut TYPE(LV) = &mut Ty +## Checking aliasability + +The goal of the aliasability check is to ensure that we never permit +`&mut` borrows of aliasable data. Formally we define a predicate +`ALIASABLE(LV, MQ)` which if defined means that +"borrowing `LV` with mutability `MQ` is ok". The +Rust code corresponding to this predicate is the function +`check_aliasability()` in `middle::borrowck::gather_loans`. + +### Checking aliasability of variables + +Local variables are never aliasable as they are accessible only within +the stack frame. + + ALIASABLE(X, MQ) // M-Var-Mut + +### Checking aliasable of owned content + +Owned content is aliasable if it is found in an aliasable location: + + ALIASABLE(LV.f, MQ) // M-Field + ALIASABLE(LV, MQ) + + ALIASABLE(*LV, MQ) // M-Deref-Unique + ALIASABLE(LV, MQ) + +### Checking mutability of immutable pointer types + +Immutable pointer types like `&T` are aliasable, and hence can only be +borrowed immutably: + + ALIASABLE(*LV, imm) // M-Deref-Borrowed-Imm + TYPE(LV) = &Ty + +### Checking mutability of mutable pointer types + +`&mut T` can be frozen, so it is acceptable to borrow it as either imm or mut: + + ALIASABLE(*LV, MQ) // M-Deref-Borrowed-Mut + TYPE(LV) = &mut Ty + ## Checking lifetime These rules aim to ensure that no data is borrowed for a scope that exceeds diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index 759131a9a11..957073ac392 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -460,6 +460,11 @@ impl<'a> GatherLoanCtxt<'a> { return; // reported an error, no sense in reporting more. } + // Check that we don't allow mutable borrows of aliasable data. + if check_aliasability(self.bccx, borrow_span, cmt, req_mutbl).is_err() { + return; // reported an error, no sense in reporting more. + } + // Compute the restrictions that are required to enforce the // loan is safe. let restr = restrictions::compute_restrictions( @@ -586,15 +591,53 @@ impl<'a> GatherLoanCtxt<'a> { } } } + + fn check_aliasability(bccx: &BorrowckCtxt, + borrow_span: Span, + cmt: mc::cmt, + req_mutbl: LoanMutability) -> Result<(),()> { + //! Implements the A-* rules in doc.rs. + + match req_mutbl { + ImmutableMutability => { + // both imm and mut data can be lent as imm; + // for mutable data, this is a freeze + Ok(()) + } + + MutableMutability => { + // Check for those cases where we cannot control + // the aliasing and make sure that we are not + // being asked to. + match cmt.freely_aliasable() { + None => { + Ok(()) + } + Some(mc::AliasableStaticMut) => { + // This is nasty, but we ignore the + // aliasing rules if the data is based in + // a `static mut`, since those are always + // unsafe. At your own peril and all that. + Ok(()) + } + Some(cause) => { + bccx.report_aliasability_violation( + borrow_span, + BorrowViolation, + cause); + Err(()) + } + } + } + } + } } pub fn restriction_set(&self, req_mutbl: LoanMutability) -> RestrictionSet { match req_mutbl { - ImmutableMutability => RESTR_EMPTY | RESTR_MUTATE | RESTR_CLAIM, - MutableMutability => { - RESTR_EMPTY | RESTR_MUTATE | RESTR_CLAIM | RESTR_FREEZE - } + ImmutableMutability => RESTR_MUTATE | RESTR_CLAIM, + MutableMutability => RESTR_MUTATE | RESTR_CLAIM | RESTR_FREEZE, } } diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 22e64064c9c..60e0baa3534 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -53,16 +53,6 @@ impl<'a> RestrictionsContext<'a> { fn restrict(&self, cmt: mc::cmt, restrictions: RestrictionSet) -> RestrictionResult { - - // Check for those cases where we cannot control the aliasing - // and make sure that we are not being asked to. - match cmt.freely_aliasable() { - None => {} - Some(cause) => { - self.check_aliasing_permitted(cause, restrictions); - } - } - match cmt.cat { mc::cat_rvalue(..) => { // Effectively, rvalues are stored into a @@ -179,28 +169,4 @@ impl<'a> RestrictionsContext<'a> { } } } - - fn check_aliasing_permitted(&self, - cause: mc::AliasableReason, - restrictions: RestrictionSet) { - //! This method is invoked when the current `cmt` is something - //! where aliasing cannot be controlled. It reports an error if - //! the restrictions required that it not be aliased; currently - //! this only occurs when re-borrowing an `&mut` pointer. - //! - //! NB: To be 100% consistent, we should report an error if - //! RESTR_FREEZE is found, because we cannot prevent freezing, - //! nor would we want to. However, we do not report such an - //! error, because this restriction only occurs when the user - //! is creating an `&mut` pointer to immutable or read-only - //! data, and there is already another piece of code that - //! checks for this condition. - - if restrictions.intersects(RESTR_ALIAS) { - self.bccx.report_aliasability_violation( - self.span, - BorrowViolation, - cause); - } - } } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index c52c21eef73..6a3e2fc63b0 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -316,7 +316,6 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> { // - `RESTR_MUTATE`: The lvalue may not be modified. // - `RESTR_CLAIM`: `&mut` borrows of the lvalue are forbidden. // - `RESTR_FREEZE`: `&` borrows of the lvalue are forbidden. -// - `RESTR_ALIAS`: All borrows of the lvalue are forbidden. // // In addition, no value which is restricted may be moved. Therefore, // restrictions are meaningful even if the RestrictionSet is empty, @@ -336,7 +335,6 @@ pub static RESTR_EMPTY: RestrictionSet = RestrictionSet {bits: 0b0000}; pub static RESTR_MUTATE: RestrictionSet = RestrictionSet {bits: 0b0001}; pub static RESTR_CLAIM: RestrictionSet = RestrictionSet {bits: 0b0010}; pub static RESTR_FREEZE: RestrictionSet = RestrictionSet {bits: 0b0100}; -pub static RESTR_ALIAS: RestrictionSet = RestrictionSet {bits: 0b1000}; impl RestrictionSet { pub fn intersects(&self, restr: RestrictionSet) -> bool { @@ -661,8 +659,8 @@ impl BorrowckCtxt { kind: AliasableViolationKind, cause: mc::AliasableReason) { let prefix = match kind { - MutabilityViolation => "cannot assign to an `&mut`", - BorrowViolation => "cannot borrow an `&mut`" + MutabilityViolation => "cannot assign to data", + BorrowViolation => "cannot borrow data mutably" }; match cause { @@ -671,17 +669,21 @@ impl BorrowckCtxt { span, format!("{} in an aliasable location", prefix)); } + mc::AliasableStatic | + mc::AliasableStaticMut => { + self.tcx.sess.span_err( + span, + format!("{} in a static location", prefix)); + } mc::AliasableManaged => { - self.tcx.sess.span_err(span, format!("{} in a `@` pointer", - prefix)) + self.tcx.sess.span_err( + span, + format!("{} in a `@` pointer", prefix)); } - mc::AliasableBorrowed(m) => { + mc::AliasableBorrowed(_) => { self.tcx.sess.span_err( span, - format!("{} in a `&{}` pointer; \ - try an `&mut` instead", - prefix, - self.mut_to_keyword(m))); + format!("{} in a `&` reference", prefix)); } } } diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index f5e3ff5db34..93f439b653c 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -1100,7 +1100,9 @@ pub fn field_mutbl(tcx: ty::ctxt, pub enum AliasableReason { AliasableManaged, AliasableBorrowed(ast::Mutability), - AliasableOther + AliasableOther, + AliasableStatic, + AliasableStaticMut, } impl cmt_ { @@ -1130,10 +1132,6 @@ impl cmt_ { } } - pub fn is_freely_aliasable(&self) -> bool { - self.freely_aliasable().is_some() - } - pub fn freely_aliasable(&self) -> Option<AliasableReason> { /*! * Returns `Some(_)` if this lvalue represents a freely aliasable @@ -1145,20 +1143,36 @@ impl cmt_ { // aliased and eventually recused. match self.cat { + cat_deref(b, _, region_ptr(MutMutable, _)) | + cat_downcast(b) | + cat_stack_upvar(b) | + cat_deref(b, _, uniq_ptr) | + cat_interior(b, _) | + cat_discr(b, _) => { + // Aliasability depends on base cmt + b.freely_aliasable() + } + cat_copied_upvar(CopiedUpvar {onceness: ast::Once, ..}) | cat_rvalue(..) | cat_local(..) | cat_arg(_) | - cat_deref(_, _, unsafe_ptr(..)) | // of course it is aliasable, but... - cat_deref(_, _, region_ptr(MutMutable, _)) => { + cat_deref(_, _, unsafe_ptr(..)) => { // yes, it's aliasable, but... None } - cat_copied_upvar(CopiedUpvar {onceness: ast::Many, ..}) | - cat_static_item(..) => { + cat_copied_upvar(CopiedUpvar {onceness: ast::Many, ..}) => { Some(AliasableOther) } + cat_static_item(..) => { + if self.mutbl.is_mutable() { + Some(AliasableStaticMut) + } else { + Some(AliasableStatic) + } + } + cat_deref(_, _, gc_ptr) => { Some(AliasableManaged) } @@ -1166,14 +1180,6 @@ impl cmt_ { cat_deref(_, _, region_ptr(m @ MutImmutable, _)) => { Some(AliasableBorrowed(m)) } - - cat_downcast(..) | - cat_stack_upvar(..) | - cat_deref(_, _, uniq_ptr) | - cat_interior(..) | - cat_discr(..) => { - None - } } } } diff --git a/src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs b/src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs index f72dacc2d81..d19538c5d36 100644 --- a/src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs +++ b/src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs @@ -27,5 +27,10 @@ fn foo3(t0: &mut &mut int) { **t1 = 22; } +fn foo4(t0: & &mut int) { + let x: &mut int = &mut **t0; //~ ERROR cannot borrow + *x += 1; +} + fn main() { } |