From f513380cf51eb5fd977b8815a7acd999e424dc93 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 30 Mar 2015 01:11:11 +0200 Subject: [PATCH] Address Issue 14270 by making `cmt::freely_aliasable` result more fine-grained. Instead of encoding the aliasability (i.e. whether the cmt is uniquely writable or not) as an option, now pass back an enum indicating either: 1. freely-aliasable (thus not uniquely-writable), 2. non-aliasble (thus uniquely writable), or 3. unique but immutable (and thus not uniquely writable, according to proposal from issue 14270.) This is all of course a giant hack that will hopefully go away with an eventually removal of special treatment of `Box` (aka `ty_unique`) from the compiler. --- src/librustc/middle/mem_categorization.rs | 41 ++++++++++++++----- src/librustc_borrowck/borrowck/check_loans.rs | 13 ++++-- .../borrowck/gather_loans/mod.rs | 23 ++++++++--- src/librustc_borrowck/borrowck/mod.rs | 6 +++ 4 files changed, 64 insertions(+), 19 deletions(-) diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index bf028accac6..b213d71a78d 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; @@ -1387,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), } @@ -1426,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. @@ -1439,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(..) => { @@ -1460,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 23ca5b63681..2187494caff 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 60c6e37768d..7788a2a7471 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/mod.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/mod.rs @@ -182,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 => { @@ -203,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), diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index b176d8d4118..a748d94e08d 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 \